Add unit and integration tests to the build system (depends on migration to Meson)

[Gorgonx7]

Describe the project you are working on

Writing tests for the engine, and attempting to make tests easier to run for devs

Describe the problem or limitation you are having in your project

Since we are re-writing the build system in Meson #1797 we should probably test that code. We should ensure there are unit tests to cover the core python functionality, as well as integration tests to test that the system works across all platforms. We should additionally try to make it easier for developers to run the tests in visual studio, in SCons you had to go out of visual studio and build via command line if you wanted to run the tests. We should aim to fix this by adding a debug_run_test configuration.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

First off adding unit tests for each function help. We want to ensure that split off into smaller files that make it easier to maintain, instead of the two or three major files that we had with the SCons system

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

We should run the unit tests in pythons built in test library so that there are less platform specific issues. similar to the mocks I started to write for the old SCons system we should aim to do this while writing the new build system if possible so that we can capture requirements easily.

If this enhancement will not be used often, can it be worked around with a few lines of script?

These changes will affect the build system, and therefore can not be worked around

Is there a reason why this should be core and not an add-on in the asset library?

This would be part of the migration to Meson which will be part of the core

[YuriSizov]

I’m not sure what good such unit tests would do us because the most important part of the build system is stability, determinism, ability to reproduce the artifacts. That means building everything for every platform with a number of combinations of arguments. Which is going to be a very long process.

Refactoring though can be a good idea.

[Calinou]

I don't think a refactoring of SCons build scripts is in order since the goal is to eventually move to Meson. Therefore, this proposal will be handled at the same time as #1797.

[Gorgonx7]

@Calinou I had no idea there was a proposal to move to Meson, I'll take a look and see what work needs doing there. Want to close this issue out and move it over to that issue?

@pycbouh That's not the kind of tests I was talking about, tests that would build for every platform with every possible argument would be integration tests as they would actually run the entire code, and I agree they would be worthless. The tests I was proposing would be unit tests on individual methods, see for a test and for the function it was testing.

The goal of writing unit tests is to write a set of very fast, short and repeatable tests that test the individual functionality of classes/functions. When working with legacy code (untested code) such as the SCons scripts we may occasionally want to change it. We need more developers to write the tests as they write functionality as this provides clarity on the intentions of the author, as well as maintainability by ensuring that no matter what if a function is refactored the test confirms the same functionality is present. This would help when making bug fixes, optimizations and new features by confirming the software has not broken in other areas after the fix. In fact I'd go as far as to say it's impossible to refactor well without a good test harness. Hope that provides clarity as to the value of such tests 😃

[YuriSizov]

@Gorgonx7 I think you misunderstood me. I actually think that integration tests would be useful (if we can somehow compare build artifacts with some desired outcome), while unit tests for this part of our project are less so. I know why one would create unit tests in general, I'm saying I don't see a good reason for them here.

Want to close this issue out and move it over to that issue?

You can repurpose this proposal to cover only the testing part, as #1797 already covers the refactoring by its nature.

[Gorgonx7]

Ah ok, from your first comment I didn't get that, I'll rewrite this to cover testing as part of #1797 testing

[Xrayez]

For comparison, if we look at recently implemented integration tests for GDScript (#1429), then those do not actually test the GDScript implementation details like tokenizer, but the parser itself.

And I think that writing unit tests for a buildsystem is quite similar to writing unit tests for GDScript tokenizer. While tests would make the system more robust, there's always a maintenance cost involved. Godot is known to be developed at a fast pace, so existing tests should not significantly decrease the development pace.

Now, if we talk about the SCons project itself, they do write unit tests for literally everything and have a good coverage. SCons development page literally says:

We write a lot of automated tests to test thoroughly test SCons. Lines of test code currently outnumber lines of code in SCons itself by about 3:1.

So, different projects have different priorities.

That said, unit tests for the engine code are more valuable than unit tests for buildsystem code, especially in relation to Godot development. Existing CI checks and the fact that Godot is officially built on various platforms already provides good coverage, not in a sense of unit testing of course, but in a way Godot is deployed/used. It's easier to fix a regression on the buildsystem level than a regression on the engine code level. Godot as a game engine is a soft real-time system, and when something breaks, nobody dies, so-to-speak.

People also have mixed opinions regarding whether contributors should be forced to write unit tests for everything: #1586.

And I'm saying this as the one who's also interested in having unit tests in Godot. I say the main focus should be directed towards completing the test coverage for the core classes, at least: godotengine/godot#43440.

The same applies if we were to switch to Meson at some point.