|
| 1 | +BigIO Test Output Cleanup: Design Alternatives |
| 2 | +=============================================== |
| 3 | + |
| 4 | +## Problem |
| 5 | + |
| 6 | +The 11 BigIO write-read tests across IO/Meta, IO/TIFF, and IO/ImageBase |
| 7 | +produce up to 47 GB of temporary files. On CI runners with limited disk |
| 8 | +(GitHub Actions ubuntu-22.04 has 146 GB total, ~89 GB available after |
| 9 | +build), these files can cause disk exhaustion and job cancellation. |
| 10 | + |
| 11 | +## Chosen approach: CTest fixture cleanup (CMake-side) |
| 12 | + |
| 13 | +A new CMake function `itk_add_bigio_test_cleanup()` uses CTest fixture |
| 14 | +semantics to wire up automatic file removal after successful test |
| 15 | +completion: |
| 16 | + |
| 17 | +- The main test is registered as `FIXTURES_SETUP` |
| 18 | +- A cleanup test (`cmake -E rm -f`) is registered as `FIXTURES_CLEANUP` |
| 19 | +- CTest only runs CLEANUP when the SETUP test passes |
| 20 | +- On test failure, output files are retained for debugging |
| 21 | +- Controlled by advanced option `ITK_REMOVE_BIGIO_FILES_ON_SUCCESS` |
| 22 | + (default ON; set OFF to retain files for inspection) |
| 23 | + |
| 24 | +For `.mhd` files, companion `.raw`/`.zraw` data files are also removed. |
| 25 | + |
| 26 | +## Alternative considered: in-test self-cleanup (C++ side) |
| 27 | + |
| 28 | +All 4 test source files follow the same pattern: write a large image, |
| 29 | +read it back, compare pixels, return EXIT_SUCCESS or EXIT_FAILURE. |
| 30 | +Adding `std::remove(filename.c_str())` before `return EXIT_SUCCESS` |
| 31 | +would be straightforward. |
| 32 | + |
| 33 | +### Why this was rejected |
| 34 | + |
| 35 | +1. **Multiple exit paths.** Each test has 2-4 `return EXIT_FAILURE` |
| 36 | + paths (catch blocks, pixel comparison failures). Cleanup must be |
| 37 | + placed only before success returns; an accidental placement before |
| 38 | + a failure return would destroy evidence needed for debugging. |
| 39 | + |
| 40 | +2. **Format-dependent companion files.** `.mhd` writes produce two |
| 41 | + files (`.mhd` header + `.raw` data). Each test would need |
| 42 | + format-aware cleanup logic duplicated in C++. The CMake function |
| 43 | + handles this centrally. |
| 44 | + |
| 45 | +3. **Reader lifetime concerns.** After `reader->Update()`, the |
| 46 | + reader and its IO object are still alive. On Windows, IO objects |
| 47 | + may hold file handles. Deleting files before these objects are |
| 48 | + destroyed could be platform-problematic. |
| 49 | + |
| 50 | +4. **Separation of concerns.** The tests exist to verify IO |
| 51 | + correctness, not manage disk space. Mixing infrastructure cleanup |
| 52 | + into test logic makes test code harder to review and reason about. |
| 53 | + |
| 54 | +5. **No runtime configurability.** Developers inspecting output files |
| 55 | + for debugging would need to modify and recompile the test source. |
| 56 | + The CMake option `ITK_REMOVE_BIGIO_FILES_ON_SUCCESS=OFF` provides |
| 57 | + that control without touching code. |
| 58 | + |
| 59 | +6. **No C++17 filesystem dependency.** The CMake approach uses |
| 60 | + `cmake -E rm -f`, avoiding any need for `<filesystem>` headers |
| 61 | + or platform-specific removal APIs in test code. |
| 62 | + |
| 63 | +### When in-test cleanup would be appropriate |
| 64 | + |
| 65 | +If tests were run outside CTest (e.g., invoking the test driver binary |
| 66 | +directly), CTest fixtures would not execute and files would remain. |
| 67 | +An RAII guard pattern could handle this: |
| 68 | + |
| 69 | +```cpp |
| 70 | +struct FileCleanupGuard { |
| 71 | + std::string path; |
| 72 | + bool disarm = false; |
| 73 | + ~FileCleanupGuard() { |
| 74 | + if (!disarm) std::remove(path.c_str()); |
| 75 | + } |
| 76 | +}; |
| 77 | +``` |
| 78 | +
|
| 79 | +However, ITK tests are designed to be run via CTest, and direct binary |
| 80 | +invocation is not the expected workflow for CI or automated testing. |
| 81 | +
|
| 82 | +## Serial execution requirement |
| 83 | +
|
| 84 | +All BigIO tests now set `RUN_SERIAL True` to prevent CTest from running |
| 85 | +multiple multi-gigabyte tests concurrently. Without this, `ctest -j3` |
| 86 | +could launch three 9 GB tests simultaneously, requiring ~27 GB of |
| 87 | +temporary disk space on top of regular test output. |
| 88 | +
|
| 89 | +The `RESOURCE_LOCK MEMORY_SIZE` property was already present but only |
| 90 | +serializes tests within the same resource group. `RUN_SERIAL` ensures |
| 91 | +no other test runs concurrently regardless of resource groups. |
0 commit comments