Junit6 remnants cleanup - #878
Merged
Merged
Conversation
TransactionOperationIT overrode the JUnit-3-era runTest(String) hook to skip on databases without transaction support, but that hook is never invoked under Jupiter (a leftover from the JUnit migration) -- the gate was silently inert. Replace it with a real @beforeeach Assumptions.assumeTrue(environmentHasFeature(TestFeature.TRANSACTION)). Refs: 870
TestCase.runTest() was JUnit 3's per-test template method. Its override pair, runTest(String testName)/runTest(), survived the JUnit migration in AbstractDatabaseIT and AbstractTableTest, but nothing under Jupiter calls it -- AbstractDatabaseIT.runTest()'s own body was commented out (// super.runTest();), and there is no TestCase base to invoke it. TruncateTableOperationIT and InsertIdentityOperationIT each overrode it redundantly with a feature check that their already-active @DisabledIfSystemProperty/@EnabledIfSystemProperty annotations already cover. Remove the whole dead chain. Refs: 869
27 AllTestsSuite/AllAssertionTestSuite/DatabaseTestSuite classes use org.junit.platform.suite.api.@suite + @SelectClasses -- a JUnit 3/4-era "suite of suites" pattern carried through the JUnit migration. None of their filenames match Surefire's **/*Test.java or Failsafe's **/*IT.java include patterns, so the JUnit Platform launcher never received them as roots. Confirmed unreferenced by any CI workflow, build script, site doc, or IDE configuration -- the root suite's own @SelectClasses list even carried a dead, already-broken commented-out entry, proof they were unmaintained rather than merely unused. Filename-pattern discovery already gives full automatic coverage with zero manual curation, so removing them has no effect on actual test execution or coverage. Refs: 871
DbUnitAssertBase.getJUnitFailureFactory() probed for junit.framework.Assert to decide whether JUnit was on the classpath -- a JUnit 3/4 class that has never been present since the project's JUnit 5/6 migration dropped the vintage engine, permanently disabling the JUnitFailureFactory path. Point the probe at org.junit.jupiter.api.Assertions instead. JUnitFailureFactory keeps returning dbUnit's own DbComparisonFailure/ DbAssertionFailedError rather than a JUnit-framework-specific type: DbComparisonFailure's own javadoc states its purpose is to avoid a direct dependency on any particular testing framework, and unifying it with org.opentest4j.AssertionFailedError would force a hard opentest4j runtime dependency onto every dbUnit user, not just those on JUnit 5/6. Removed the stale comments and the never-resolved "TODO Junit5 update" that no longer matched what the code does. Refs: 872
The enforcer plugin's requireJavaVersion rule was tied to compileSource (1.8), a leftover from before the JUnit 6 migration -- but junit-jupiter 6.x class files require Java 17 to load (verified: bytecode major version 61), so mvn test/verify cannot run on Java 8-16 regardless of what the enforcer claims. CI only worked because .github/actions/jdk-setup pins JDK 21 independently of this rule. Split the concerns: a new enforcerJavaVersion property (17) drives requireJavaVersion, while compileSource (1.8) is unchanged and continues to set the shipped jar's bytecode level for consumers who don't need dbUnit's JUnit test-support classes. Refs: 873
…adoc The "dbUnit in 5 Minutes" tutorial told new users "JDK 8 or newer" immediately above "JUnit 5 (org.junit.jupiter:junit-jupiter) on your test classpath" -- but the current JUnit Jupiter coordinates (6.x) require Java 17 to load. Clarify that JDK 17+ is needed to build/run the tutorial's test, while JDK 8 remains sufficient for dbUnit's core API alone. Refs: 874
The two tracked Eclipse launch configs at the repo root ("dbUnit tests
- derby.launch", "dbUnit tests - hsqldb.launch") were last updated in
2024, well before the JUnit 5/6 migrations, and still specify
org.eclipse.jdt.junit.loader.junit4 -- Eclipse's JUnit 4 loader cannot
discover or run Jupiter @test methods without a vintage-engine bridge
this project doesn't depend on, so as configured these find zero
tests. Both also carried dbunit.profile.unsupportedFeatures VM
arguments that had drifted from the current *-dbunit.properties files
(e.g. missing TIMESTAMP_WITH_TIMEZONE, added later per issue #831).
Removed rather than fixed, per maintainer confirmation that no one
relies on Eclipse-native (non-Maven) test runs for these profiles.
Refs: 875
Hamcrest has not been a dependency since the JUnit migration -- no hamcrestVersion property and no org.hamcrest import exist anywhere in the tree -- so this pattern in the test-dependencies group has silently matched zero packages on every Dependabot run. Refs: 876
testcases.adoc and testcases/MigratingToIDatabaseTester.adoc described DatabaseTestCase's lifecycle-method behavior as "JUnit 5" specifically, even though the project now ships and tests against JUnit 6 (same org.junit.jupiter:junit-jupiter coordinates, so this is a wording generalization, not a correction). fiveminutes.adoc's equivalent mention was already fixed alongside its JDK prerequisite (05dd06e4). Historical-milestone text (index.adoc's "Since 3.0.0..." framing) and changes.xml action text are left untouched, since those describe what was true at the time. Refs: 877
📝 WalkthroughWalkthroughChangesJUnit modernization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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`:
- Around line 100-126: Update the 3.4.1-SNAPSHOT release entry in changes.xml by
replacing the empty description attribute with a brief summary of the included
JUnit modernization changes. Keep the existing action entries unchanged and
ensure the description is concise and release-focused.
In `@src/main/java/org/dbunit/assertion/JUnitFailureFactory.java`:
- Around line 25-31: In the JavaDoc for JUnitFailureFactory, capitalize the
first word of the second sentence beginning with “dbUnit's”; preserve the
existing wording and ensure the sentence remains properly punctuated.
🪄 Autofix (Beta)
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: 6e2c4345-3050-4b41-9158-e28a73824060
📒 Files selected for processing (43)
.github/dependabot.ymldbUnit tests - derby.launchdbUnit tests - hsqldb.launchpom.xmlsrc/changes/changes.xmlsrc/main/java/org/dbunit/assertion/DbUnitAssertBase.javasrc/main/java/org/dbunit/assertion/JUnitFailureFactory.javasrc/site/asciidoc/fiveminutes.adocsrc/site/asciidoc/testcases.adocsrc/site/asciidoc/testcases/MigratingToIDatabaseTester.adocsrc/test/java/org/dbunit/AbstractDatabaseIT.javasrc/test/java/org/dbunit/AllTestsSuite.javasrc/test/java/org/dbunit/assertion/AllAssertionTestSuite.javasrc/test/java/org/dbunit/assertion/JUnitFailureFactoryTest.javasrc/test/java/org/dbunit/database/DatabaseTestSuite.javasrc/test/java/org/dbunit/database/search/AllTestsSuite.javasrc/test/java/org/dbunit/database/statement/AllTestsSuite.javasrc/test/java/org/dbunit/dataset/AbstractTableTest.javasrc/test/java/org/dbunit/dataset/AllTestsSuite.javasrc/test/java/org/dbunit/dataset/common/handlers/AllTestsSuite.javasrc/test/java/org/dbunit/dataset/csv/AllTestsSuite.javasrc/test/java/org/dbunit/dataset/datatype/AllTestsSuite.javasrc/test/java/org/dbunit/dataset/excel/AllTestsSuite.javasrc/test/java/org/dbunit/dataset/filter/AllTestsSuite.javasrc/test/java/org/dbunit/dataset/sqlloader/AllTestsSuite.javasrc/test/java/org/dbunit/dataset/stream/AllTestsSuite.javasrc/test/java/org/dbunit/dataset/xml/AllTestsSuite.javasrc/test/java/org/dbunit/ext/AllTestsSuite.javasrc/test/java/org/dbunit/ext/db2/AllTestsSuite.javasrc/test/java/org/dbunit/ext/h2/AllTestsSuite.javasrc/test/java/org/dbunit/ext/hsqldb/AllTestsSuite.javasrc/test/java/org/dbunit/ext/mckoi/AllTestsSuite.javasrc/test/java/org/dbunit/ext/mssql/AllTestsSuite.javasrc/test/java/org/dbunit/ext/mssql/InsertIdentityOperationIT.javasrc/test/java/org/dbunit/ext/mysql/AllTestsSuite.javasrc/test/java/org/dbunit/ext/oracle/AllTestsSuite.javasrc/test/java/org/dbunit/ext/postgresql/AllTestsSuite.javasrc/test/java/org/dbunit/operation/AllTestsSuite.javasrc/test/java/org/dbunit/operation/TransactionOperationIT.javasrc/test/java/org/dbunit/operation/TruncateTableOperationIT.javasrc/test/java/org/dbunit/util/AllTestsSuite.javasrc/test/java/org/dbunit/util/search/AllTestsSuite.javasrc/test/java/org/dbunit/util/xml/AllTestsSuite.java
💤 Files with no reviewable changes (34)
- src/test/java/org/dbunit/dataset/common/handlers/AllTestsSuite.java
- dbUnit tests - hsqldb.launch
- src/test/java/org/dbunit/assertion/AllAssertionTestSuite.java
- dbUnit tests - derby.launch
- src/test/java/org/dbunit/util/AllTestsSuite.java
- src/test/java/org/dbunit/dataset/csv/AllTestsSuite.java
- src/test/java/org/dbunit/util/search/AllTestsSuite.java
- src/test/java/org/dbunit/ext/postgresql/AllTestsSuite.java
- src/test/java/org/dbunit/ext/db2/AllTestsSuite.java
- src/test/java/org/dbunit/ext/mysql/AllTestsSuite.java
- src/test/java/org/dbunit/ext/mssql/AllTestsSuite.java
- src/test/java/org/dbunit/dataset/datatype/AllTestsSuite.java
- src/test/java/org/dbunit/database/statement/AllTestsSuite.java
- src/test/java/org/dbunit/ext/h2/AllTestsSuite.java
- src/test/java/org/dbunit/operation/AllTestsSuite.java
- src/test/java/org/dbunit/ext/oracle/AllTestsSuite.java
- src/test/java/org/dbunit/database/DatabaseTestSuite.java
- src/test/java/org/dbunit/dataset/sqlloader/AllTestsSuite.java
- src/test/java/org/dbunit/dataset/stream/AllTestsSuite.java
- src/test/java/org/dbunit/dataset/AbstractTableTest.java
- .github/dependabot.yml
- src/test/java/org/dbunit/database/search/AllTestsSuite.java
- src/test/java/org/dbunit/AbstractDatabaseIT.java
- src/test/java/org/dbunit/dataset/excel/AllTestsSuite.java
- src/test/java/org/dbunit/util/xml/AllTestsSuite.java
- src/test/java/org/dbunit/dataset/xml/AllTestsSuite.java
- src/test/java/org/dbunit/dataset/filter/AllTestsSuite.java
- src/test/java/org/dbunit/AllTestsSuite.java
- src/test/java/org/dbunit/ext/hsqldb/AllTestsSuite.java
- src/test/java/org/dbunit/ext/mckoi/AllTestsSuite.java
- src/test/java/org/dbunit/dataset/AllTestsSuite.java
- src/test/java/org/dbunit/ext/AllTestsSuite.java
- src/test/java/org/dbunit/operation/TruncateTableOperationIT.java
- src/test/java/org/dbunit/ext/mssql/InsertIdentityOperationIT.java
2 tasks
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
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 by CodeRabbit
Compatibility
Bug Fixes
Documentation
Chores