search

MikeSchulze / gdUnit4

A Godot Unit Test Framework. Support for GDScript and C# unit testing
MIT License
567 stars 30 forks source link

GD-380: Allow Godot headless mode by using property `--ignoreHeadlessMode` #380

Closed valknight closed 6 months ago

valknight commented 6 months ago

Is your feature request related to a problem? Please describe.

We cannot currently use GdUnit in our CI pipeline, due to our test running server lacking a display driver of any kind, and as such, requires usage of --headless with Godot. GdUnit errors out when Godot is in headless mode, causing our tests to fail to run.

Describe the solution you'd like

Some way of disabling the headless check when running in these environments, with an acknowledgement of the limitations brought about by doing so.

Describe alternatives you've considered

We currently maintain an internal fork of GdUnit, with line 273 of GdUnitCmdTool.gd changed to the following:

if DisplayServer.get_name() == "headless" and OS.get_environment("HEADLESS_GDUNIT") != "enabled":

This allows us to set HEADLESS_GDUNIT on the server, while ensuring the same checking for these cases of tests that use UI interaction.

valknight commented 6 months ago

To add to this - I'm happy to work on a PR for this if you're OK with this feature. Let me know how you would want this implemented, if at all 😄

MikeSchulze commented 6 months ago

There is a reason why headless mode is not allowed. The problem is that in headless mode the mouse inputs are not accepted and therefore the SceneRunner runs into problems. Have you ever used the gdunit4 action? This makes it very easy to set up a CI without having to worry about the environment.

valknight commented 6 months ago

Hi @MikeSchulze - I'm very much aware of this context. Some important context on our side - we don't have any kind of display server on our CI runner - importantly, we selfhost both our repositories and our CI pipeline.

We fully know that mouse inputs aren't accepted, and as such we're going to be in an unsupported state. However, for our usages, this works fine - and the alternative is the current situation of the entire test runner crashing (instead of giving us an option to continue regardless). Ultimately, we'll take the situation where we can do something, rather than nothing.

As such, your action is still incompatible for us. xvfb-run isn't present, which breaks the workaround as per index.js#L56. For us, we're aware of the limitations surrounding headless mode - and, for us, it's entirely fine for the context of what we need GdUnit to do.

If this is still out of scope, feel free to close this issue - we're happy to maintain our own private fork if this is a strong line in the sand 😄

However, I would say this has been a significant headache for us, considering that --headless is totally supported for builds, and headless modes for unit testing are supported by the big engines. Having a tool that intentionally triggers a crash (instead of a warning of "here be dragons! this is absolutely unsupported") is a pretty major breakage from the norm.

As such - if you are fundamentally opposed to removing the intentional hard crash instead of a warning, would it be worth adding a note about --headless causing these issues to the documentation? Thanks 💕

MikeSchulze commented 6 months ago

I understand your problem, but why do you say that the runner is crashed? That should not be the case, when the headless mode is detected, the runner ends with a custom error and exit code 103.

To solve this problem, I suggest adding a new property --ignoreHeadlessMode to show a warning instead of the actual error and continue the processing.