Add integration tests for GDScript

[Xrayez]

Describe the project you are working on: Godot Engine. 🙂

Describe the problem or limitation you are having in your project: GDScript is constantly evolving, bugs are getting fixed, but at the same time, I've experienced many regressions throughout my game development experience with Godot (starting with Godot 2.1), and probably some are left unnoticed, causing various crashes and/or undefined behavior in release builds of the engine.

Describe the feature / enhancement and how it helps to overcome the problem or limitation: I'd like that we have a proper system for testing GDScript implementation. It cannot be unit tested per se (languages are not typically unit tested), but providing a regression/integration suite is, in my opinion, essential to the prosperity of a programming language, which should be robust and stable by definition. See the many language bindings, and I wouldn't be surprised to find out if one of the motivations behind creating those bindings is in fact GDScript instability.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams: I propose that we implement those kind of tests using already integrated doctest unit testing framework. It's still useful even for integration testing because we're interested that any changes to GDScript implementation are properly covered by our GitHub CI.

I insist that we maintain those tests directly within the main repository, so contributors can work on testing out GDScript implementation without much hassle, and to run tests on CI easily, as long as we provide proper documentation for this. Tests can also be marked as "allowed to fail" if GDScript is still under heavy development.

It's not possible to bloat the engine with tests either. You have to explicitly compile the engine with scons tests=yes in order to run tests in the first place. There is godotengine/gdscript-tests which is separated from the main repository. There's modules support for doctest as implemented in godotengine/godot#40720, so this can be made self-contained (modules/gdscript/tests). GDScript being a module is already a separation from core, I don't see a reason why we should split it further away from Godot. Even the tests themselves run through a different entry point.

On the other hand, there's also a fuzzing approach to testing: #168. For these kind of tests, yeah it would certainly make sense to run outside of main repository, because the task is to find bugs by breaking the parser, and not providing a deterministic regression suite, which must be reproducible in contrast.

There are a number of pull requests already in work (some of them already working) to showcase the power of GDScript testing:

• [ ] https://github.com/godotengine/godot/pull/41074
• [x] https://github.com/godotengine/godot/pull/41355
• [ ] https://github.com/godotengine/godot/pull/41616

Possible enhancements marginally related to this proposal: #1307.

If this enhancement will not be used often, can it be worked around with a few lines of script?: It's possible to maintain a set of tests to check whether GDScript is working outside of the main repository, but then it defeats the purpose of the proposal. You need to be able to react to any regressions quickly enough, else those bugs will be left ignored.

Is there a reason why this should be core and not an add-on in the asset library?: GDScript is implemented as a C++ module. It's possible to test GDScript for simple parse errors with godot --script --check-only command-line options, but it doesn't really makes sense because people can usually find bugs (if lucky) by simply working with the engine.

[strank]

I fully agree with this proposal. Doctests in the main repository make it very easy to get fully automated regression testing, and it lets people like me try new features in a test-driven way. Without starting their own setup, because they didn't spend enough time looking for already existing ones outside of the main repo... godotengine/godot#41616 ;)

[vnen]

My main concern about keeping the test cases in the main repo is that there will likely be a lot of those. Most people who clone the repo won't have a need for that because there's not much reason to run the test suite if you're not changing GDScript.

On CI it can clone the submodule for testing, so it shouldn't be much big of a deal.

The only advantage I can see is matching a bug fix with a new regression test case, which could be sent in a single PR if the tests are in the main repo.

[Xrayez]

Quoting @akien-mga https://github.com/godotengine/godot/pull/41616#issuecomment-686340495:

I also wonder about this. If we want to have a full test suite for all GDScript features, each time with a .gd file and more files to show what the expect behavior/output should be, it can quickly be a lot of files. It might make sense to have those in a separate repo that could be added here as a Git submodule.

Having a separate repo as a submodule is OK as long as it only contains the test data (scripts), and not the test runner of course. But I wonder what @reduz thinks about adding submodules to Godot, so far I've seen that we don't prefer using submodules according to Godot documentation:

Please note that Git submodules are not used in the Godot repository. If you are developing a module to be merged into the main Godot repository, you should not use submodules. If your module doesn't get merged in, you can always try to implement the external library as a GDNative C++ plugin.

Also, would it really impact the git checkout operation that much? It would be great if we do have thousands of those scripts of course, but I don't see that happening in the (near) future. 😮

Also, perhaps you worry about that Godot repository will become GDScript-based, and not C++ 😃:

I personally think that would be a great promotion for GDScript!

If we're aiming for something like https://github.com/CyberShadow/DBugTests, then yeah I guess makes sense, but that's probably because more of maintenance aspect I suspect. So far the direction was "everything bundled" in Godot.

The only advantage I can see is matching a bug fix with a new regression test case, which could be sent in a single PR if the tests are in the main repo.

That's the entire point of this proposal, I estimate that only a few of the contributors would be willing to contribute to the regression suite outside of Godot. You'd also have to update the submodule itself everytime in Godot. I believe this should be done often enough because otherwise there's a chance of a bug slipping through the system.

Most people who clone the repo won't have a need for that because there's not much reason to run the test suite if you're not changing GDScript.

If those GDScript tests take too long to execute, doctest has a feature of filtering out tests, either manually or hardcoded via code, But again even if we have 1000 scripts, it should be fast enough to run.

[strank]

I think worrying about too many tests cluttering the repo is a premature worry. In the lucky future where this actually becomes a problem for someone, you can always turn it into a submodule then. The benefit of having code change and test update in the same commit is a big one.

[Calinou]

I don't think the GDScript testing data will surpass ~2 MB anytime soon, so it's probably fine to have it in the main repository.

[Xrayez]

I think worrying about too many tests cluttering the repo is a premature worry. In the lucky future where this actually becomes a problem for someone, you can always turn it into a submodule then.

I don't think the GDScript testing data will surpass ~2 MB anytime soon, so it's probably fine to have it in the main repository.

I fully agree with those statements because The problem has to exist (... in order for it to be solved):

Believing that problems may arise in the future and that the software needs to be ready to solve them by the time they appear is called "Future proofing"...

This is generally considered a bad habit, because trying to solve problems that don't actually exist in the present will very often lead to code that will be written but never used, or to code that is considerably more complex to use and maintain than it needs to be.

I just follow the best practices here, of course there are exceptions to the rules and I think the decision is up to core developers anyways.

But yeah if you think that including test data as part of the main repository will lead to more contributors who will write a lot of tests (which is more likely than having this outside the main repository), then yeah those tests will have to be maintained, surely, but I think the game is totally worth the candle in this case. 😛

[vnen]

Submodules for the actual code is different than submodules for GDScript test files. You would still be able to download and compile Godot without checking out the submodule. The main problem is that they are not included in the source tarball on GitHub, but for test files it isn't a big deal. To be clear: I'm talking about the test cases, not the test suite implementation.

Also if we can tell something will be a problem we can take preventive measures. We don't need to wait until it actually becomes a problem.

I just want to point out that rules exist for a reason when we might prefer breaking the rules than go against the reason itself (at which point we may tweak the wording of the rules).

That is to say that I'm no prominently against keeping the test files in the repo. I just think twice before adding anything to the main repository TBH. If @akien-mga is fine with it we can keep it all in the main repo.

[Xrayez]

To be clear: I'm talking about the test cases, not the test suite implementation.

Yeah we're on the same page.

Also if we can tell something will be a problem we can take preventive measures. We don't need to wait until it actually becomes a problem.

I'm actually happy to hear those words, for a change. 😛

But yeah, I mean, it's almost the same as having tests written in C++, but it just happens to be that those "tests" will be written in GDScript. One can follow the same logic and say that unit tests written in C++ do not belong to the main repository (if fact all existing C++ doctests can be moved out from the tests folder and put into modules/tests which can be added as a git submodule and maintained in a separate repository with the existing implementation. And yeah those kind of tests also have the potential to "bloat" the main repository currently (but only on the level of diffs, number of commits, and files, not binary size for official builds) just like GDScript test data currently, maybe not at the exact same scale but quite close, yet GDScript is only part of the equation called Godot!

So yeah up to @akien-mga or @reduz to decide I think.

[Xrayez]

Legit curious (rhetorical) question: if modules/gdscript were implemented as a git submodule in the main repository, would you also add modules/gdscript/tests/scripts as another nested git submodule inside that module or would the test data remain in a single git submodule? To make it more graphic:

godot ← gdscript (git submodule of godot under "modules/gdscript") ← gdscript_test_data (git submodule of gdscript)

I think this kind of setup would be a hell to maintain.

[Calinou]

This is now implemented in the master branch, closing.