Skip to content

COMP: Enable ITKGoogleTest whenever BUILD_TESTING is on - #6778

Closed
hjmjohnson wants to merge 1 commit into
InsightSoftwareConsortium:mainfrom
hjmjohnson:comp-googletest-required-for-testing
Closed

COMP: Enable ITKGoogleTest whenever BUILD_TESTING is on#6778
hjmjohnson wants to merge 1 commit into
InsightSoftwareConsortium:mainfrom
hjmjohnson:comp-googletest-required-for-testing

Conversation

@hjmjohnson

Copy link
Copy Markdown
Member

BUILD_TESTING=ON now mandates ITKGoogleTest, instead of relying on each module to list it in TEST_DEPENDS. Any module's test kit may call CreateGoogleTestDriver, and the set of modules with no GoogleTest tests keeps shrinking, so tracking the dependency per module is the wrong lever.

Fixes a configure failure on current main:

$ cmake -DITK_BUILD_DEFAULT_MODULES=OFF -DModule_ITKIONRRD=ON -DBUILD_TESTING=ON ...
CMake Error at CMake/ITKModuleTest.cmake:320 (target_link_libraries):
  Target "ITKIONRRDGTestDriver" links to:
    GTest::gtest
  but the target was not found.
Call Stack (most recent call first):
  Modules/IO/NRRD/test/CMakeLists.txt:425 (creategoogletestdriver)

Found while auditing #6776 / #6777; independent of both.

Scope: seven modules have the same latent omission

72 modules call CreateGoogleTestDriver; these seven do not name ITKGoogleTest in their itk-module.cmake, so any selection that enables one of them without separately pulling in ITKGoogleTest hits the error above:

Modules/Filtering/CurvatureFlow
Modules/Filtering/ImageCompare
Modules/Filtering/DisplacementField
Modules/Segmentation/RegionGrowing
Modules/Nonunit/Review
Modules/IO/NRRD
Modules/IO/IOTransformDCMTK

Adding seven TEST_DEPENDS entries would fix today's instances and leave the next one to be discovered the same way. This PR removes the class instead. The seven entries are left alone; they are harmless where present.

Verification
configuration before after ITKGoogleTest enabled after
minimal + Module_ITKIONRRD=ON + BUILD_TESTING=ON configure fails configures yes
minimal + Module_ITKIONRRD=ON + BUILD_TESTING=OFF configures configures no
default modules + BUILD_TESTING=ON configures configures yes

Enablement checked by the presence of lib/cmake/ITK-6.0/Modules/ITKGoogleTest.cmake in each build tree. The guard is BUILD_TESTING AND NOT DISABLE_MODULE_TESTS, so a testing build that adds no module test directories does not pull the module in.

Any module's test kit may create a GoogleTest driver, but ITKGoogleTest was
enabled only as a TEST_DEPENDS of individual modules. Selections that omitted
it failed at configure time with GTest::gtest not found, for instance
-DITK_BUILD_DEFAULT_MODULES=OFF -DModule_ITKIONRRD=ON -DBUILD_TESTING=ON.

Seven modules that call CreateGoogleTestDriver do not list ITKGoogleTest in
their itk-module.cmake, and the set of modules without GoogleTest tests keeps
shrinking, so require the module for testing builds rather than tracking the
dependency per module.
@github-actions github-actions Bot added type:Compiler Compiler support or related warnings type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots labels Aug 21, 2026
@blowekamp

Copy link
Copy Markdown
Member

The premise of the first sentence is wrong. Each module needs to have it in there test depends to ensure cmake interfaces are correct.

@hjmjohnson

Copy link
Copy Markdown
Member Author

Closing. The premise is wrong: TEST_DEPENDS ITKGoogleTest is what makes a module's CMake interface correct, so blanket enablement is not a substitute for the per-module declaration.

The seven modules that call CreateGoogleTestDriver without declaring ITKGoogleTest (CurvatureFlow, ImageCompare, DisplacementField, RegionGrowing, Review, IONRRD, IOTransformDCMTK) should have the declaration added instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:Compiler Compiler support or related warnings type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants