test: migrate server tests to JUnit 5 - #19911
Conversation
|
imo a 12k line PR is kind of hard to review, not to mention disruptive to any open PR that touches a test in the entire module (this is part of why i have broken up druid-processing into several smaller migrations) |
You are right. I didn't expect such a large change. I already asked Codex to analyze and split changes into multiple PRs based on modules. Codex is still monitoring the CI status of these PRs and will apply fixes automatically until CI is green. After that let's split the changes into multiple PRs again. |
Expanded JUnit 5 migration scope and evidenceThis update preserves the existing server migration in #19911 and adds the dependency-coupled Shared server test fixtures consumed by these modules, including Scoped validation
Known environment/baseline blockers
This PR is part of issue #13948 and is related to #19875, #19908, #19909, and #19910. Published commit: |
|
CI follow-up for #13948 CI exposed that Validation:
Commit: |
CI dependency updateThe fresh run on head 64f2e3c shows the helper boundary is fixed: no missing TempDirExtension or CliPeonTest type errors remain. The remaining indexing-service test errors are the expected merge-order boundary: unchanged indexing tests on master still annotate the server TestDerbyConnector JUnit5 callback with JUnit4 @rule, producing InvalidTestClass errors. SQL QTest jobs independently stop at the known temporaryFolder.newFolder() calls in the un-migrated SQL batch. Please merge/rebase #19910 before evaluating this PR’s full reactor checks; #19910 owns the indexing consumers, while #19911 owns the server/services/embedded fixture migration. No compatibility dependency was reintroduced. |
Summary
server/pom.xml.GroupByQueryRunnerTest.apache/druid/master; the complete diff is limited toserver/**(411 files).Validation
mvn -ntp -pl server test-compile -DskipTests -Dweb.console.skip=true— passed.QuerySchedulerTest(22) andDirectDruidClientTest(12) — passed.mvn -ntp -pl server spotbugs:check -DskipTests -Dweb.console.skip=true— passed; 0 BugInstances, 0 errors.junit:junit, Vintage, JUnit migration-support, Hamcrest, Jersey test-framework, or Truth artifacts../dev/validate-junit5-usage— passed; server reportshasj4: 0andhasVintage: 0.git diff --check— passed.Shared fixture stacking prerequisite
#19875 currently contains older copies of:
server/src/test/java/org/apache/druid/server/QueryStackTests.javaserver/src/test/java/org/apache/druid/metadata/TestDerbyConnector.javaThis PR is the sole owner of the finalized server versions. Before merging either PR, rebase #19875 to drop those two server paths (or apply the equivalent stack/rebase ordering); do not retain divergent copies in both PRs.
The server
CachingClusteredClientTestalso consumesprocessing/src/test/java/org/apache/druid/segment/TestHelper.java. On current master that shared fixture still calls JUnit 4org.junit.Assert; with the cleaned server classpath, the test currently fails withNoClassDefFoundError: org/junit/Assert(262 tests attempted, 150 errors). #19875 contains the JUnit 5 processing-fixture migration. After #19875 drops its two older server paths, its processing fixture changes must land or be stacked/rebased beforeCachingClusteredClientTestcan run. This is the narrow prerequisite; this PR intentionally retains no JUnit 4/Hamcrest compatibility dependencies.Part of #13948
Prior migration PRs: #19875, #19876, #19877, #19878, #19879, #19880.