Pass the cluster mode arguments through to pybnf in the full test runner - #641
Merged
Conversation
The run_all.py test runner worked out the command line arguments that select the cluster mode, but it never passed them to pybnf. Because of this the ssh and sf modes quietly ran every test on a single machine instead of across the cluster. This change moves the argument handling into two small functions. parse_mode turns the command line into the extra arguments and the output file name. build_pybnf_command builds the pybnf command and adds the extra arguments to the end so they reach pybnf. The run_all and run_test functions now take these values and pass the extra arguments down to each test. A new test file, tests/test_full_test_runner.py, covers both functions. One test checks that the cluster arguments really appear in the built command, so the original bug cannot come back unnoticed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The full test runner in tests/full_tests/run_all.py can run its tests on a
single machine or across a cluster. You pick the mode with a command line
argument. No argument runs locally. The argument ssh uses the automatic dask
ssh setup. The argument sf uses a manual cluster with a scheduler file.
The runner worked out the right pybnf arguments for each mode but never passed
them to pybnf. So the ssh and sf modes looked like they used the cluster but
really ran every test on a single machine.
This change fixes that. The argument handling now lives in two small functions.
parse_mode turns the command line into the extra arguments and the output file
name. build_pybnf_command builds the pybnf command and puts the extra arguments
at the end so they reach pybnf. The extra arguments are passed down to every
test.
I also added tests/test_full_test_runner.py, which tests both functions. One
test checks that the cluster arguments appear in the built command, so this bug
cannot come back without a test failing.
How to check: run
All six tests pass. Running the full suite across a cluster needs a SLURM
allocation, so I did not run that here.
This is the code half of issue 624. Pull request 640 covers the documentation
and cluster script half.