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

With TUDAT_DISABLE_TESTS = ON, all tests are still built #638

Closed
rodyo opened this issue Aug 21, 2020 · 1 comment · May be fixed by #639
Closed

With TUDAT_DISABLE_TESTS = ON, all tests are still built #638

rodyo opened this issue Aug 21, 2020 · 1 comment · May be fixed by #639

Comments

@rodyo
Copy link

rodyo commented Aug 21, 2020

I noticed that this flag has no effect in practice; regardless of its value, all tests are built.

Since (re)building the tests takes quite a while, I did a bit of digging. Turns out, the definitions of the unit tests are a bit...messy.

All unit tests (defined in tudat/Tudat/**/CMakeLists.txt) follow this pattern:

add_executable(<target_name> <list_of_sourcefiles>)
setup_custom_test_program(<target_name> <custom_build_path>)
target_link_libraries(<target_name> <libriaries_to_be_linked>)

So, although the setup_custom_test_program macro in the top-level CMakeLists.txt implements the TUDAT_DISABLE_TESTS flag correctly, the unit tests in all subdirectories are defined such that the flag is ignored.

The pattern above has a few drawbacks:

  • It doesn't lend itself to exclude the executables using the TUDAT_DISABLE_TESTS flag; all tests everywhere have to be refactored
  • <target_name> is repeated 3 times
  • It's not very intuitive; it's not abundantly clear what setup_custom_test_program does without navigating to the macro (on first glance: ...isn't add_executable and target_link_libraries enough?)
  • Upon closer inspection, the macro completely ignores <custom_build_path>; all tests that use it thus have a useless, cluttery string

TL;DR: this is all in need of a cleanup & refactor.

I went ahead and did just that - see PR. My proposal tackles the issues above, but of course, I may be unaware of some underlying reasons why it was done the way it was...

@DominicDirkx
Copy link
Member

NOTE: This Tudat version is no longer supported. See https://docs.tudat.space/en/stable/ and https://github.com/tudat-team/tudat-bundle for the new version

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

Successfully merging a pull request may close this issue.

2 participants