Skip to content

Conversation

@eisenhauer
Copy link
Member

@vicentebolea In addition to other random cleanup (move SmallTestData.h contents into TestHelpers.h and eliminate that now-extra file), this changes the tests that don't use relatively-fixed internal fnames for their output so that those tests only cleanup files if you specify a --cleanup flag. That flag is specified in run_test.py, so files should always be cleaned up in CI (where we are in control of arguments). This is at least a step closer to safety. What do you think?

eisenhauer and others added 2 commits January 7, 2026 15:44
This commit consolidates test helper code and improves test cleanup:

1. Merge SmallTestData.h into TestHelpers.h
   - Moved SmallTestData struct and helper functions to TestHelpers.h
   - Deleted SmallTestData.h to reduce code duplication
   - Updated 43 test files to use consolidated TestHelpers.h include

2. Simplify CleanupTestFiles function
   - Removed optional engine parameter and file engine detection logic
   - Function now simply takes a path and performs cleanup unconditionally
   - File vs streaming engine determination moved to test driver layer

3. Add conditional cleanup for staging-common tests
   - Modified 5 staging-common reader tests to only cleanup with --cleanup flag
   - Added Cleanup boolean to ParseArgs.h for --cleanup command-line argument
   - Updated run_test.py.gen.in to automatically add --cleanup for file engines
   - Tests: TestCommonRead, TestCommonReadAttrs, TestCommonReadLocal,
     TestCommonReadShared, TestCommonSpanRead

4. Add cleanup to TestEngineCommon.cpp
   - Added CleanupTestFiles call for generated attributes files
   - Ensures proper synchronization via MPI_Barrier before cleanup

Benefits:
- Reduced code duplication by consolidating test utilities
- Cleaner separation of concerns (cleanup logic vs test driver logic)
- Consistent test cleanup across BP, HDF5, and staging engines
- Simplified maintenance with single source of truth for test data

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Updated testing/h5vol/TestH5VolWriteReadBPFile.cpp to use
TestHelpers.h instead of the removed SmallTestData.h.

This completes the consolidation of test utilities into TestHelpers.h.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@vicentebolea
Copy link
Collaborator

@eisenhauer this has added private keys for xrootd, is this intended?

@eisenhauer
Copy link
Member Author

It was not! Thanks. Stuff I was trying out that leaked in here.. Not sure how it got added.

@vicentebolea
Copy link
Collaborator

looks good! It would be nice if we move to c++17 and we can use std::filesystem and consolidate all of fs interactions

@eisenhauer
Copy link
Member Author

looks good! It would be nice if we move to c++17 and we can use std::filesystem and consolidate all of fs interactions

Yeah. I'm not opposed to requiring a newer C++ than 14, and the std::filesystem stuff is a pretty good incentive. Something we should chat about...

@eisenhauer eisenhauer requested a review from pnorbert January 8, 2026 01:44
@vicentebolea
Copy link
Collaborator

looks good! It would be nice if we move to c++17 and we can use std::filesystem and consolidate all of fs interactions

Yeah. I'm not opposed to requiring a newer C++ than 14, and the std::filesystem stuff is a pretty good incentive. Something we should chat about...

It would be nice, also the parallel algorithms, better constexpr (we can refactor our poor man SFINAE for Kokkos/adios views), and many other things. It might also be a good chance to move to C11 as well.

@eisenhauer eisenhauer merged commit b30565c into ornladios:master Jan 8, 2026
68 of 70 checks passed
@eisenhauer eisenhauer deleted the consolidate-test-helpers branch January 8, 2026 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants