Skip to content

test(sql): migrate SQL and Quidem tests to JUnit 5 - #19909

Open
FrankChen021 wants to merge 5 commits into
apache:masterfrom
FrankChen021:codex/tmp-junit5-sql-quidem
Open

test(sql): migrate SQL and Quidem tests to JUnit 5#19909
FrankChen021 wants to merge 5 commits into
apache:masterfrom
FrankChen021:codex/tmp-junit5-sql-quidem

Conversation

@FrankChen021

@FrankChen021 FrankChen021 commented Aug 6, 2026

Copy link
Copy Markdown
Member

Summary

  • Migrate the existing SQL and Quidem JUnit 4 tests to JUnit 5.
  • Integrate the validated MSQ JUnit 5 migration from adac0cc117 and 1869c51f68, restricted to multi-stage-query/**.
  • Make this PR own the combined SQL + Quidem + MSQ test migration boundary; no embedded-tests/** paths are included.
  • Remove MSQ's direct JUnit 4, Vintage, migration-support, JUnitParams, and Hamcrest test dependencies, and replace remaining MSQ exception assertions with Jupiter-backed DruidExceptionAssertions and local MSQ matchers.
  • Keep SQL's DruidExceptionAssertions API non-Hamcrest; MSQ now consumes that API directly.
  • Include the current-master temp-directory fixes in the SQL schema fixtures: temporaryFolder.newFolder() is replaced by the shared newTempFolder() helper backed by FileUtils.createTempDirInLocation.

Root cause

Run 31129350780 exposed the dependency boundary after the SQL migration: unchanged MSQ tests still passed SQL DruidExceptionAssertions/legacy exception matchers through Hamcrest APIs, producing errors such as DruidExceptionAssertions cannot be converted to org.hamcrest.Matcher<Throwable>. The same unresolved MSQ consumer caused the QTest shards, validate-dist, static/packaging checks, and CodeQL jobs to fail.

This PR integrates the MSQ migration without restoring JUnit 4 or Hamcrest compatibility. It intentionally does not modify embedded-tests/** or shared fixture modules outside the requested scope.

Validation

  • SQL focused schema fixtures: 24 tests passed, 0 failures/errors.
  • SQL and all MSQ sources compile through the MSQ test-compile boundary until the current master's shared indexing-service test fixture: MSQCompactionTaskRunTest still invokes TaskActionTestKit.before(), whose inherited org.junit.rules.ExternalResource is supplied by the separate shared-fixture migration. No JUnit 4 dependency was added to work around that out-of-scope prerequisite.
  • MSQ Checkstyle: passed, 0 violations.
  • MSQ SpotBugs: passed, 0 BugInstances and 0 errors.
  • Effective MSQ test dependency tree: no junit:junit, Vintage, JUnit migration-support, or Hamcrest artifacts.
  • Scoped source audit: no JUnit 4/Hamcrest imports/usages, no .newFolder()/.newFile() calls, and no embedded-tests paths.
  • git diff --check: passed.

Part of #13948.

Copilot AI lite review requested due to automatic review settings August 6, 2026 22:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@FrankChen021

Copy link
Copy Markdown
Member Author

Root cause: run 31129350780 failed because the SQL JUnit 5 migration removed the Hamcrest-returning exception API while unchanged MSQ tests still consumed it as Matcher<Throwable>. That dependency boundary caused the QTest shards plus validate-dist, static/packaging, and CodeQL failures.

Fixed in 038fbef729 by integrating the validated MSQ migration from adac0cc117 and 1869c51f68 under multi-stage-query/** only, removing MSQ's legacy JUnit/Hamcrest dependencies, and converting remaining MSQ exception assertions to Jupiter-backed helpers. The SQL schema temp-dir fixes are also present via newTempFolder(); the 24 focused SQL tests pass. MSQ Checkstyle and SpotBugs pass, and scoped audits find no JUnit 4/Hamcrest source imports or .newFolder()/.newFile() calls. The only local MSQ test-compile limitation is the separate shared indexing fixture's JUnit 4 TaskActionTestKit superclass, which is outside this PR's permitted paths and was not reintroduced as a compatibility dependency.

@github-actions github-actions Bot added Area - Batch Ingestion Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 labels Aug 6, 2026
@FrankChen021

Copy link
Copy Markdown
Member Author

Validation evidence for the MSQ processing-helper follow-up:

  • Replaced all three MSQ-owned GroupByQueryRunnerTest consumers with the JUnit 5-safe GroupByQueryRunnerTestHelper.makeQueryRunnerFactory(objectMapper, config, buffers) API.
  • Audited multi-stage-query Java sources: no GroupByQueryRunnerTest façade references remain.
  • On a disposable combined validation tree containing PR test: migrate extensions-core and processing tests to JUnit 5 #19875's processing helper and the related sibling migration changes, mvn -pl multi-stage-query test-compile -DskipTests -Pskip-static-checks -Dweb.console.skip=true -T1C passed.
  • Focused MSQTaskQueryMakerTest: 9 tests passed.
  • Checkstyle: 0 violations. SpotBugs: 0 bugs and 0 errors. Forbidden-API scan: 0 errors.
  • MSQCompactionTaskRunTest reaches test setup but is currently blocked before test bodies by the shared TaskActionTestKit.before() initialization (IllegalStateException at MSQCompactionTaskRunTest.setup:217), with the existing teardown NPE because the executor was never created. No helper compilation or API errors occur.
  • git diff --check passed; the commit changes only the three MSQ test files.

@FrankChen021

FrankChen021 commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

Self-contained JUnit 5 helper follow-up

Failure evidence from the original PR head 738ffbba9fc2e513eb1347d1fa793fedb791b0f7:

  • The QTest CI run, job 92730410803, failed druid-multi-stage-query test compilation with the exact missing API:
    MSQTestBase.java:[513,71] cannot find symbol
      symbol: method makeQueryRunnerFactory(ObjectMapper, GroupByQueryConfig, TestGroupByBuffers)
      location: class GroupByQueryRunnerTestHelper
    MSQCompactionTaskRunTest.java:[248,65] cannot find symbol
      symbol: method makeQueryRunnerFactory(ObjectMapper, GroupByQueryConfig, TestGroupByBuffers)
    MSQTaskQueryMakerTest.java:[197,50] cannot find symbol
      symbol: method makeQueryRunnerFactory(ObjectMapper, GroupByQueryConfig, TestGroupByBuffers)
    
  • The same CI compilation also reported MSQCompactionTaskRunTest.java:[217,22] cannot access org.junit.rules.ExternalResource. This is the separate shared indexing-service fixture boundary: CompactionTaskRunBase still exposes JUnit 4 TaskActionTestKit.

Fix in dd94dd93539b6f1241bb2911f7308fb7228bdd10:

  • Added the minimal JUnit 5-safe GroupByQueryRunnerTestHelper.makeQueryRunnerFactory(ObjectMapper, GroupByQueryConfig, TestGroupByBuffers) API and its test configuration constants to the existing processing test helper, following the isolated helper change from PR #19875.
  • Updated TestGroupByBuffers.createDefault() to use that helper's processing config, so the migrated consumers do not load the JUnit 4 GroupByQueryRunnerTest class.
  • No processing production changes, SQL/MSQ/Quidem POM changes, JUnit 4 or Hamcrest compatibility dependencies, or consumer refactors were added.

Validation:

  • Before the fix, mvn -ntp -pl multi-stage-query -am test-compile -DskipTests -Pskip-static-checks -Dweb.console.skip=true -T1C reproduced all three missing-method diagnostics above plus the ExternalResource diagnostic.
  • After the fix, the same command removed all three missing-method diagnostics; processing test compilation completed, and only the separate ExternalResource diagnostic at MSQCompactionTaskRunTest.java:[217,22] remains.
  • Focused tests passed: processing GroupByResourcesReservationPoolTest (3 run, 1 skipped), SQL SqlQueryPlusTest + SqlRowTransformerTest (6/6), Quidem LauncherSmokeTest (2/2), and filtered QTest qaWin/basics_group_by.all (4/4).
  • Checkstyle, PMD, SpotBugs (0 bugs/0 errors), Enforcer, dev/validate-junit5-usage, and git diff --check passed for the scoped modules. SQL/Quidem/MSQ source audits found no non-Jupiter JUnit or Hamcrest references. The only matching POM entries are pre-existing transitive-dependency exclusions; the effective dependency audit resolved none of the forbidden JUnit 4/Vintage/migration-support/Hamcrest artifacts. Processing forbidden-API test scanning reported 0 errors; the MSQ forbidden-API scan could not complete because the remaining compile blocker left the test output incomplete.

The remaining ExternalResource failure is owned by the shared indexing migration tracked in PR #19910, not by the GroupBy helper API. I left it untouched to preserve #19909's no-JUnit4/no-Hamcrest scope. This fix addresses the self-contained portion of issue #13948.

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

Labels

Area - Batch Ingestion Area - Dependencies Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 Area - Querying

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants