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.

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

Skip test if adios2 python module built without MPI. #3124

Merged
merged 14 commits into from
Sep 10, 2024

Conversation

jhale
Copy link
Member

@jhale jhale commented Apr 15, 2024

It would also be good to not build against ADIOS2 if it wasn't built against MPI...

@jhale jhale requested a review from drew-parsons April 15, 2024 08:22
python/test/unit/io/test_adios2.py Outdated Show resolved Hide resolved
cpp/CMakeLists.txt Outdated Show resolved Hide resolved
@jhale jhale added backport? Suggest PR for backporting build Build system and compiler issues testing Test system issues labels Apr 15, 2024
@jhale jhale added this to the 0.9.0 milestone Apr 15, 2024
@jhale jhale modified the milestones: 0.9.0, 0.10.0 Jul 10, 2024
python/test/unit/io/test_adios2.py Outdated Show resolved Hide resolved
@mscroggs mscroggs modified the milestones: 0.10.0, 0.9.0 Sep 3, 2024
@jorgensd
Copy link
Sponsor Member

jorgensd commented Sep 6, 2024

@drew-parsons We have decided that this test will only run if the user supplies adios2>=2.10 as this is not a requirement in the actual code base.

@jorgensd jorgensd self-requested a review September 6, 2024 11:10
Copy link
Contributor

@drew-parsons drew-parsons left a comment

Choose a reason for hiding this comment

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

Makes sense to check adios2.is_built_with_mpi if the adios2 serial build can't be used.

python/test/unit/io/test_adios2.py Outdated Show resolved Hide resolved
@jorgensd jorgensd linked an issue Sep 10, 2024 that may be closed by this pull request
@jorgensd
Copy link
Sponsor Member

@garth-wells are you happy with the simplification? I can't get it to auto-merge without "force-dismissing" your review, which seems rather rude;)

@jorgensd jorgensd added this pull request to the merge queue Sep 10, 2024
Merged via the queue into main with commit fd47df4 Sep 10, 2024
26 of 27 checks passed
@jorgensd jorgensd deleted the jhale/fix-adios2-tests branch September 10, 2024 16:37
schnellerhase pushed a commit to schnellerhase/dolfinx that referenced this pull request Sep 11, 2024
* This test only works if ADIOS2 was built with MPI.

* Require MPI ADIOS2 build during configuration.

* Reformat ruff.

* Require adios2.10 for given test

* Ruff

* Simplify

* Add back skip

---------

Co-authored-by: Garth N. Wells <gnw20@cam.ac.uk>
Co-authored-by: jorgensd <dokken92@gmail.com>
Co-authored-by: Jørgen S. Dokken <dokken@simula.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport? Suggest PR for backporting build Build system and compiler issues testing Test system issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update ADIOS2 test to 2.10
5 participants