Add support for a prerequisite tag in COMPASS #559
Conversation
This merge makes a dict of test cases, each with a dict of info: core, configuration, resolution, test, procs and treads. This will be useful later on in both determining whether prerequisite tasks are present and in determining how to run many tests in parallel
Make sure they are also in the test suite
This prevents variables being in global scope that we may not want.
|
|
||
|
|
||
| def process_test_clean(test_tag, work_dir, suite_script): # {{{ | ||
| def process_test_clean(test_tag, work_dir): # {{{ |
There was a problem hiding this comment.
This is just clean-up of an unused argument
| if __name__ == "__main__": | ||
| def main (): # {{{ |
There was a problem hiding this comment.
Making main() a function to avoid having module-level variables (sometimes prevents bugs).
| args.work_dir + '/manage_regression_suite.py.out'] | ||
| print('\nCase setup output:') | ||
| print(subprocess.check_output(cmd)) | ||
| print(subprocess.check_output(cmd).decode('utf-8')) |
There was a problem hiding this comment.
This looks like garbage in python 3 without the decode
| testcases = get_test_case_procs(suite_root) | ||
| setup_suite(suite_root, args.work_dir, args.model_runtime, | ||
| args.config_file, args.baseline_dir, args.verbose) | ||
| summarize_suite(suite_root) | ||
| summarize_suite(testcases) |
There was a problem hiding this comment.
The changes require doing the parsing that was previously in summarize_suite() before doing setup_suite() so we can error out if the prerequisites are not present. The info from this parsing is stored in nested dicts that will likely be useful for future efforts for parallelization.
xylar
requested review from
mark-petersen,
matthewhoffman,
pwolfram and
vanroekel
|
@mark-petersen, @matthewhoffman, and @pwolfram, this is work that feeds into work that @jamilgafur is doing, supervised by @vanroekel and me, toward paralleling regression test suites. For that work, it will be important to know which test cases must run before which others, and which can run in parallel. |
|
@matthewhoffman, I'm mostly wanting to make sure you are okay with these changes, and to have you run a quick test of your regression suite(s) with these changes to make sure nothing breaks. |
|
@mark-petersen, the easiest way to test this would be to copy this version of |
|
@jamilgafur and @vanroekel, I would definitely appreciate your feedback, too, particularly since this work is mainly supposed to help with the parallelization that @jamilgafur is working on. |
|
@xylar, just to be clear here-- you need this tag to build a dependency graph so you can execute in parallel, right? Basically, something like if there is a prerequisite and it isn't fulfilled, move case to bottom of queue and if only one item in queue report error. |
|
@pwolfram, yep, that's the idea. Since we don't typically run in parallel, I'm just checking to see that any prerequisites are already in the list of test cases to run when a new test case gets added, because otherwise serial execution gets broken. In parallel, one approach would be what you suggest. |
|
@xylar Just a FYI that @jamilgafur has merged this to his local branch for testing the parallel nightly regression. After that test, I will approve. If you'd prefer me to approve sooner, let me know and I can look more closely at the code itself to approve. |
|
@vanroekel, no rush. My guess is that this won't go in particularly soon, so @jamilgafur should test as much as needed. |
mark-petersen
left a comment
mark-petersen
left a comment
There was a problem hiding this comment.
Merged #559 and #560 locally, set up and ran nightly regression suite successfully. Removed init set from nightly regression and got the expected error (#560 (review)).
This can be merged into develop any time.
|
@vanroekel, what is the order of merging this vs #593? I had thought this would get approved and merged before too much work went into #593. |
vanroekel
left a comment
vanroekel
left a comment
There was a problem hiding this comment.
this has worked great in testing with @jamilgafur.
|
Great, thanks @vanroekel! |
5 participants
Several ocean test cases depend on other ocean test cases. Currently, this is used to create a mesh and/or initial condition that are then reused by one or more other tests.
Currently, the user simply must make sure the prerequisites are present when setting up a test case. However, this merge makes sure that prerequisites are always present a regression suite and that they run before the dependent test case. The main motivation for this change is future support for running multiple test cases from the nightly test suite in parallel.
This is done by parsing a tag like the following example from a
config_driver.xmlfor the dependent test case:These tags will be added to individual test cases in separate PRs to the individual
<core>/developbranches.