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 runtest for non-make generators #1952

Merged
merged 3 commits into from
Mar 12, 2020
Merged

Fix runtest for non-make generators #1952

merged 3 commits into from
Mar 12, 2020

Conversation

ZedThree
Copy link
Member

@ZedThree ZedThree commented Mar 6, 2020

Using shell("make") in runtest files doesn't work when using CMake and Ninja or on Windows. Previous fix was to add build_and_log("test name") which does nothing on Windows. This goes further and does nothing if using CMake, as the executable should've already been built.

Fixes most remaining uses of shell("make"), except for MMS/advection, as this is a bit different from everything else. Probably needs changing to avoid using symlinks and building in parent directory.

MMS/advection is a bit weird, using symlinks and nested
directories. Probably a better way of doing that
@ZedThree ZedThree added this to the BOUT-5.0 milestone Mar 6, 2020
@dschwoerer
Copy link
Contributor

Note that with make check the additional invocation of make shouldn't do anything on autotools either.
This is purely if you didn't build the tests (e.g. because they are disabled) and e.g. want to fix it.

This is of course in contrast to cmake, where it seems the all target should not only build the library, but basically anything that could be build, and later stages do not support making (is this the case?).

If we make the same split in cmake, to only build the library (and maybe some tests) then the make would still be needed, to ensure that this test is up-to-date with the current library.

@ZedThree
Copy link
Member Author

ZedThree commented Mar 6, 2020

With the makefile buildsystem, make gets called but doesn't do anything if the executable is already up-to-date. By default on linux, CMake uses make as well, so it generates a makefile in the test directory, and shell("make") works, even if it's just a no-op.

The issue is when using CMake with Ninja, it doesn't create any makefiles, so shell("make") fails. This skips calling make entirely. Another solution would be to still call make but catch RunetimeError and check the return code was 2 (no makefile found).

In next at least, CMake follows the makefile buildsystem:

  • no target and all: build just the library
  • build-check: build and tests
  • check: build and run tests

Additionally, CMake knows how to build the individual tests as top-level targets:

  • test_yupdown: build test_yupdown

These can be run in a generator-agnostic way with cmake --build <build-dir> --target <target>.
This will trigger a rebuild of everything needed: that is, if you touch a header, test_yupdown will rebuild both the library and the test executable.
You can then rerun just that test from the build directory with ctest -R test_yupdown (actually takes a regexp).

So, it would be possible to make sure the executable is up-to-date for CMake too, but it would need to know both the top-level build-directory and the target name, and that seems trickier to maintain.

@ZedThree ZedThree merged commit 4b74f70 into next Mar 12, 2020
@ZedThree ZedThree deleted the fix-runtest-for-non-make branch March 12, 2020 14:41
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 this pull request may close these issues.

None yet

3 participants