feat(junit5): Add DbUnitExtension for JUnit 5/6 lifecycle management - #912
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's GuideIntroduces a new JUnit 5/6 DbUnitExtension that discovers an IDatabaseTester field on the test instance, runs its onSetup/onTearDown around each test method using JUnit’s BeforeTestExecutionCallback/AfterTestExecutionCallback hooks, and adds tests plus build metadata and dependencies to validate lifecycle ordering and error handling. Sequence diagram for DbUnitExtension test lifecycle managementsequenceDiagram
participant JUnitEngine
participant DbUnitExtension
participant ExtensionContext
participant TestInstance
participant IDatabaseTester
JUnitEngine->>DbUnitExtension: beforeTestExecution(ExtensionContext)
DbUnitExtension->>ExtensionContext: getTestInstance()
ExtensionContext-->>DbUnitExtension: Optional[TestInstance]
DbUnitExtension->>DbUnitExtension: resolveTester(ExtensionContext)
DbUnitExtension->>TestInstance: access IDatabaseTester field
TestInstance-->>DbUnitExtension: IDatabaseTester
DbUnitExtension->>ExtensionContext: getStore(NAMESPACE).put(TESTER_KEY, tester)
DbUnitExtension->>IDatabaseTester: onSetup()
DbUnitExtension-->>JUnitEngine:
JUnitEngine->>DbUnitExtension: afterTestExecution(ExtensionContext)
DbUnitExtension->>ExtensionContext: getStore(NAMESPACE).get(TESTER_KEY, IDatabaseTester)
ExtensionContext-->>DbUnitExtension: IDatabaseTester
DbUnitExtension->>IDatabaseTester: onTearDown()
DbUnitExtension-->>JUnitEngine:
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughChangesJUnit DbUnit extension lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant JUnit as JUnit test execution
participant DbUnitExtension
participant TestInstance
participant IDatabaseTester
JUnit->>DbUnitExtension: beforeTestExecution
DbUnitExtension->>TestInstance: resolve IDatabaseTester field
TestInstance-->>DbUnitExtension: IDatabaseTester
DbUnitExtension->>IDatabaseTester: onSetup
JUnit->>DbUnitExtension: afterTestExecution
DbUnitExtension->>IDatabaseTester: onTearDown
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 found 1 issue, and left some high level feedback:
- The new DbUnitExtension is annotated with
@since 3.2.0but the change is recorded under the 3.4.1-SNAPSHOT release in changes.xml; align the@sincetag with the actual version this is introduced in. - changes.xml now contains two separate
<action>entries describing the addition of DbUnitExtension (issues 751 and TBD, in different releases); consider consolidating or removing the duplicate so the changelog reflects a single, accurate entry.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new DbUnitExtension is annotated with `@since 3.2.0` but the change is recorded under the 3.4.1-SNAPSHOT release in changes.xml; align the `@since` tag with the actual version this is introduced in.
- changes.xml now contains two separate `<action>` entries describing the addition of DbUnitExtension (issues 751 and TBD, in different releases); consider consolidating or removing the duplicate so the changelog reflects a single, accurate entry.
## Individual Comments
### Comment 1
<location path="src/main/java/org/dbunit/junit5/DbUnitExtension.java" line_range="114-117" />
<code_context>
+ clazz = clazz.getSuperclass();
+ }
+
+ throw new IllegalStateException("No IDatabaseTester field found in "
+ + testInstance.getClass().getName()
+ + " or its superclasses. Declare a non-static field of type IDatabaseTester"
+ + " to use DbUnitExtension.");
+ }
+}
</code_context>
<issue_to_address>
**nitpick (typo):** Error message could be clearer about accepting IDatabaseTester subtypes, not just the raw interface.
Since the resolution logic uses `IDatabaseTester.class.isAssignableFrom(field.getType())`, it also accepts subclasses/implementations. The current wording (“field of type IDatabaseTester”) could confuse users with a custom `MyDatabaseTester extends IDatabaseTester`. Consider rephrasing to “Declare a non-static field whose type implements IDatabaseTester” to reflect the actual behavior.
```suggestion
throw new IllegalStateException("No IDatabaseTester field found in "
+ testInstance.getClass().getName()
+ " or its superclasses. Declare a non-static field whose type implements IDatabaseTester"
+ " to use DbUnitExtension.");
```
</issue_to_address>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: 3
🧹 Nitpick comments (1)
src/test/java/org/dbunit/junit5/DbUnitExtensionLifecycleTest.java (1)
48-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the lifecycle test methods to state condition and outcome.
Use names such as
testBeforeAndAfterTestExecution_testMethodSucceeds_callbacksRunInOrderandtestAfterTestExecution_testMethodFails_onTearDownCalled.As per coding guidelines, “use method names in the form
test<MethodName>_<StartingStateConditions>_<AssertedOutcome>.”🤖 Prompt for 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. In `@src/test/java/org/dbunit/junit5/DbUnitExtensionLifecycleTest.java` around lines 48 - 69, Rename the two lifecycle test methods to follow the test<MethodName>_<StartingStateConditions>_<AssertedOutcome> convention: use testBeforeAndAfterTestExecution_testMethodSucceeds_callbacksRunInOrder for the successful lifecycle-order test and testAfterTestExecution_testMethodFails_onTearDownCalled for the failing-test cleanup case.Source: Coding guidelines
🤖 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/changes/changes.xml`:
- Line 543: Correct the release attribution for DbUnitExtension: remove its
historical 3.2.0 action, including issue="TBD", from src/changes/changes.xml
lines 543-543; remove the historical 3.4.0 action from src/changes/changes.xml
lines 511-513; and update the DbUnitExtension `@since` tag in
src/main/java/org/dbunit/junit5/DbUnitExtension.java line 63 to 3.4.1.
In `@src/main/java/org/dbunit/junit5/DbUnitExtension.java`:
- Around line 73-87: Add complete JavaDoc to the public callback methods
beforeTestExecution and afterTestExecution in DbUnitExtension, including a
capitalized, period-terminated summary, a `@param` description for context, and a
`@throws` description for Exception.
- Around line 96-109: Update resolveTester() to collect non-static fields
assignable to IDatabaseTester in the nearest class, reject the case where more
than one matches with a clear IllegalStateException, and retain the existing
null validation and tester return for exactly one match. Add a test covering a
class with multiple IDatabaseTester fields and asserting the rejection.
---
Nitpick comments:
In `@src/test/java/org/dbunit/junit5/DbUnitExtensionLifecycleTest.java`:
- Around line 48-69: Rename the two lifecycle test methods to follow the
test<MethodName>_<StartingStateConditions>_<AssertedOutcome> convention: use
testBeforeAndAfterTestExecution_testMethodSucceeds_callbacksRunInOrder for the
successful lifecycle-order test and
testAfterTestExecution_testMethodFails_onTearDownCalled for the failing-test
cleanup case.
🪄 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: 9bcdf0a7-8c29-435d-96e0-3fe56a6bb896
📒 Files selected for processing (5)
pom.xmlsrc/changes/changes.xmlsrc/main/java/org/dbunit/junit5/DbUnitExtension.javasrc/test/java/org/dbunit/junit5/DbUnitExtensionLifecycleTest.javasrc/test/java/org/dbunit/junit5/DbUnitExtensionTest.java
jeffjensen
force-pushed
the
feat/junit5-extension
branch
2 times, most recently
from
August 6, 2026 21:08
ffce9ef to
76cffa9
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/test/java/org/dbunit/junit/jupiter/DbUnitExtensionTest.java (1)
195-203: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStrengthen the no-stored-tester assertion.
The store returns
null, sodatabaseTestercan never receiveonTearDown(). Thenever()check therefore always passes. UseverifyNoInteractions(databaseTester)to state the intent, or assert thatafterTestExecutioncompletes without an exception.Proposed change
- verify(databaseTester, never()).onTearDown(); + verifyNoInteractions(databaseTester);Add the import:
import static org.mockito.Mockito.verifyNoInteractions;🤖 Prompt for 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. In `@src/test/java/org/dbunit/junit/jupiter/DbUnitExtensionTest.java` around lines 195 - 203, Strengthen testAfterTestExecution_noStoredTester_doesNotCallOnTearDown by replacing the ineffective verify(databaseTester, never()) assertion with verifyNoInteractions(databaseTester), adding the corresponding Mockito static import.src/test/java/org/dbunit/junit/jupiter/DbUnitExtensionLifecycleTest.java (2)
46-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the blank line after the opening brace.
Proposed fix
class DbUnitExtensionLifecycleTest { - `@Test`As per coding guidelines, "Do not leave a blank line immediately after an opening curly brace."
🤖 Prompt for 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. In `@src/test/java/org/dbunit/junit/jupiter/DbUnitExtensionLifecycleTest.java` around lines 46 - 47, Remove the blank line immediately after the opening brace of the DbUnitExtensionLifecycleTest class, leaving the class declaration directly followed by its first member or statement.Source: Coding guidelines
126-127: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStatic
CALL_LOGfields couple the sample classes to the outer test methods.
FailingTestSample.CALL_LOGandFailingSetupSample.CALL_LOGhold state across executions. The outer methods clear each list beforeEngineTestKitruns. If the project later enables JUnit parallel execution for this class, the clear-then-assert sequence becomes order dependent, even though the lists are synchronized.
LifecycleOrderSampleavoids this problem by asserting inside the sample class with an instance list. Consider the same approach for the two failure samples, or annotate this class with@Execution(SAME_THREAD).This is optional. The current tests are correct under sequential execution.
Also applies to: 142-143
🤖 Prompt for 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. In `@src/test/java/org/dbunit/junit/jupiter/DbUnitExtensionLifecycleTest.java` around lines 126 - 127, Remove the static CALL_LOG state from FailingTestSample and FailingSetupSample to avoid cross-execution coupling. Use instance-owned logs and perform lifecycle assertions inside each sample, matching LifecycleOrderSample; alternatively, annotate the enclosing test class with `@Execution`(SAME_THREAD) to preserve the clear-then-assert ordering.src/main/java/org/dbunit/junit/jupiter/DbUnitExtension.java (2)
108-135: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider supporting
@Nestedtest classes throughgetRequiredTestInstances().
resolveTesteronly scans the innermost test instance and its superclasses. The class JavaDoc documents this limitation. JUnit Jupiter exposesExtensionContext.getTestInstances(), which returns all enclosing instances for@Nestedclasses. You can scan those instances from innermost to outermost and remove the limitation.This is optional. The current behavior is documented and fails with a clear message.
🤖 Prompt for 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. In `@src/main/java/org/dbunit/junit/jupiter/DbUnitExtension.java` around lines 108 - 135, Optionally update resolveTester to use ExtensionContext.getTestInstances() and scan all enclosing `@Nested` test instances from innermost to outermost, applying the existing field lookup and null validation to each instance. Remove or revise the JavaDoc and failure message that state only the innermost instance and its superclasses are supported.
137-152: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
findTesterFieldmixes lookup and message formatting.The
testInstanceparameter is used only to build the exception message. The method name states field lookup. Consider moving the ambiguity check message to the caller, or rename the method to state both duties.This is a small clarity point. The behavior is correct.
🤖 Prompt for 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. In `@src/main/java/org/dbunit/junit/jupiter/DbUnitExtension.java` around lines 137 - 152, Refactor findTesterField so field lookup and ambiguity-message construction are separated: remove the testInstance dependency from the lookup method and move the multiple-field exception formatting to its caller, or rename the method to explicitly reflect both responsibilities. Preserve the existing ambiguity validation and exception message behavior.
🤖 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/site/asciidoc/testcases/IDatabaseTester.adoc`:
- Around line 103-106: Update the DbUnitExtension tip to clarify that it
replaces only the manual onSetup() and onTearDown() calls, not the entire
configuration hook. In the example around the `@BeforeEach/`@AfterEach pair,
instruct users to retain `@BeforeEach` when it configures the tester with the
dataset and teardown operation.
---
Nitpick comments:
In `@src/main/java/org/dbunit/junit/jupiter/DbUnitExtension.java`:
- Around line 108-135: Optionally update resolveTester to use
ExtensionContext.getTestInstances() and scan all enclosing `@Nested` test
instances from innermost to outermost, applying the existing field lookup and
null validation to each instance. Remove or revise the JavaDoc and failure
message that state only the innermost instance and its superclasses are
supported.
- Around line 137-152: Refactor findTesterField so field lookup and
ambiguity-message construction are separated: remove the testInstance dependency
from the lookup method and move the multiple-field exception formatting to its
caller, or rename the method to explicitly reflect both responsibilities.
Preserve the existing ambiguity validation and exception message behavior.
In `@src/test/java/org/dbunit/junit/jupiter/DbUnitExtensionLifecycleTest.java`:
- Around line 46-47: Remove the blank line immediately after the opening brace
of the DbUnitExtensionLifecycleTest class, leaving the class declaration
directly followed by its first member or statement.
- Around line 126-127: Remove the static CALL_LOG state from FailingTestSample
and FailingSetupSample to avoid cross-execution coupling. Use instance-owned
logs and perform lifecycle assertions inside each sample, matching
LifecycleOrderSample; alternatively, annotate the enclosing test class with
`@Execution`(SAME_THREAD) to preserve the clear-then-assert ordering.
In `@src/test/java/org/dbunit/junit/jupiter/DbUnitExtensionTest.java`:
- Around line 195-203: Strengthen
testAfterTestExecution_noStoredTester_doesNotCallOnTearDown by replacing the
ineffective verify(databaseTester, never()) assertion with
verifyNoInteractions(databaseTester), adding the corresponding Mockito static
import.
🪄 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: cbeb13ab-c397-4f84-bc2e-ecde3cdee2ce
📒 Files selected for processing (10)
CLAUDE.mdpom.xmlsrc/changes/changes.xmlsrc/main/java/org/dbunit/junit/jupiter/DbUnitExtension.javasrc/site/asciidoc/testcases.adocsrc/site/asciidoc/testcases/DbUnitExtension.adocsrc/site/asciidoc/testcases/IDatabaseTester.adocsrc/site/site.xmlsrc/test/java/org/dbunit/junit/jupiter/DbUnitExtensionLifecycleTest.javasrc/test/java/org/dbunit/junit/jupiter/DbUnitExtensionTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
- pom.xml
- src/changes/changes.xml
Add DbUnitExtension, a JUnit 5/6 extension that wires IDatabaseTester into the test lifecycle via BeforeTestExecutionCallback and AfterTestExecutionCallback. The extension discovers the IDatabaseTester by scanning instance fields (including inherited), so test classes configure their tester in @beforeeach and annotate with @ExtendWith(DbUnitExtension.class) rather than extending DatabaseTestCase. Package the extension as org.dbunit.junit.jupiter, mirroring JUnit's own org.junit.jupiter namespace, rather than org.dbunit.junit5: JUnit 6 kept the Jupiter programming model and its org.junit.jupiter.* API package unchanged from JUnit 5 (dbUnit itself now builds and tests against JUnit 6), so "jupiter" stays accurate across major versions where a version-numbered package name would not. Reject a test class that declares more than one non-static IDatabaseTester-assignable field at the same class level instead of silently using Class#getDeclaredFields()'s undefined iteration order, and cover the case with a test. Add an EngineTestKit lifecycle test verifying onTearDown() still runs (and the test method itself never executes) when onSetup() throws. Document the @nested limitation and the field-ambiguity rejection in the class Javadoc. Add the testcases/DbUnitExtension.adoc site page, cross-linked from testcases.adoc and testcases/IDatabaseTester.adoc, and list the new org.dbunit.junit.jupiter package in CLAUDE.md's package structure. Clarify the IDatabaseTester.adoc TIP on DbUnitExtension: it only automates the onSetup()/onTearDown() calls, not tester configuration, so tell readers to keep @beforeeach when it still configures the tester (e.g. setDataSet()/setTearDownOperation()) instead of implying the whole method can be deleted. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Refs: 751
jeffjensen
force-pushed
the
feat/junit5-extension
branch
from
August 7, 2026 00:11
76cffa9 to
13371a3
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
This was referenced Aug 15, 2026
Merged
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
DbUnitExtension, a JUnit 5/6 extension that wiresIDatabaseTesterinto the test lifecycle viaBeforeTestExecutionCallbackandAfterTestExecutionCallback.IDatabaseTesterby scanning instance fields (including inherited), so test classes configure their tester in@BeforeEachand annotate with@ExtendWith(DbUnitExtension.class)rather than extendingDatabaseTestCase.Fixes #751
Test plan
./mvnw clean test— full unit test suite passes (1985 tests, 0 failures)./mvnw clean install site— builds and site generation succeed🤖 Generated with Claude Code
https://claude.ai/code/session_015h89ycy4uviGgM4tUBdarR
Summary by CodeRabbit
New Features
DatabaseTestCaseinheritance.Documentation
Tests