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

SCons: Build tests/ and main/ in cloned environments #40726

Merged
merged 1 commit into from
Jul 26, 2020

Conversation

akien-mga
Copy link
Member

Allows switching tests=yes/no and rebuilding only tests and main,
instead of the whole engine.

Supersedes #40725.

tests/SCsub Outdated Show resolved Hide resolved
Allows switching `tests=yes`/`no` and rebuilding only tests and main,
instead of the whole engine.

Co-authored-by: Andrii Doroshenko (Xrayez) <[email protected]>
@Xrayez
Copy link
Contributor

Xrayez commented Jul 26, 2020

Yeah I don't see where we'd need to use TESTS_ENABLED except for main, and we don't embed test code in other places (if that's a feature of doctest), perhaps it could be hypothetically used for modules #40720 but that's far fetched as you say. 🙂

@akien-mga
Copy link
Member Author

perhaps it could be hypothetically used for modules #40720 but that's far fetched as you say.

Well I do think #40720 is a good change, but I don't think tests written for modules need to have access to TESTS_ENABLED, they'll be called by tests/test_main.cpp which has it defined already. Or do I misunderstand how they work?

@Xrayez
Copy link
Contributor

Xrayez commented Jul 26, 2020

Yeah likely modules don't need to know about TESTS_ENABLED anyways. Collecting those headers seems to be enough to make it work. What's left is to port those tests with sources #40659.

@akien-mga akien-mga merged commit e7a56a2 into godotengine:master Jul 26, 2020
@akien-mga akien-mga deleted the scons-tests-self-contained branch July 26, 2020 14:57
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