Repository navigation
test: refactor runtests - #5741
Conversation
|
test:mpich/ch3/tcp ✔️ |
|
test:mpich/ch3/tcp |
raffenet
left a comment
raffenet
left a comment
There was a problem hiding this comment.
Looks mostly good. A couple of comments.
| @@ -66,7 +69,7 @@ our $g_total_seen = 0; | |||
| our $g_num_timeout; | |||
| our $g_num_timeout_thresh = 5; | |||
|
|
|||
| $mpiexec = "@MPIEXEC@"; # Name of mpiexec program (including path, if necessary) | |||
There was a problem hiding this comment.
I'm pretty sure we've got some test scripts that rely on this old method. This is probably fixable, so just noting it here.
if test "$jenkins_configure" = "pmix" ; then
MPIEXEC="$HOME/software/openmpi-3.0.0/bin/mpiexec -oversubscribe"
elif test "$MPIEXEC" = "" ; then
MPIEXEC=$INSTALL_PREFIX/bin/mpiexec
fi
export MPIEXEC
There was a problem hiding this comment.
I can easily add and let runtests to check MPIEXEC environment variable, which should work for the test scripts.
There was a problem hiding this comment.
Actually, configure will set the variable in Makefile so make testing will pass the correct mpiexec via runtests command line -mpiexec=. So the mechanism still works. The only difference is if the user want to directly invoke runtests and when mpiexec is not in the path, then user need to manually add -mpiexec= option. Let me add the ability to pickup MPIEXEC environment, so the user have one extra option to invoke runtests.
| # Run the programs listed in the file given as the argument. | ||
| # This file describes the tests in the format | ||
| # programname number-of-processes [ key=value ... ] | ||
| # If the second value is not given, the default value is used. | ||
| # |
There was a problem hiding this comment.
I think this comment needs an update with this change.
There was a problem hiding this comment.
@raffenet Done. Added the last two commits to address your comment.
|
test:mpich/ch3/tcp |
The mpix=true option is not used and functionally the same as strict=false. Remove to simplify.
Use explicit options rather than autoconf substitutions. It is more flexible and removes the script dependency on configure.
It is unnecessary to use reference.
Move the option to g_opt hash. Remove the error check since mpiexec is always set and never will be empty. Avoid duplication of long cmd string.
Reduce code duplication.
Remember the working directory and top source directory.
Use strict to prevent many potential errors. Move the option variables into %g_opt hash to signify options.
This cleans up the code. It also provides the flexibility to separating the loading of tests from testlists from building or running the tests.
Consistently use prefix g_ to name global variables. It is not necessary to use our since it is not a module. Update g_err_count in RunTestFailed.
Only update global test counters in one of the three test reporting routines to ensure consistency.
This provide an alternative of way of customize mpiexec path in addition to using the command line option -mpiexec=path/to/mpiexec.
Pull Request Description
Preparation for #5725.
Refactor runtests and prepare for modifications to add ability to run multiple tests inside a single MPI_Init/MPI_Finalize window.
[skip warnings]
Author Checklist
Particularly focus on why, not what. Reference background, issues, test failures, xfail entries, etc.
Commits are self-contained and do not do two things at once.
Commit message is of the form:
module: short descriptionCommit message explains what's in the commit.
Whitespace checker. Warnings test. Additional tests via comments.
For non-Argonne authors, check contribution agreement.
If necessary, request an explicit comment from your companies PR approval manager.