-
Notifications
You must be signed in to change notification settings - Fork 308
add support to ConfigureMake for tweaking (first part of) test command via 'test_cmd' #2726
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
Conversation
Micket
left a comment
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 @ocaisa Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
|
@boegel Interestingly, if I use this (with but if I do the same thing in extended dry run mode, it reports the default: That's because the easyblock git patch is never actually applied, which I think would perhaps not be expected. I'll open an issue for that. |
|
Test report by @boegel Overview of tested easyconfigs (in order)
Build succeeded for 11 out of 11 (11 easyconfigs in total) |
| - default: None | ||
| """ | ||
|
|
||
| if self.cfg['runtest']: |
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.
@boegel Hm, i just realized this leads the situation that you need to specify something in the runtest part, else testing won't be considered.
Perhaps change this to
if self.cfg['runtest'] or self.cfd['test_cmd'] != DEFAULT_TEST_CMD:Otherwise you'd have to do possibly silly things:
test_cmd = './tests.sh'
runtest = ' ' # only to trigger the testsor
test_cmd = 'ctest'
runtest = '-j' # add one flag here or something to make it actually run.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.
@Micket Makes sense, please open a PR for that ASAP, maybe (just maybe) we can still include this into EasyBuild v4.5.5
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.
followed up in #2737
No description provided.