-
Notifications
You must be signed in to change notification settings - Fork 36
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
Alter steps_to_run
in config file or command line
#104
Conversation
TestingI have run the I also verified that I could modify the config option |
@mark-petersen requested something like this in #80. This is my suggestion. @mark-petersen and @matthewhoffman, if you are willing to review, I would appreciate a little testing. And please let me know if you are happy with this solution or if you have alternative suggestions or changes. |
I tested this at a test case level and it works, both with flags and *.cfg file. Thanks, that is handy! One of my common workflows is to run the full nightly regression suite once, then rerun it with other compilers or debug, openmp settings, but turn off the QU240 mesh and init to save time. Is there a way to do that? I think this PR does not address omitting a test case from a suite altogether. I tried it with this functionality, ie
with a blank, but it ran that step anyway. The suite function must override the config setting. |
My suggestion would be to make version of the nightly suite with and without the test you don't want to run. I can't think of any way to efficiently specify that you don't want to run a particular step but you could open an issue with a suggested method for doing that.
Hmm, I can look into that. If |
@mark-petersen, indeed, Please also comment on #114 so I can make sure to address your needs there. |
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.
@xylar , this looks great. I added inline comments for the 2 items we discussed on the phone.
python -m compass run --no-steps restart_run | ||
|
||
Would both accomplish the same thing in this example -- skipping the | ||
``restart_run`` step of the test case. |
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.
Add note that using command line option will override the cfg version.
'steps_to_run').replace(',', ' ').split() | ||
|
||
if steps_not_to_run is not None: | ||
steps_to_run = [step for step in steps_to_run if step not in |
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.
Check if step name specified is actually a valid step name or not. (This should be done for both --steps
and --no-steps
.)
@matthewhoffman, thank you for both of your excellent suggestions. I believe they are now addressed. Please give this another look when you have a chance. |
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.
Looks good. Thanks for these improvements.
@matthewhoffman, still waiting on a re-review here. Is that something you can do soon? |
This will be at the top of the config file for every test case and will contain the default steps to run, which the user can then alter before running the test case.
These are used to explictly run (or not run) steps in the test case.
When a bad step name is given to the `steps_to_run` config option or the `--steps` or `--no-step` command-line flags, an error is raised.
I'm looking at this again now. I got myself confused because I hadnt realized it was rebased on the dev env. |
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.
Looks good @xylar. Thanks for adding the additional error-checking. I tried all the ways of asking it to do incorrect things, and it gave sensible errors each time.
Thanks! |
This merge adds the ability for users to choose which steps in a test case they want to run. They have 2 options.
First, they can alter the
steps_to_run
config option in thetest_case
section at the very top of the test case's config file:The list of
steps_to_run
can be separated by spaces, commas or both.Or they can specify which steps to run (or which steps not to run) via the
--steps
(or--no-steps
) flag tocompass run
:Both of these accomplish the same thing -- skipping the
restart_run
step of the test case.The new capability has been added to the documentation.
closes #80