InsightSoftwareConsortium / InsightSoftwareConsortium/ITK
Consolidate duplicated test-helper macros (`TEST_INITIALIZATION_ERROR` x6, `CHECK_FOR_VALUE` x3, ...)
- Dominant language
- C++
- Stars
- 1.7k
- Forks
- 748
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 64
Description
Several test-helper macros are defined identically in multiple files. None are stringify variants — those are tracked in #6788 — so this is a separate, low-urgency cleanup: move each shared helper into one header and delete the copies.
The duplicates
| Macro | Copies | Bodies | Locations |
|---|---|---|---|
| `TEST_INITIALIZATION_ERROR(ComponentName, badComponent, goodComponent)` | 6 | identical | `Registration/Common/test/itkImageRegistrationMethodTest.cxx`, `…/itkMeanSquaresImageMetricTest.cxx`, `…/itkMultiResolutionImageRegistrationMethodTest.cxx`, `Segmentation/LevelSets/test/itkGeodesicActiveContourShapePriorLevelSetImageFilterTest.cxx`, +2 |
| `CHECK_FOR_VALUE(a, b)` | 3 | identical | `Core/Common/test/itkObjectFactoryTest.cxx`, `Video/Core/test/itkTemporalDataObjectTest.cxx`, `Video/Core/test/itkTemporalRegionTest.cxx` |
| `ALIGN_THE_ARROW_OF_TIME(seconds, micro_seconds)` | 2 | identical | `Core/Common/src/itkRealTimeInterval.cxx`, `Core/Common/src/itkRealTimeStamp.cxx` |
| `PRINT_VALUE(R, C)` | 2 | identical | `Core/Transform/test/itkBSplineDeformableTransformTest.cxx`, `Core/Transform/test/itkBSplineTransformTest.cxx` |
`ALIGN_THE_ARROW_OF_TIME` is the easiest: both copies are in the same module directory (`Core/Common/src`), so a small private header there removes the duplication with no cross-module coupling.
`TEST_INITIALIZATION_ERROR` is the most valuable at 6 copies, but it spans Registration and Segmentation tests, so the shared home is most likely `Core/TestKernel` — which makes it a public test API decision rather than a pure move. See #6789 regarding unprefixed macros in `itkTestingMacros.h`; if that discussion lands on prefixing, this one should adopt the resulting convention rather than adding another unprefixed name.
Related but NOT duplication — do not fold these in
**`ITK_CONVERT_BUFFER_IF_BLOCK`** — 2 identical copies, `IO/ImageBase/include/itkImageFileReader.hxx` and `Video/IO/include/itkVideoFileReader.hxx`. Correctly prefixed *and* `#undef`'d in both files, so it is well-behaved; consolidating it would couple Video/IO to IO/ImageBase for little gain. Leave it.
**`TRY_INDEX`** — 2 definitions with *different* parameter lists:
```c
Core/Common/test/itkFixedArrayGTest.cxx #define TRY_INDEX(T)
Core/Common/test/itkImageComputeOffsetAndIndexTest.cxx #define TRY_INDEX(dim)
```
Same name, different jobs, in the same module's test directory. That is a naming collision to resolve by renaming, not duplication to consolidate.
**`RAISE_EXCEPTION`** — 3 definitions across 2 signatures, two of them in installed public headers. Tracked in #6789 as a macro-leak bug, not here.
How the list was produced
```python
# every function-like macro defined in ITK proper, grouped by name;
# report any name defined in more than one file, flagging whether bodies match
git grep -n -E '^[[:space:]]*#[[:space:]]*define[[:space:]]+[A-Za-z_][A-Za-z0-9_]*\(' \
-- Modules ':!Modules/ThirdParty/*'
```
Ten macro names are defined in more than one file. Six are the stringify family (`_STRING` ×16, `TOSTRING` ×16 — see #6788); the four in the table above plus the three excluded cases are the remainder.
Contributor guide
Research direction
Start with the two ALIGN_THE_ARROW_OF_TIME definitions in Core/Common/src/itkRealTimeInterval.cxx and itkRealTimeStamp.cxx, then inspect nearby header conventions. For the broader cleanup, review itkTestingMacros.h and #6789 before choosing a shared home for TEST_INITIALIZATION_ERROR. Done means identical helpers have one shared definition, copies are removed, and the excluded macros remain untouched.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- testing
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100