Skip to content

Tests: Convert test headers to compilation files#112041

Merged
Repiteo merged 1 commit into
godotengine:masterfrom
Repiteo:tests/compiled
Feb 19, 2026
Merged

Tests: Convert test headers to compilation files#112041
Repiteo merged 1 commit into
godotengine:masterfrom
Repiteo:tests/compiled

Conversation

@Repiteo

@Repiteo Repiteo commented Oct 26, 2025

Copy link
Copy Markdown
Contributor

It's no secret that test_main.cpp is, by a significant margin, the most header-intensive file in the repository. This is on account of it including the contents of every single test, as they've been setup in headers. Moving to compilation units would be preferable, but that means that the tests themselves would fail to link, as a result of those scripts being isolated. In trying to look for workarounds, I took to the doctest repo itself and found out how they managed this process in CMake.

One SCons-reimplementation later, and everything actually works! Now all a designated test file has to do is include the define TEST_FORCE_LINK() and pass the name of the file; generator scripts will take care of the rest. No need to redeclare our entire library setup or an immediate need to resolve all encapsulation issues, so the transition isn't nearly as messy as I thought it would be.

There's a lot of changes made in this PR, but that's an inevitability when retooling this many files to be compilation files instead of headers. Several headers had to be added to compilation files, as they no longer included everything transitively. Transitive includes are still somewhat of an issue thanks to test_macros.h, but that can be handled in a followup PR.

Two tests had to be suppressed after the conversion: [Variant] Utility functions and [ClassDB] Add exposed classes, builtin types, and global enums. The silver lining is I'm now reasonably certain that these two are the culprits behind our tests occasionally hanging for hours, as both consistently recreate the failure states when active. Though for whatever reason, the results are often amplified, with both cases outright segfaulting. Both absolutely warrant a more thorough dive, but that is beyond the scope of this PR & most likely would go over my head. EDIT: Tested out on a separate branch and compilation worked again, so this might've been a false positive. Will keep an eye on.

NOTE: This conversion excludes module tests, as they'd be a more involved process than the base tests. As such, #include "modules/modules_tests.gen.h" remains unchanged, meaning test_main.cpp still has some header bloat for the time being. Now that I know this is possible, a followup PR will take care of the module tests once this is merged.

@Ivorforce

Ivorforce commented Oct 26, 2025

Copy link
Copy Markdown
Member

I'd be interested to see a compile time comparison before and after this PR.
On my computer, the file takes a good 10-20 seconds to compile, which often ends up being a critical path for me during compilation. But generally, this 'SCU build' strategy is faster overall, and if we turn this single 10 second thread-time into 50 times 1 second thread times, that should still be an overall loss of performance.

A middle ground could be acceptable, where e.g. we split the one test file into ~10 compile units. This may still leave us with slightly worse overall thread time, but should solve the 'critical path' problem.

Comment thread tests/SCsub Outdated
@Repiteo

Repiteo commented Oct 26, 2025

Copy link
Copy Markdown
Contributor Author

@Ivorforce Better yet, I've just made sure this accounts for SCU builds as well. It does make sense to replicate the old behavior when already expecting a single compilation unit. It's a bit hacky, as SCU registration/identification is pretty hardcoded atm, but it gets the job done

For standard builds, it'd only be a loss for initial compilations, otherwise the only tests that'd be recompiled are those that had differences in their headers. I was going to trim down the headers in stuff like test_macros.h at a later point to keep the scope limited, but I could handle that in this PR if we'd rather gain those benefits right away instead

@akien-mga akien-mga left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me, and I can confirm it reduces incremental rebuild time by a few seconds when test_main.cpp would be recompiled.

@akien-mga akien-mga modified the milestones: 4.x, 4.7 Feb 19, 2026
• Excludes module tests, as they'd be a more involved process
@Repiteo

Repiteo commented Feb 19, 2026

Copy link
Copy Markdown
Contributor Author

Went ahead and un-suppressed the [Variant] Utility functions and [ClassDB] Add exposed classes, builtin types, and global enums tests. I restored them on a separate branch and compilation/testing worked without issue, so that might've been a false positive. Will keep an eye on, but I'll leave them untouched (assuming everything passes here too)

@AThousandShips AThousandShips left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Just two questions

Comment thread tests/core/io/test_http_client.cpp
Comment thread tests/core/io/test_stream_peer_gzip.cpp
@Repiteo Repiteo merged commit f0b1644 into godotengine:master Feb 19, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment