Skip to content

test(indexing-service): migrate tests to JUnit 5 - #19910

Draft
FrankChen021 wants to merge 6 commits into
apache:masterfrom
FrankChen021:codex/tmp-junit5-indexing-services
Draft

test(indexing-service): migrate tests to JUnit 5#19910
FrankChen021 wants to merge 6 commits into
apache:masterfrom
FrankChen021:codex/tmp-junit5-indexing-services

Conversation

@FrankChen021

@FrankChen021 FrankChen021 commented Aug 6, 2026

Copy link
Copy Markdown
Member

Summary

  • Migrate the remaining indexing-service tests to JUnit 5.
  • Migrate indexing-service consumers of the shared DruidException test matchers to Hamcrest-free Jupiter assertions.
  • Remove direct JUnit 4, Vintage, migration-support, JUnitParams, system-rules, and Hamcrest dependencies that are no longer needed.

This PR is strictly scoped to indexing-service/**. The services/** migration was removed from this branch and is owned by #19911, because services consumes server test-jar fixtures. This PR does not modify processing, server, SQL, MSQ, services, or extensions.

Validation

  • Full reactor indexing-service test compilation passed.
  • Focused validation passed 316 tests with 0 final failures or errors. One pre-existing timing-sensitive test flaked once, passed on Surefire retry, and passed again in isolation.
  • Checkstyle: 0 violations.
  • PMD, SpotBugs, Enforcer, and forbidden APIs: passed with no findings.
  • Indexing-service source/POM audit: zero DruidExceptionMatcher, ExceptionMatcher, JUnit4, or Hamcrest references.
  • Indexing-service test dependency audit: no Hamcrest, JUnit4, or Vintage artifacts.
  • git diff --check: passed.

Tracking

Part of #13948.

Related migration PRs: #19875, #19908, #19909, and #19911.

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

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 wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

@FrankChen021

Copy link
Copy Markdown
Member Author

Follow-up audit and cleanup pushed in commit f0d72c8c23.

  • Removed the unused test-scoped com.github.stefanbirkner:system-rules declaration from indexing-service/pom.xml only.
  • Audited indexing-service and services: no source imports or fully qualified references to org.junit.contrib.java.lang.system, other JUnit 4 APIs, or Hamcrest APIs remain; no stale direct JUnit 4, Vintage, JUnit migration-support, Hamcrest, or system-rules declarations remain in those module POMs.
  • Confirmed the root dependency-management entry and Java security-manager configuration were left unchanged because server/pom.xml still has the direct system-rules declaration. Shared root/server cleanup is deferred to a final dependency-cleanup PR after that ownership is removed.
  • Validation passed: mvn test-compile -pl indexing-service,services -am -Pskip-static-checks -Dweb.console.skip=true -T1C (including checkstyle and test compilation).

@FrankChen021

Copy link
Copy Markdown
Member Author

Fixed and pushed as 5cfd2c5.

CI failure: testCompile could not access org.hamcrest.DiagnosingMatcher and org.hamcrest.Matcher at SegmentTransactionalInsertActionTest.java:234 and ConcurrentReplaceAndAppendTest.java:1213 after PR #19875 removed Hamcrest from the shared processing test classpath.

The two requested files now use JUnit5Assertions, JUnit5Matchers, and direct DruidException field assertions. No dependencies or other modules changed.

Validation:

  • Focused source compilation of both changed tests and test-source dependencies passed.
  • Surefire passed: 36 tests, 0 failures/errors (32 ConcurrentReplaceAndAppendTest; 4 SegmentTransactionalInsertActionTest).
  • mvn -pl indexing-service -am -DskipTests -Dweb.console.skip=true -T1C validate passed: Checkstyle 0 violations; PMD and Enforcer passed.
  • mvn -pl indexing-service -am -DskipTests -Dweb.console.skip=true -T1C compile passed: forbidden-API scans 0 errors.
  • Indexing-service audit found no JUnit4/Hamcrest imports or direct dependency declarations; git diff --check passed.

The full reactor testCompile on this standalone checkout still stops at inherited KillUnusedSegmentsTaskTest.java:407 because PR #19875 custom processing matcher sources are not present locally. That unrelated file/module was not modified.

@FrankChen021

Copy link
Copy Markdown
Member Author

Follow-up pushed as c4c7fcaffc.

Audited all indexing-service test consumers of org.apache.druid.error.ExceptionMatcher (the remaining consumers were KillUnusedSegmentsTaskTest and GlobalTaskLockboxTest) and migrated both to JUnit5Assertions plus JUnit5Matchers. No Hamcrest or JUnit 4 dependency was added, and no other module was changed.

Validation:

  • mvn -pl indexing-service -am -DskipTests test-compile -Pskip-static-checks -Dweb.console.skip=true -T1C: this standalone checkout now advances past the former ExceptionMatcher failures, then stops at the inherited KillUnusedSegmentsTaskTest.java:1290 DruidExceptionMatcher fixture. That custom processing fixture is supplied by PR test: migrate extensions-core and processing tests to JUnit 5 #19875 but is not present in this checkout; no new follow-up failure occurs in either migrated test.
  • Focused Surefire coverage: 45 tests passed, 0 failures, 0 errors across the affected tests and related existing fixtures. A temporary Hamcrest runtime classpath was used only for the two stale pre-test: migrate extensions-core and processing tests to JUnit 5 #19875 DruidExceptionMatcher fixtures; no project dependency or source reference was added.
  • mvn -pl indexing-service -am -DskipTests -Dweb.console.skip=true -T1C validate compile: passed Checkstyle, PMD/Enforcer, and forbidden-API checks.
  • Exact ExceptionMatcher source audit: 0 indexing-service test imports/usages.
  • JUnit 4/Hamcrest source audit: 0 matches in indexing-service/src/test and indexing-service/pom.xml.
  • Test dependency-tree audit for org.hamcrest:*: no indexing-service dependency.
  • git diff --check: passed.

The branch contains only the intended two follow-up test-file changes on top of the prior direct assertion fixes.

@FrankChen021

Copy link
Copy Markdown
Member Author

Follow-up pushed as 00cd6a1256.

The remaining CI failure was caused by all indexing-service test consumers of org.apache.druid.error.DruidExceptionMatcher resolving the old processing test-jar matcher, which extends Hamcrest. I migrated all 26 consumers across 11 indexing-service test classes to the existing Hamcrest-free JUnit5Matchers helper. The local matcher checks the same DruidException target persona, category, error code, and exact message. No processing, server, SQL, or contrib files were modified, and no Hamcrest dependency was restored.

Validation on the pushed tree:

  • Full reactor test-compile (mvn -pl indexing-service -am -DskipTests test-compile -Dweb.console.skip=true -T1C): passed all six modules and compiled all 258 indexing-service test sources.
  • Focused Surefire run covering the two original direct-fix tests and all 11 migrated consumer classes: 316 passed, 0 failures, 0 errors, with no additional Hamcrest classpath.
  • Non-skipped reactor validation/static goals: passed Checkstyle (0 violations), PMD, Enforcer, and forbidden-API scans (0 errors).
  • Final indexing-service test compilation: passed Checkstyle (0 violations) and test forbidden-API scan (0 errors).
  • Source audit over indexing-service/src and indexing-service/pom.xml: 0 DruidExceptionMatcher, ExceptionMatcher, JUnit4, or Hamcrest references.
  • Test dependency-tree audit for Hamcrest and JUnit4/vintage artifacts: no matches.
  • git diff --check: passed.

@FrankChen021 FrankChen021 changed the title test: migrate indexing and services tests to JUnit 5 test: migrate indexing-service tests to JUnit 5 Aug 7, 2026

@github-advanced-security github-advanced-security AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@FrankChen021 FrankChen021 changed the title test: migrate indexing-service tests to JUnit 5 test(indexing-service): migrate tests to JUnit 5 Aug 7, 2026
@FrankChen021

Copy link
Copy Markdown
Member Author

Ownership update for PR #19910 (current head 149332e673):

Validation after the scope split: full indexing-service reactor test-compile passed; focused tests finished with 316 final passes (one existing timing-sensitive test passed on retry and in isolation); Checkstyle, PMD, SpotBugs, Enforcer, and forbidden APIs passed; source and dependency audits found zero DruidExceptionMatcher/ExceptionMatcher, JUnit4, or Hamcrest references.

@FrankChen021
FrankChen021 marked this pull request as draft August 7, 2026 09:38

@FrankChen021 FrankChen021 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I have reviewed the code for correctness, edge cases, concurrency, and integration risks; no issues found.

Reviewed 232 of 232 changed files.


This is an automated review by Codex GPT-5.6-Luna(max)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants