-
Notifications
You must be signed in to change notification settings - Fork 283
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
enhance CMakeMake
easyblock to run ctest
command if runtest
is True
#2838
enhance CMakeMake
easyblock to run ctest
command if runtest
is True
#2838
Conversation
@Flamefire At first glance, this will cause trouble with the There may be other easyblocks with a similar problem. |
runtest = True
for CMakeMakeCMakeMake
easyblock to run ctest
command if runtest
is True
I'm very sure there cannot be a problem. I introduced
True which IMO is safe to assume to be an error.
|
Anything missing here? |
@Flamefire A battery of tests with existing easyconfigs that either use Thanks for clarifying why this shouldn't cause trouble, that helps. |
Test report by @Flamefire Overview of tested easyconfigs (in order)
Build succeeded for 24 out of 27 (17 easyconfigs in total) |
I don't have the Amber sources so they failed, rest looks good though. |
running Amber with this eb now... |
Note that this PR requires #2837 too although I expect for Amber it doesn't make a difference. |
Test report by @akesandgren Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Test report by @akesandgren Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
Going in, thanks @Flamefire! |
(created using
eb --new-pr
)When using the
CMakeMake
easyblock or a child of it likeCMakeNinja
you can useruntest = 'test'
to run the tests which is however not user friendly and forCMakeNinja
it does not work because it runsmake test
, so you needtest_cmd
Some EasyBlocks already allow
runtest = True
to enable running tests, this PR introduces that for CMakeMake and defaults thetest_cmd
in that case toctest
and (if supported by recent CMake) appends--no-tests=error
so thatruntest = True
will not silently succeed when no tests are found.Requires #2837 as setting
test_cmd
was not enough (error when concatenating theNone
ofruntest
) and it needs a default ofNone
fortest_cmd
so it is able to detect if the user set it or wants the default.Written with backward compatibility in mind.
requires:
test_cmd
withoutruntest
forConfigureMake
#2837