feat(assertion): Add DefaultPrepAndExpectedTestCase.setFailureHandler() - #911
Merged
Merged
Conversation
Reviewer's GuideAdds an optional FailureHandler to DefaultPrepAndExpectedTestCase so verifyData()/compareData() can use a caller-supplied handler instead of always failing fast with the default, and introduces tests plus changelog updates to cover and document the new behavior. Sequence diagram for DefaultPrepAndExpectedTestCase.compareData() failure handlingsequenceDiagram
participant TestCase as DefaultPrepAndExpectedTestCase
participant Assertion
participant FailureHandler
TestCase->>TestCase: verifyData()
TestCase->>TestCase: compareData(expectedTable, actualTable, ...)
alt failureHandler is null
TestCase->>Assertion: assertWithValueComparer(expectedTable, actualTable, additionalColumnInfo, defaultValueComparer, columnValueComparers)
Assertion-->>TestCase: uses DefaultFailureHandler (fail-fast)
else failureHandler is set
TestCase->>Assertion: assertWithValueComparer(expectedTable, actualTable, failureHandler, defaultValueComparer, columnValueComparers)
Assertion->>FailureHandler: delegate mismatches to supplied handler
end
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 44 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough
ChangesFailure handler support
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Test
participant DefaultPrepAndExpectedTestCase
participant Assertion
participant FailureHandler
Test->>DefaultPrepAndExpectedTestCase: verifyData()
DefaultPrepAndExpectedTestCase->>Assertion: compareData()
alt No configured handler
Assertion->>Assertion: use default fail-fast handler
Assertion-->>Test: throw Error on mismatch
else Configured handler
DefaultPrepAndExpectedTestCase->>Assertion: pass FailureHandler
Assertion->>FailureHandler: collect mismatch
Assertion-->>Test: complete without throwing
end
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The Javadoc on getFailureHandler/setFailureHandler uses
@see {@link #failureHandler}which is a bit redundant and non-idiomatic; consider simplifying to@see #failureHandleror inlining the reference in the descriptive text instead. - In testVerifyData_withMismatchAndNoFailureHandlerConfigured_throwsError, asserting against the broad Error type makes the contract less clear; consider asserting the concrete comparison failure type (e.g., DbComparisonFailure or the specific Error subtype actually thrown) to better document the behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The Javadoc on getFailureHandler/setFailureHandler uses `@see {@link #failureHandler}` which is a bit redundant and non-idiomatic; consider simplifying to `@see #failureHandler` or inlining the reference in the descriptive text instead.
- In testVerifyData_withMismatchAndNoFailureHandlerConfigured_throwsError, asserting against the broad Error type makes the contract less clear; consider asserting the concrete comparison failure type (e.g., DbComparisonFailure or the specific Error subtype actually thrown) to better document the behavior.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/org/dbunit/DefaultPrepAndExpectedTestCase.java`:
- Around line 71-77: Update the JavaDoc in DefaultPrepAndExpectedTestCase so the
sentence introduced by verifyData() starts with a capitalized subject rather
than beginning directly with the method name. Keep the existing explanation
about DefaultFailureHandler and setFailureHandler(FailureHandler) unchanged, but
rewrite the topic text to read as a complete sentence with a capital letter and
proper punctuation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0703c4b2-118d-403f-8b3a-f1a537b66865
📒 Files selected for processing (3)
src/changes/changes.xmlsrc/main/java/org/dbunit/DefaultPrepAndExpectedTestCase.javasrc/test/java/org/dbunit/DefaultPrepAndExpectedTestCaseTest.java
Some tests want DiffCollectingFailureHandler to collect every mismatch instead of failing on the first one, but that should not become the default. Add a failureHandler property, defaulting to null so compareData() keeps using Assertion's own additionalColumnInfo-based DefaultFailureHandler unchanged; when set, compareData() uses the configured FailureHandler instead. Refs: 865
jeffjensen
force-pushed
the
865-prepandexpectedtestcase-failurehandler
branch
from
August 6, 2026 14:30
2e1c5a0 to
dcef8a5
Compare
Member
Author
|
Addressed the review feedback in dcef8a5 (amended, single commit):
Full unit suite (1969 tests) green after the changes; force-pushed once. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DefaultPrepAndExpectedTestCase.verifyData()always compared tables viaAssertion.assertWithValueComparer(ITable, ITable, Column[], ValueComparer, Map), which internally builds its ownDefaultFailureHandlerand throws on the first mismatch found — there was no way to plug in a differentFailureHandler.failureHandlerproperty (getter/setter), defaulting tonullsocompareData()keeps using the additionalColumnInfo-basedAssertion.assertWithValueComparer(...)overload (and itsDefaultFailureHandler) unchanged; when set,compareData()uses theFailureHandler-based overload with the configured handler instead — e.g. aDiffCollectingFailureHandlerto collect everyDifferenceinstead of failing fast.DefaultPrepAndExpectedTestCaseTestcovering the null-default (unchanged) path and the configured-handler (collects-differences-instead-of-throwing) path.Fixes #865
Test plan
./mvnw clean test— 1969 unit tests pass🤖 Generated with Claude Code
Summary by Sourcery
Allow DefaultPrepAndExpectedTestCase to use a configurable FailureHandler when verifying data while preserving the existing fail-fast default behavior.
New Features:
Documentation:
Tests:
Summary by CodeRabbit
New Features
Documentation