You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "FrankChen021 (via GitHub)" <gi...@apache.org> on 2026/08/18 14:58:50 UTC
[PR] test(multi-stage-query): migrate tests to JUnit 5 (druid)
FrankChen021 opened a new pull request, #20064:
URL: https://github.com/apache/druid/pull/20064
Part of #13948.
### Description
Migrate the `multi-stage-query` test suite from JUnit 4 to JUnit 5 following `dev/junit5-migration-guidance.md`.
- Convert JUnit 4 tests, lifecycle annotations, assertions, parameterized tests, and Mockito setup to Jupiter APIs.
- Reuse `TemporaryFolderExtension` for instance-scoped Druid temporary directories and the shared `ThrowableMatcher`/Druid assertion helpers.
- Remove the local JUnit assertion and matcher copies that were only needed by the JUnit 4 suite.
- Remove `junit:junit`, Vintage, migration-support, and JUnitParams dependencies from `multi-stage-query`; exclude JUnit 4 from Guice testlib's transitive test dependency and add Mockito's Jupiter integration.
- Update shared test fixtures used by the MSQ tests (`TaskActionTestKit`, `MockServiceClient`, and `TestDerbyConnector`) so the migrated tests do not load JUnit 4 classes at runtime.
- Remove the JUnit 4 `AssumptionViolatedException` reference from SQL's `NotYetSupported` test extension, which was required after the MSQ test runtime stopped providing JUnit 4.
Generic Hamcrest remains in the module because a small set of tests still use its generic matchers; no JUnit 4 matcher or engine remains in the module's resolved test dependency tree.
The remaining JUnit 5 `@TempDir` usages are lifecycle-constrained: one is used by a static `@BeforeAll` setup and one is a static directory required by a superclass constructor. Other migrated instance-scoped temporary directories use `TemporaryFolderExtension`.
### Validation
- `mvn -ntp -pl multi-stage-query -am test -Dtest='org.apache.druid.msq.**,org.apache.druid.sql.avatica.MSQDruidMeta' -Dsurefire.failIfNoSpecifiedTests=false -Pskip-static-checks -Dweb.console.skip=true -T1C`
- 7,685 tests, 0 failures, 0 errors, 698 skips; one transient `DartWorkerRunnerTest` failure passed Surefire's retry.
- `mvn -ntp -pl multi-stage-query,server,indexing-service,sql -am checkstyle:check -Dweb.console.skip=true -T1C`
- `mvn -ntp -pl multi-stage-query,server,indexing-service -am spotbugs:check -Dweb.console.skip=true -T1C`
- `mvn -ntp -pl multi-stage-query dependency:tree -Dscope=test -Dincludes=junit:junit,org.junit.vintage:junit-vintage-engine -Dweb.console.skip=true`
- `git diff --check`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] test(multi-stage-query): migrate tests to JUnit 5 (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #20064:
URL: https://github.com/apache/druid/pull/20064#discussion_r3813285460
##########
multi-stage-query/src/test/java/org/apache/druid/msq/shuffle/output/ByteChunksInputStreamTest.java:
##########
@@ -46,11 +44,11 @@
int c;
while ((c = in.read()) != -1) {
- MatcherAssert.assertThat("InputStream#read contract", c, Matchers.greaterThanOrEqualTo(0));
+ Assertions.assertTrue(c >= 0, "InputStream#read contract");
Review Comment:
Addressed in f169fe9ea6. Removed the redundant c >= 0 assertion; ByteChunksInputStream.read() returns b & 0xFF, so the assertion is always true inside the c != -1 loop. The focused ByteChunksInputStreamTest (6 tests), Checkstyle, and SpotBugs all pass.
##########
multi-stage-query/src/test/java/org/apache/druid/msq/shuffle/output/ByteChunksInputStreamTest.java:
##########
@@ -62,11 +60,11 @@
int c;
while ((c = in.read()) != -1) {
- MatcherAssert.assertThat("InputStream#read contract", c, Matchers.greaterThanOrEqualTo(0));
+ Assertions.assertTrue(c >= 0, "InputStream#read contract");
Review Comment:
Addressed in f169fe9ea6. Removed the redundant c >= 0 assertion; ByteChunksInputStream.read() returns b & 0xFF, so the assertion is always true inside the c != -1 loop. The focused ByteChunksInputStreamTest (6 tests), Checkstyle, and SpotBugs all pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] test(multi-stage-query): migrate tests to JUnit 5 (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 merged PR #20064:
URL: https://github.com/apache/druid/pull/20064
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] test(multi-stage-query): migrate tests to JUnit 5 (druid)
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #20064:
URL: https://github.com/apache/druid/pull/20064#discussion_r3810716861
##########
multi-stage-query/src/test/java/org/apache/druid/msq/shuffle/output/ByteChunksInputStreamTest.java:
##########
@@ -46,11 +44,11 @@
int c;
while ((c = in.read()) != -1) {
- MatcherAssert.assertThat("InputStream#read contract", c, Matchers.greaterThanOrEqualTo(0));
+ Assertions.assertTrue(c >= 0, "InputStream#read contract");
Review Comment:
## CodeQL / Useless comparison test
Test is always true, because of [this condition](1).
[Show more details](https://github.com/apache/druid/security/code-scanning/11882)
##########
multi-stage-query/src/test/java/org/apache/druid/msq/shuffle/output/ByteChunksInputStreamTest.java:
##########
@@ -62,11 +60,11 @@
int c;
while ((c = in.read()) != -1) {
- MatcherAssert.assertThat("InputStream#read contract", c, Matchers.greaterThanOrEqualTo(0));
+ Assertions.assertTrue(c >= 0, "InputStream#read contract");
Review Comment:
## CodeQL / Useless comparison test
Test is always true, because of [this condition](1).
[Show more details](https://github.com/apache/druid/security/code-scanning/11883)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org