InsightSoftwareConsortium / InsightSoftwareConsortium/ITK

Consolidate duplicated test-helper macros (`TEST_INITIALIZATION_ERROR` x6, `CHECK_FOR_VALUE` x3, ...)

Open
#6,790 0 comments 0 reactions 0 assignees View on GitHub
type:Style type:Testing
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.