Skip to content
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.

Sign up for GitHub

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

Jump to bottom

We are incorrectly running tst_parallel_zlib on 1 processor - it should only ever run on 4 #2323

Merged
merged 4 commits into from
May 2, 2022
Merged

We are incorrectly running tst_parallel_zlib on 1 processor - it should only ever run on 4 #2323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2715513
added sequential test for enddef issue
edwardhartnett Apr 28, 2022
b4d2c7d
no longer run tst_parallel_zlib with 1 processor in testing
edwardhartnett Apr 30, 2022
6945f53
undoing unintended change
edwardhartnett Apr 30, 2022
9674385
undoing unintended change
edwardhartnett Apr 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion nc_test4/run_par_test.sh.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ echo "Parallel I/O test for Collective I/O, contributed by HDF Group."
if test "@HAS_PAR_FILTERS@" = "yes"; then
echo
echo "Parallel I/O test with zlib."
@MPIEXEC@ -n 1 ./tst_parallel_zlib
@MPIEXEC@ -n 4 ./tst_parallel_zlib

echo
Expand Down
4 changes: 4 additions & 0 deletions nc_test4/tst_parallel_zlib.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ main(int argc, char **argv)
/*printf("mpi_name: %s size: %d rank: %d\n", mpi_name,
mpi_size, mpi_rank);*/

/* This program must have exactly 4 processors. */
if (mpi_size != 4)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-running checks, I suspect (without looking) that the 'failure' was a timeout from Github Actions. I may also add a printf() statement here to avoid a issue in the future where this is failing with mpi_size != 4 but that cause is not immediately obvious. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI failed because of the DAP remote3 test in the CMake build. If you merge my other open PR, which turns off DAP remote testing in the CMake build, then rerun these tests, they will pass.

A printf is not needed when the ERR macro is used. It prints an error message that gives the line number, and then I go to the source and find the exact line that failed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough for us and other devs, but we get a lot of support questions from end users who send in the error message for us to dig in to. A decent subset of these are people who are running tests/files outside of the normal 'make check/make test' workflow. Encouraging that folk are curious and poking around. Anyways, adding the explicit message (which I've got in a separate branch I'll add, nothing needs change here) should at least hopefully circumvent any of those questions. Thanks!

ERR;

#ifdef USE_MPE
MPE_Init_log();
s_init = MPE_Log_get_event_number();
Expand Down