Skip to content

ENH: Convert 12 ITKCommon CTests to GoogleTest framework#5874

Open
hjmjohnson wants to merge 11 commits intoInsightSoftwareConsortium:mainfrom
hjmjohnson:simplified-conversion-to-gtest
Open

ENH: Convert 12 ITKCommon CTests to GoogleTest framework#5874
hjmjohnson wants to merge 11 commits intoInsightSoftwareConsortium:mainfrom
hjmjohnson:simplified-conversion-to-gtest

Conversation

@hjmjohnson
Copy link
Member

Summary

Converts no-argument CTests in Modules/Core/Common/test/ to GoogleTest,
each in a separate commit. Adds the ITK_GTEST_SET_GET_VALUE macro to
itkGTest.h for use in subsequent conversions.

Converted tests

Old CTest New GTest file Key assertions added
itkVersionTest itkVersionGTest EXPECT_EQ for major/minor/patch versions
itkDataTypeTest itkDataTypeGTest EXPECT_EQ per vector element
itkEventObjectTest itkEventObjectGTest EXPECT_TRUE/FALSE for derivation/matching, EXPECT_STREQ for name
itkModifiedTimeTest itkModifiedTimeGTest EXPECT_GT/GE for time ordering
itkByteSwapTest itkByteSwapGTest EXPECT_EQ per type after double-swap round-trip
itkIntTypesTest itkIntTypesGTest EXPECT_TRUE for size/signedness per type group
itkMathRoundTest + itkMathRoundTest2 + itkMathRoundProfileTest1 itkMathRoundGTest EXPECT_TRUE on templated round/floor/ceil correctness
itkMathCastWithRangeCheckTest itkMathCastWithRangeCheckGTest EXPECT_THROW for overflow, EXPECT_TRUE per target type
itkTimeStampTest itkTimeStampGTest EXPECT_LE/EXPECT_EQ for thread-safe monotonic timestamps
itkBoundingBoxTest itkBoundingBoxGTest EXPECT_EQ/EXPECT_NEAR for bounds, center, diagonal, corners
itkBoundaryConditionTest itkBoundaryConditionGTest EXPECT_EQ for all 15 boundary neighborhood pixel values

Approach

  • All diagnostic std::cout output preserved
  • Expected values determined by running original tests against the build
  • Tests split into logical sub-cases within each file
  • Old Test.cxx files removed; entries removed from ITKCommon1Tests list and itk_add_test() calls
  • New GTest.cxx files added to ITKCommonGTests list

Test plan

  • Each GTest file builds without errors
  • Each GTest runs and passes (--gtest_filter=<TestSuite>*)
  • CMakeLists.txt updated correctly (removed from driver list, added to GTest list)
  • Full CI green

🤖 Generated with Claude Code

@github-actions github-actions bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Enhancement Improvement of existing methods or implementation type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Core Issues affecting the Core module labels Mar 6, 2026
@hjmjohnson
Copy link
Member Author

FYI: I'm trying to keep up with students use of AI coding tools to better understand how to use them effectively. I'm just starting to use claude-code from the command line, and my first attempt at making this use full was to try the following prompt:

Pre-conditions

  • purchase claude-code subscription
  • install homebrew claude-code
  • copy AGENTS.md to CLAUDE.md
  • get to the following prompt
Find git commits that convert a ctest to a google test with removal of files with suffix Test.cxx and addition of file GTest.cxx in the same commit.  Report all the commit hashes that meet this requirement. 

Find ctests  in the Modules/Core/Common/test directory that do not take any command line arguments and convert them to 
 GTest (GoogleTest), one-by-one, with a separate git commit for each test converted.   The converted tests should have a filename suffix of GTest.cxx.   Keep the original code structure as much as possible, do not split the ctest into multiple google tests, even if that would be best practice.  preserve all diagnostic console output, even if it does not produce testable cases.  The diagnostic print statements are required to exercise the code even without testing the values.

Use git commits at 
35ff93ef67384eb6e1403f1f30deebc418c7c033
7d9584d3d4c2310b2077152a15750df417e30203
532b323af33ca59fd044d0bcdf83b6dbb2497283                                                                                                                                                                                                                                                     
 5d3c74e76fc430f2aebbb6305fe44a137724c6b4                                                                                                                                                                                                                        
 1fa3910d47d05ce022650aa862e396761fe8863f
 13e1b701fc2af59e5cc35e863ff093940d4e3700                                                                                                                                                                                                                        
 9b0c475d0183c4b06e6265904deff3bc32f5d6f9                                                                                                                                                                                                                        
 bf21063b980b5f0f7084669fa32f5ba2b5ab04f7                                                                                                                                                                                                                        
 4e424b45a2324df40d8af94b99efaa4d0b431958                                                                                                                                                                                                                        
 d3fd4a19b58a912792b6f2c109080d90de74476a                                                                                                                                                                                                                        
 97aeb448be5c8fd7a8d2fe3584204b7f6874b725                                                                                                                                                                                                                        
 1325efa2c35a10b542fec7d2634e84e3d8472a11                                                                                                                                                                                                                        
  532b323af33ca59fd044d0bcdf83b6dbb2497283                                                                                                                                                                                                                       
  d5c348e7ef75b4a6d5e3ab7b528f11618221a754 
as examples of how to accomplish the conversions.    

I would like you to augment the GoogleTest versions and convert the console-printed statements into test values. You can run the tests from the build directory to determine the correct values.  Additionally, some comments that attribute the purpose or authorship of the tests should be preserved.  Do this for all converted tests since upstream/main.              

Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will re-visit this PR once it is marked "ready for review".

@thewtex
Copy link
Member

thewtex commented Mar 6, 2026

copy AGENTS.md to CLAUDE.md

This is not necessary -- Claude Code will read AGENTS.md.

@hjmjohnson
Copy link
Member Author

hjmjohnson commented Mar 6, 2026

copy AGENTS.md to CLAUDE.md

This is not necessary -- Claude Code will read AGENTS.md.

I thought so too, but according to google:

Claude Code does not natively support the AGENTS.md standard, preferring CLAUDE.md for project-specific instructions. While other AI tools (Cursor, Copilot) use AGENTS.md, Claude Code ignores this file. To work around this, you can symlink CLAUDE.md to AGENTS.md or instruct CLAUDE.md to read AGENTS.md. 

Workarounds to use AGENTS.md in Claude Code:
Symlink: Create a symbolic link so CLAUDE.md points to AGENTS.md.
Reference: Within CLAUDE.md, explicitly tell Claude to read the AGENTS.md file.
Content Copying: Manually duplicate instructions from AGENTS.md into CLAUDE.md. 

As of early 2026, the lack of native support for AGENTS.md is a known friction point for users switching between different AI tools, as CLAUDE.md remains necessary for specialized instructions.

Of course google could be wrong.

hjmjohnson and others added 11 commits March 6, 2026 17:22
Replace legacy CTest driver test with GoogleTest framework.
Adds EXPECT_EQ assertions for ITK major, minor, and patch versions.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace legacy CTest driver test with GoogleTest framework.
Adds EXPECT_EQ assertions verifying each vector element value.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace legacy CTest driver test with GoogleTest framework.
Adds assertions for event derivation, self-matching, unrelated event
rejection, and event name verification.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace legacy CTest driver test with GoogleTest framework.
Adds EXPECT_GT/EXPECT_GE assertions that modified time increases after
calling Modified() and that bounding box time reflects point changes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace legacy CTest driver test with GoogleTest framework. Splits into
separate test cases per type (char, short, int, long, long long, float,
double) and adds EXPECT_EQ/EXPECT_TRUE assertions after double-swap
round-trips.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace legacy CTest driver test with GoogleTest framework. Splits into
separate test cases for fixed-width, least, fast, and max/pointer integer
types with EXPECT_TRUE assertions on size and signedness.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Consolidate itkMathRoundTest, itkMathRoundTest2, and
itkMathRoundProfileTest1 into a single GoogleTest file. The profiling
test was purely diagnostic (no assertions) and its coverage is now
subsumed by the typed round tests.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ckGTest

Replace legacy CTest driver test with GoogleTest framework. Splits into
separate test cases per target type with EXPECT_TRUE assertions. Uses
EXPECT_THROW to verify range overflow detection.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace legacy CTest driver test with GoogleTest framework. Adds a
TypeTraits test verifying static_assert conditions at runtime, and a
ThreadSafeMonotonicity test with EXPECT_EQ/EXPECT_LE assertions
verifying that multi-threaded Modified() calls assign unique,
consecutive modified times.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace legacy CTest driver test with GoogleTest framework. Splits into
separate test cases: EmptyBoxDefaults, OneDimensionalBox,
ThreeDimensionalIsInside, ComputeCorners, and DeepCopy. Adds EXPECT_EQ,
EXPECT_TRUE/FALSE, and EXPECT_NEAR assertions.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace legacy CTest driver test with GoogleTest framework. Adds
EXPECT_EQ assertions verifying the 15 neighborhood pixel values at the
image boundary corner with ConstantBoundaryCondition(0), and verifies
ZeroFluxNeumannBoundaryCondition traversal completes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@hjmjohnson hjmjohnson force-pushed the simplified-conversion-to-gtest branch from 35749d1 to e8fb9a9 Compare March 6, 2026 23:23
@hjmjohnson hjmjohnson marked this pull request as ready for review March 7, 2026 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Core Issues affecting the Core module type:Enhancement Improvement of existing methods or implementation type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants