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

Add initialization for the language before GDScript debug tests #42938

Merged
merged 1 commit into from
Oct 20, 2020

Conversation

vnen
Copy link
Member

@vnen vnen commented Oct 20, 2020

This ensures that the tests will use a full environment with correct settings so global classes and autoloads can be properly found.

Follow up from #41355.
Solves part of #41701.

This ensures that the tests will use a full environment with correct
settings so global classes and autoloads can be properly found.
@vnen vnen added this to the 4.0 milestone Oct 20, 2020
@Xrayez
Copy link
Contributor

Xrayez commented Oct 20, 2020

This is a downside of isolating tests that way, most of the initialization logic used to happen in Main::setup() etc. The more we test stuff in the engine, the more the test environment will look like Main::setup() basically, so perhaps we'll need to think about godotengine/godot-proposals#1307 at some point. It takes lots of work to figure out how the initialization works currently, and quite a bunch of functionality may be difficult to decouple currently, as seen in #40980, I'm not that familiar with engine internals myself...

#40980 provided the "minimal" context so far, but GDScript requires more than that as seen in this PR, I personally expected that the script server is initialized somewhere at the register_core_types() instead. 😃

godot/main/main.cpp

Lines 372 to 376 in 1810825

#ifdef TESTS_ENABLED
// The order is the same as in `Main::setup()`, only core and some editor types
// are initialized here. This also combines `Main::setup2()` initialization.
Error Main::test_setup() {
OS::get_singleton()->initialize();

I'm saying that perhaps ScriptServer::init_languages() should be done in test_setup(), if we also aim to run GDScript tests to run on CI as well: #41074.

Main::test_setup() is run before debug tests, and then Main::test_cleanup() is called.

@vnen
Copy link
Member Author

vnen commented Oct 20, 2020

@Xrayez

I'm saying that perhaps ScriptServer::init_languages() should be done in test_setup(), if we also aim to run GDScript tests to run on CI as well: #41074.

Note that all that I did here is needed if you intend to use singletons and global classes in the scripts (and if we want to test GDScript itself, they are needed). If not, calling GDScriptLanguage::init() is enough, also assuming that the other languages are not used.

I do think we can split the setup in main into chunks so tests can easily call those instead of re-implementing the logic. That will require quite a bit of work which I didn't want to wait before fixing this particular issue.

@vnen vnen merged commit c77466b into godotengine:master Oct 20, 2020
@vnen vnen deleted the gdscript-test-compiler-fix branch October 20, 2020 17:23
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.

3 participants