COMP: Auto-remove transient single-owner test outputs in itk_add_test

[hjmjohnson]

Add an optional ITK_REMOVE_TEMPORARY_TEST_FILES <file>... argument to itk_add_test() so a test can list the transient ${ITK_TEST_OUTPUT_DIR}/${TEMP} outputs it owns; each such test gets a FIXTURES_CLEANUP companion that deletes them. Gated by the ITK_REMOVE_TEMPORARY_TEST_FILES option (ON by default); configure -DITK_REMOVE_TEMPORARY_TEST_FILES=OFF to retain outputs for debugging.

1355 tests instrumented across 126 test/CMakeLists.txt. Supersedes the original 12-file scope after a reviewer asked why only a subset was cleaned.

Scope: how the test set was selected

A deterministic parser over all Modules/**/test/CMakeLists.txt groups every writable ${ITK_TEST_OUTPUT_DIR}/${TEMP} output by the tests referencing it:

• 1355 tests have outputs owned by exactly one test → safe to clean, wired here.
• 42 outputs are referenced by more than one test (one writes, another consumes) → excluded so the file persists for the consumer (BSplineWarping moving image, KdTree .dot→*Plot, IOScanco read-written .isq/.aim, etc.). No DEPENDS-based consumer chains exist, so no shared file is missed.

Commented-out itk_add_test blocks and tests whose NAME is an unresolved ${var} are skipped.

Default behavior (default-ON is intentional)

Removing transient outputs is the intended default. Per CMake, a FIXTURES_CLEANUP test runs unconditionally after its fixture — pass, fail, or skip — so outputs are removed even on failure. A developer who needs to inspect outputs opts out with -DITK_REMOVE_TEMPORARY_TEST_FILES=OFF. The option documentation in CMake/ITKModuleTest.cmake states this explicitly.

Helper details

• The keyword is the only one parsed by itk_add_test, so it must appear last; this is documented in the helper, and CMake/stubs/itk_add_test.cmake declares it so gersemi formats call sites correctly.
• The cleanup command uses cmake -E rm -rf, so directory outputs (DICOM series, FileTools dirs) are removed as well as plain files.
• Detached-header outputs auto-remove their companion raw data: .mhd → .raw/.zraw, .nhdr → .raw/.raw.gz.

Local validation

• Rebased on current upstream/main; pre-commit run --all-files clean.
• Rebuilt affected test drivers; all 1068 _cleanup tests in the default build config pass (0 failures).
• Targeted run of the 3 directory-output tests + a .nhdr test + a comma-list Video test: pass, with the actual outputs (dirs, files, companions) confirmed removed.

[dzenanz]

Rebased on top of main, to keep this PR one commit only.

[dzenanz]

Why were these test outputs selected for cleanup, and not the other ~3000?

[hjmjohnson]

Why were these test outputs selected for cleanup, and not the other ~3000?

They are exactly the set of tests touched by the merged race fix #6413. #6413 found 13 write-write output collisions among 2719 tests and renamed one side of each colliding pair. This PR wires itk_add_file_test_cleanup() for precisely those same tests, in the same CMake scopes #6413 had already edited. The scope is "the tests we just de-raced," not "the best ~13 of 3000."

The other ~3000 were originally deliberately deferred: the mechanism (itk_add_file_test_cleanup, gated by the default-ON ITK_REMOVE_TEST_FILES_ON_SUCCESS) will be fully general across all tests.

I will apply more broadly soon, but originall thought it would be too much for single PR. This initial PR proves the mechanism on the recently-touched test cases.

** I'll make an update to this PR that includes all the cases.

[hjmjohnson]

@dzenanz Re: "why these and not the other ~3000?" — addressed by instrumenting all of them.

The PR now wires itk_add_file_test_cleanup() into every test whose ${ITK_TEST_OUTPUT_DIR}/${TEMP} outputs are owned by a single test — 1317 tests across 126 test/CMakeLists.txt. The only outputs left uncleaned are the 42 intentionally-shared files (one test writes, another consumes): KdTree .dot→*Plot, BSplineWarping moving image, IOScanco read-written .isq/.aim, etc. — the same exclusion set as #6413. Cleaning those would break their consumers, so they persist by design.

Selection was deterministic (a parser over all 2692 itk_add_test invocations grouping outputs by writer), not a hand-picked subset. Validated locally: full configure clean, build 0 errors, instrumented tests + their *_cleanup companions pass, cleaned outputs removed, shared outputs survive for consumers.

[blowekamp]

Sorry I got to looking this over late. Consider adding a "CLEANUP" argument to add these that takes a list of files to delete on success.

[hjmjohnson]

Done — adopted in 735262c. Added a CLEANUP <file>... multi-value argument to itk_add_test() (it parses CLEANUP out, passes the remainder to ExternalData_Add_Test unchanged, and wires the FIXTURES_CLEANUP companion internally). All 1355 sites are converted from the separate itk_add_file_test_cleanup() companion calls to inline CLEANUP args, so each test's cleanup list now lives with the itk_add_test that produces the files.

Verified behavior-preserving: the generated CTest *_cleanup tests (names, rm -f file lists, and FIXTURES_SETUP/FIXTURES_CLEANUP associations) are byte-identical to the prior companion-call version (diff of the configured CTestTestfiles is empty apart from the build-dir path). gersemi + pre-commit clean.

[dzenanz]

I like this approach better. Some in-line suggestions.

[blowekamp]

Thank you for stream linking into the one function. It looks good. updating the stub file will get gersemi to format the call correctly.

[hjmjohnson]

Pushed c676e1d7 (rebased on current upstream/main). Addresses all review threads and fixes the red CI:

• CI fix: the 3 failing _cleanup tests named directories (itkFileToolsTest/mytestdir, itkDOMTest8/xmldom-test8, GDCM series dir); cmake -E rm -f can't remove directories → changed to rm -rf.
• Option and keyword renamed to ITK_REMOVE_TEMPORARY_TEST_FILES; gersemi stub updated; helper documents the keyword must be last (@blowekamp).
• .nhdr companions (.raw/.raw.gz) now auto-removed like .mhd (@dzenanz).
• Comma-joined cleanup lists → space-separated multi-values (they were silently no-ops); comma support intentionally not added (@blowekamp).

Local validation

Rebuilt affected drivers; all 1068 _cleanup tests in the default build config pass (0 failures). Targeted run of the 3 directory tests + a .nhdr + a comma-list Video test pass, with outputs (dirs/files/companions) confirmed removed. pre-commit run --all-files clean.

[hjmjohnson]

Re-triggering ITK.macOS.Python: the prior run (buildId 15781) hit the agent wall-clock limit (~1h46m) during the build step — all post-job tasks were abandoned and CDash recorded 0 compile errors / 0 test failures. This PR only renames CMake test-cleanup keywords and adds no compile units, so the timeout is an infrastructure flake on the slowest pipeline, not a regression.

[hjmjohnson]

/azp run ITK.macOS.Python

[dzenanz]

Thank you for addressing my feedback (and co-authorship).

[dzenanz]

Some .mhds are accompanied by both .raw and .zraw. Is this intentionally done because compression can be applied?

[hjmjohnson]

Some .mhds are accompanied by both .raw and .zraw. Is this intentionally done because compression can be applied?

Why both .raw and .zraw are there: it was me being overly optimistic/defensive, not because both files exist. When I expanded each .mhd entry, I added both companions without checking which one each test actually writes, relying on the fact that file(GLOB) simply no-ops on the non-existent one. So nothing extra is deleted (a .zraw that isn't generated can't match), but the source lists a companion the code never generates. This was easier to implement.

I'm now doing a full build/test/experiment to validate what is actually written to disk, limiting the patterns to only what is actually written, not what could be written under the same input name.

[hjmjohnson]

Good catch — that double-listing was me being defensive, not evidence-based. Fixed in b2a774f.

I built ITK with full ExternalData + BrainWeb + examples and ran the suite with cleanup OFF to record exactly which data file each detached-header test writes. No test writes both: each .mhd produces either .raw (uncompressed, the default) or .zraw (compression on) — never both — decided at write time by the writer, not an environment variable.

So each shared stem is now a single brace entry listing only the extensions actually produced:

• stem.{mhd,raw} — 113 (uncompressed)
• stem.{mhd,zraw} — 9, all Filtering/DistanceMap (Danielsson/Maurer signed-distance tests enable compression)
• stem.{nhdr,raw} — 15
• stem.{nhdr,raw,raw.gz} — 1 (testNrrd, which genuinely writes both)

itkRemoveTestFiles.cmake expands the {...} set into globs and removes files only (never directories). Verified locally: 1132/1132 CLEANUP fixtures pass and the DistanceMap .zraw is actually removed.