Skip to content

ENH: Improve VariationalRegistration test conventions and failure handling - #7113

Open
womputer wants to merge 1 commit into
InsightSoftwareConsortium:mainfrom
womputer:work/6355-improve-tests
Open

womputer wants to merge 1 commit into
InsightSoftwareConsortium:mainfrom
womputer:work/6355-improve-tests

Conversation

@womputer

@womputer womputer commented Nov 4, 2026

Copy link
Copy Markdown

Update VariationalRegistration tests to ITK conventions and retain failures while continuing independent checks. Guard pixel comparisons behind successful image updates. Fixes #6355.

Implementation

Rename both tests and their CMake registrations with the itk prefix. Replace manual exception checks and Print calls with ITK testing macros. Immediately invoked lambdas confine macro early returns to individual checks; an integer testStatus accumulates their results. Reset the registration pipeline between update scenarios.

Remove unused helpers, use explicit constants and casts, and apply the repository's formatting. Registration settings and the ten-pixel comparison tolerance are unchanged.

Local validation

macOS arm64, Apple clang 17.0.0, Release build with Module_VariationalRegistration enabled:

cmake --build build --target VariationalRegistrationTestDriver --parallel 4
ctest --test-dir build --output-on-failure -R '^itkVariationalRegistration(Filter|MultiResolutionFilter)Test$'

CTest output after restoring the temporary fault injections:

100% tests passed out of 2
Total Test time (real) =   1.30 sec

Fault-harness output (each check asserts exit code 1 and the expected later output; update failures must skip pixel access):

pixel-failure itkVariationalRegistrationFilterTest verified: failure exit and expected subsequent behavior
pixel-failure itkVariationalRegistrationMultiResolutionFilterTest verified: failure exit and expected subsequent behavior
update-failure itkVariationalRegistrationFilterTest verified: failure exit and expected subsequent behavior
update-failure itkVariationalRegistrationMultiResolutionFilterTest verified: failure exit and expected subsequent behavior
missing-expected-exception itkVariationalRegistrationFilterTest verified: failure exit and expected subsequent behavior

The complete ITK suite and external-data image-baseline tests were not run. Cross-platform CI remains pending.

AI assistance

OpenAI Codex assisted with code inspection, implementation, style cleanup, and local validation, including temporary failure injection. Contributor review remains in progress; this PR stays in Draft until that review and CI are complete.

PR Checklist
  • No API changes were made.
  • No major design changes were made.
  • Updated existing tests; registration behavior unchanged. Temporary failure-path validation described above.
  • API documentation unchanged because API is unchanged.
  • Existing license headers retained in renamed test files.
  • Python wrapping: not applicable to C++ test-only changes.
  • ITK examples: not applicable; no new major feature.
Failure-continuation reproduction

Temporarily force the pixel comparison to fail in each 128-by-128 synthetic
image test and add a marker immediately before its later independent object
check. At baseline 84ce566, both return exit 1 without reaching the marker.
With this patch, both return exit 1 and reach the marker. For this experiment,
baseline entry points were renamed only to reuse the test driver. All mutations
were restored before the final build and normal test run.

                    single-resolution   multiresolution
baseline exit       1                   1
baseline marker     absent              absent
modified exit       1                   1
modified marker     present             present
normal tests        2/2 passed after restoration

@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:Registration Issues affecting the Registration module labels Nov 4, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thank you for contributing a pull request! 🙏

Welcome to the ITK community! 🤗👋☀️

We are glad you are here and appreciate your contribution. Please keep in mind our community participation guidelines. 📜
More support and guidance on the contribution process can be found in our contributing guide. 📖

This is an automatic message. Allow for time for the ITK community to be able to read the pull request and comment
on it.

@womputer
womputer force-pushed the work/6355-improve-tests branch from 44fe0cf to ec88f55 Compare November 5, 2026 20:51
@womputer
womputer marked this pull request as ready for review November 6, 2026 18:37
@greptile-apps

greptile-apps Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] The PR appears safe to merge; no actionable issues were found.

Summary

Updates both VariationalRegistration tests to ITK conventions without changing registration settings or the ten-pixel tolerance.

  • Single-resolution registration checks continue after an individual check fails.
  • Multi-resolution registration checks continue after an individual check fails.

No actionable issues were found. Tests were not run during this review.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Run image update] --> B{Update succeeds?}
  B -->|Yes| C[Compare pixels]
  B -->|No| D[Record failure and skip pixels]
  C --> E[Keep check result]
  D --> F[Run later checks]
  E --> F
  F --> G[Return accumulated status]
Loading

Reviews (1) · Last reviewed commit: "ENH: Improve VariationalRegistration tes..." · Reviewed by Greptile

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

Labels

area:Registration Issues affecting the Registration 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.

Improve tests.

1 participant