InsightSoftwareConsortium / InsightSoftwareConsortium/ITK
ENH: Audit source-level test temp-file locations not covered by the `itk_add_test` cleanup
- Dominant language
- C++
- Stars
- 1.7k
- Forks
- 748
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 64
Description
PRs #6413 and #6414 fixed output-name collisions and added `ITK_REMOVE_TEMPORARY_TEST_FILES` cleanup, but both operate on the CMake `itk_add_test()` registration and cannot see files a test opens from C++. GoogleTest drivers are the gap: they never receive `${ITK_TEST_OUTPUT_DIR}` as an argv, so each one invents its own temp-path convention. This asks for a deliberate pass over the source-level write sites and a documented convention for GTests.
Background and trigger
ITK tests write temporary artifacts to a variety of locations with no consistent policy on (a) where they go, (b) whether two tests can collide on the same path, and (c) whether they are removed when a test passes. The consequences are silent disk growth (`Testing/Temporary/` trees routinely reach multiple GB; large image read/write tests emit 1–9 GB files) and latent flakiness when tests run in parallel and reuse a filename.
The trigger was PR #6392's new GoogleTest `itkSWCMeshIOGTest.cxx`, which originally wrote bare filenames such as `swc_gtest_no_trailing_newline.swc` into whatever the current working directory happened to be. Fixed there by switching to `std::filesystem::temp_directory_path()` — see review comment https://github.com/InsightSoftwareConsortium/ITK/pull/6392#discussion_r3355874281 — but that fix exposed the toolkit-wide question.
What is already covered (verified)
- **PR #6413** — "BUG: Give racing test outputs unique names to avoid parallel collisions", merged **2026-06-08**.
- **PR #6414** — "COMP: Auto-remove transient single-owner test outputs in `itk_add_test`", merged **2026-06-11**. `ITK_REMOVE_TEMPORARY_TEST_FILES` is implemented in `CMake/ITKModuleTest.cmake`.
Between them these cover basename collisions among `itk_add_test`-registered outputs and deletion of transient outputs on success (1355 tests instrumented across 126 `test/CMakeLists.txt`; 42 deliberately-shared outputs excluded).
What is NOT covered — the remaining surface (measured on current main)
Both PRs act on the CMake test registration and are blind to paths opened from C++. Measured on `upstream/main`:
| Pattern | Count in `Modules/*/*/test/` |
|---|---|
| hard-coded `"/tmp/…"` literals in `*.cxx` | **0** |
| `std::filesystem::temp_directory_path()` | **2** files (`itkSWCMeshIOGTest.cxx`, `itkVTIImageIOGeneratedFixturesGTest.cxx`) |
| `*GTest.cxx` files toolkit-wide | **~300** |
So the hard-coded-`/tmp` problem the audit was originally scoped around does not exist, and the `temp_directory_path()` population is tiny. The real exposure is structural: ~300 GoogleTest source files, none of which receive `${ITK_TEST_OUTPUT_DIR}` the way the classic test driver does, each free to invent a convention — and many `TEST()` cases share one process and one working directory, so a same-binary basename collision is possible in addition to the cross-test parallel hazard.
Suggested approach
1. Enumerate every test that writes via a bare relative path (CWD), a hard-coded `/tmp`, or `std::filesystem::temp_directory_path()` rather than `ITK_TEST_OUTPUT_DIR`. Starting greps:
- `rg -n "temp_directory_path|\"/tmp/|ITK_TEST_OUTPUT_DIR" Modules/*/*/test`
- flag `*GTest.cxx` files that construct bare relative output paths.
2. Cross-tabulate written basenames to find reuse across tests/modules, and specifically *within* a single GTest driver.
3. Propose a standard convention/helper for GoogleTest temp output — e.g. a unique per-test subdirectory under the build's testing temp directory, exposed through a small `itk::testing` helper so GTests do not each roll their own.
4. Decide and document the cleanup-on-success policy. If adopted, a shared RAII/scope-guard that unlinks temp files when a test passes (and retains them when it fails, for post-mortem) keeps disk usage minimal without losing failure artifacts. This mirrors on the C++ side what `ITK_REMOVE_TEMPORARY_TEST_FILES` does on the CMake side.
5. Coordinate with the CTest→GoogleTest conversion effort (issue #6388, still open, 669 candidate tests) — newly converted GTests are exactly the population most likely to mishandle temp paths, so the convention should land before the bulk of the conversion.
Contributor guide
Research direction
Start with the suggested rg searches under Modules/*/*/test and inspect the two files using std::filesystem::temp_directory_path(), then review CMake/ITKModuleTest.cmake and issue #6388. Audit GoogleTest source-level writes, including basename reuse, and document or implement an agreed convention and cleanup behavior without losing failure artifacts.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cmake, cpp, git
- Domain
- build-system, testing-qa
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100