Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix running tests in template builds #88452

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented Feb 17, 2024

No description provided.

@AThousandShips AThousandShips added this to the 4.3 milestone Feb 17, 2024
@AThousandShips AThousandShips requested a review from a team as a code owner February 17, 2024 18:00
@AThousandShips AThousandShips changed the title Disable default tests with dev_mode without editor Don't default tests = true with dev_mode without editor Feb 17, 2024
@AThousandShips AThousandShips changed the title Don't default tests = true with dev_mode without editor Don't enable tests by default with dev_mode without editor Feb 17, 2024
@AThousandShips AThousandShips force-pushed the template_test branch 2 times, most recently from 8b4304c to 955e4db Compare February 17, 2024 19:02
@AThousandShips

This comment was marked as outdated.

@akien-mga
Copy link
Member

I did some testing and it doesn't really make sense to have tests enabled with export, will look at disabling it completely for this as well

What are the issues? In theory it seems useful to me that we can run unit tests on non-tools builds (especially the release template), to validate that the API behaves as intended even in release builds.

It's not something we would enable for official export templates of course, but ideally we would run such tests on CI. If some tests depend on editor-only functionality, they should be #ifdef TOOLS_ENABLED away.

@AThousandShips
Copy link
Member Author

Indeed, will look deeper into the specific issues, but there are some tests that simply won't work without even being in the correct directory, but will leave this for later digging and not add it here

Note that we couldn't use it in CI unless we do an export completely as the template binary can't be run then

@akien-mga
Copy link
Member

Note that we couldn't use it in CI unless we do an export completely as the template binary can't be run then

--test is a standalone command, it seems to work fine on a template build (aside from it currently running a single test suite, which segfaults):

./bin/godot.linuxbsd.template_debug.dev.x86_64.mono --test
[doctest] doctest version is "2.4.11"
[doctest] run with "--help" for options
===============================================================================
./modules/gdscript/tests/gdscript_test_runner_suite.h:41:
TEST SUITE: [Modules][GDScript]
TEST CASE:  Script compilation and runtime

./modules/gdscript/tests/gdscript_test_runner_suite.h:41: FATAL ERROR: test case CRASHED: SIGSEGV - Segmentation violation signal

===============================================================================
[doctest] test cases:  1 |  0 passed | 1 failed | 866 skipped
[doctest] assertions: 25 | 25 passed | 0 failed |
[doctest] Status: FAILURE!
Segmentation fault (core dumped)

@AThousandShips
Copy link
Member Author

That's strange it popped up the dialog for me first, you're right it works fine in that case, then no worries here, then I'll just leave it as such for now :)

@akien-mga
Copy link
Member

Well actually in second thought, since dev_mode is for engine developers, I do think we should have tests enabled by default in dev builds even for templates. We just need to fix the bugs with running tests, by disabling the tests which can't work.

@AThousandShips
Copy link
Member Author

AThousandShips commented Feb 22, 2024

I'll take a look :)

@AThousandShips AThousandShips marked this pull request as draft February 22, 2024 15:38
@AThousandShips AThousandShips changed the title Don't enable tests by default with dev_mode without editor Fix running tests in template builds Feb 22, 2024
@AThousandShips
Copy link
Member Author

Identified the first issue, the file modules/gdscript/tests/scripts/analyzer/errors/cyclic_ref_override.gd crashes on release builds, unsure how to fix that, will see if I can get better output details

@AThousandShips
Copy link
Member Author

For now I'm disabling some of the offending GDScript tests, to be fixed later, and fixing some other tests and improving them, will push soon

@AThousandShips AThousandShips marked this pull request as ready for review February 22, 2024 17:05
@AThousandShips AThousandShips requested review from a team as code owners February 22, 2024 17:05
@AThousandShips
Copy link
Member Author

AThousandShips commented Feb 22, 2024

There, some temporary fixes in there but this makes it possible to generally run tests, with some improvements

There are still some test cases that fail on disable_3d builds but will look into those specifically in a separate PR as they're a bit trickier, and have some more complex cases

@akien-mga akien-mga merged commit 8a9c9ef into godotengine:master Feb 22, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@AThousandShips AThousandShips deleted the template_test branch February 23, 2024 06:50
@AThousandShips
Copy link
Member Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants