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 Linux sanitizer build to github workspace to test tests and editor #40920

Merged
merged 1 commit into from
Aug 16, 2020

Conversation

qarmin
Copy link
Contributor

@qarmin qarmin commented Jul 31, 2020

We need to test tests because no one need untested tests which results are unpredictable ¯\_(ツ)_/¯

Additionally this build with address(which contains also leak sanitizer on Linux by default) and undefined sanitizer would be really good to run and test editor by xvfb-run tool when GLES 2 will be available.

This will prevent from a lot of bugs which were hidden to end users by default like #40848.

@Xrayez
Copy link
Contributor

Xrayez commented Jul 31, 2020

There are still memory leaks even after #40921 is fixed, I think this requires minimal initialization of core and proper deinit of (probably implicitly initialized) objects, but in any case this signifies that there are certain initialization dependencies which could be either fixed in core or make sure that the test runner initializes stuff in a way to ensure the rest of the testing goes smooth.

But I'm not sure, perhaps the test runner should be actually be called somewhere in Main::setup() or Main::setup2(). 😃

@Xrayez
Copy link
Contributor

Xrayez commented Jul 31, 2020

Actually there was just one thing left which blocked this: #40930.

What I've said above came from me trying to OS::get_singleton()->finalize() after tests, which did cause some crashes related to sanitizer...

@qarmin qarmin force-pushed the test_linux_sanitizers branch from 0806702 to 081e72a Compare August 1, 2020 06:58
@Xrayez
Copy link
Contributor

Xrayez commented Aug 3, 2020

What I've said above came from me trying to OS::get_singleton()->finalize() after tests, which did cause some crashes related to sanitizer...

Managed to port ClassDB tests: #40980. The hidden crash I described above was coming from OS::get_singleton()->finalize() with uninitialized joypads. 👀

@qarmin qarmin force-pushed the test_linux_sanitizers branch 4 times, most recently from b3fdda7 to 8f754f0 Compare August 15, 2020 18:45
@qarmin qarmin force-pushed the test_linux_sanitizers branch from 8f754f0 to 2fec1b9 Compare August 16, 2020 06:32
@akien-mga akien-mga merged commit 87ae509 into godotengine:master Aug 16, 2020
@akien-mga
Copy link
Member

Thanks!


# Upload cache on completion and check it out now
- name: Load .scons_cache directory
id: linux-editor-cache
Copy link
Member

@akien-mga akien-mga Aug 16, 2020

Choose a reason for hiding this comment

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

I merged too fast, this is going to corrupt the cache since both Linux editor builds share the same id/keys.

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