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

1896 Use only unique filenames in unit tests #1897

Merged

nlslatt merged 1 commit into develop from 1896-use-unique-filenames-in-tests

Aug 15, 2022

Collaborator

nlslatt commented Aug 3, 2022 •

edited

Loading

This PR attempts to stop using the same disk files from multiple unit tests, including runs with different numbers of MPI ranks, so that multiple tests can be run concurrently (as happens in Gitlab CI testing) without file system conflicts. This PR does not yet handle parameterizations or filenames/directories used for LB data files in addAdditionalArgs() in collection/test_lb.extended.cc. Does anyone have ideas on how to tackle those?

Closes #1896

nlslatt requested review from PhilMiller, stmcgovern, lifflander, cz4rs, nmm0, thearusable, JacobDomagala and jstrzebonski

August 3, 2022 22:07

github-actions bot commented Aug 3, 2022 •

edited

Loading

Pipelines results

PR tests (gcc-5, ubuntu, mpich)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (clang-3.9, ubuntu, mpich)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (clang-5.0, ubuntu, mpich)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (gcc-9, ubuntu, mpich, zoltan)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (gcc-10, ubuntu, openmpi, no LB)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (gcc-6, ubuntu, mpich)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (nvidia cuda 11.0, ubuntu, mpich)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (nvidia cuda 10.1, ubuntu, mpich)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (clang-9, ubuntu, mpich)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (gcc-7, ubuntu, mpich, trace runtime, LB)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (intel icpx, ubuntu, mpich)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (clang-11, ubuntu, mpich)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (clang-13, alpine, mpich)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (clang-13, ubuntu, mpich)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (clang-12, ubuntu, mpich)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (clang-14, ubuntu, mpich)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (gcc-11, ubuntu, mpich)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (gcc-12, ubuntu, mpich)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (intel icpc, ubuntu, mpich)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (gcc-8, ubuntu, mpich, address sanitizer)

Build for edab5ac

Compilation - successful

Testing - passed

PR tests (clang-10, ubuntu, mpich)

Build for edab5ac

Compilation - successful

Testing - passed

codecov bot commented Aug 3, 2022 •

edited

Loading

Codecov Report

Merging #1897 (edab5ac) into develop (e89215e) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop    #1897      +/-   ##
===========================================
+ Coverage    84.34%   84.40%   +0.05%     
===========================================
  Files          760      761       +1     
  Lines        26833    26866      +33     
===========================================
+ Hits         22633    22675      +42     
+ Misses        4200     4191       -9

Impacted Files	Coverage Δ
tests/unit/test_parallel_harness.h	`93.54% <ø> (+0.21%)`	⬆️
tests/unit/collection/test_checkpoint.extended.cc	`100.00% <100.00%> (ø)`
tests/unit/collection/test_lb.extended.cc	`92.04% <100.00%> (+0.32%)`	⬆️
tests/unit/runtime/test_initialization.cc	`100.00% <100.00%> (ø)`
tests/unit/test_helpers.h	`100.00% <100.00%> (ø)`
tests/unit/trace/test_trace_spec_reader.cc	`100.00% <100.00%> (ø)`
src/vt/vrt/collection/manager.impl.h	`96.48% <0.00%> (+0.98%)`	⬆️
src/vt/phase/phase_manager.cc	`92.56% <0.00%> (+1.35%)`	⬆️

cz4rs reviewed

View reviewed changes

tests/unit/collection/test_checkpoint.extended.cc Outdated Show resolved Hide resolved

nlslatt force-pushed the 1896-use-unique-filenames-in-tests branch 2 times, most recently from 3f454be to 4b0ecc8 Compare

August 5, 2022 22:47

nlslatt requested a review from cz4rs

August 5, 2022 22:47

PhilMiller reviewed

View reviewed changes

tests/unit/runtime/test_initialization.cc Outdated Show resolved Hide resolved

cz4rs reviewed

View reviewed changes

tests/unit/test_helpers.h Outdated Show resolved Hide resolved

cz4rs reviewed

View reviewed changes

tests/unit/test_helpers.h Outdated Show resolved Hide resolved

cz4rs reviewed

View reviewed changes

tests/unit/test_helpers.h Outdated Show resolved Hide resolved

nlslatt force-pushed the 1896-use-unique-filenames-in-tests branch from 4b0ecc8 to cc900ca Compare

August 12, 2022 18:09

nlslatt requested review from PhilMiller and cz4rs

August 12, 2022 18:12

cz4rs approved these changes

View reviewed changes

Contributor

cz4rs left a comment

Looks good to me apart from strdup issue that Phil has mentioned.

nlslatt force-pushed the 1896-use-unique-filenames-in-tests branch 2 times, most recently from ac78ff8 to 51a14e5 Compare

August 15, 2022 18:06


          #1896: tests: use unique filenames to allow parallel testing

edab5ac

nlslatt force-pushed the 1896-use-unique-filenames-in-tests branch from 51a14e5 to edab5ac Compare

August 15, 2022 18:07

PhilMiller approved these changes

View reviewed changes

nlslatt merged commit 1dbfb5e into develop

nmm0 mentioned this pull request

Meeting Agenda [do not close] #925

Open

nlslatt mentioned this pull request

TestCheckpoint.test_checkpoint_1 failures in Gitlab CI #1894

Closed

nlslatt mentioned this pull request

TestCheckpoint.test_checkpoint_in_place_2 failures in Gitlab CI #1895

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

cz4rs cz4rs approved these changes

PhilMiller PhilMiller approved these changes

stmcgovern Awaiting requested review from stmcgovern

lifflander Awaiting requested review from lifflander

nmm0 Awaiting requested review from nmm0

thearusable Awaiting requested review from thearusable

JacobDomagala Awaiting requested review from JacobDomagala

jstrzebonski Awaiting requested review from jstrzebonski

Labels

None yet