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/04 15:06:09 UTC
[PR] test: migrate histogram extension tests to JUnit 5 (druid)
FrankChen021 opened a new pull request, #19879:
URL: https://github.com/apache/druid/pull/19879
## Description
Migrates the `extensions-core/histogram` test suite from JUnit 4 to JUnit 5.
This is Batch 5 of the extensions-core migration and is part of [#13948](https://github.com/apache/druid/issues/13948).
Depends on the earlier migration batches in [#19875](https://github.com/apache/druid/pull/19875), [#19876](https://github.com/apache/druid/pull/19876), [#19877](https://github.com/apache/druid/pull/19877), and [#19878](https://github.com/apache/druid/pull/19878).
The shared `AggregationTestHelper` now also accepts a JUnit 5 `@TempDir` directory while retaining the existing JUnit 4-compatible API for remaining migration batches.
## Tests
```text
mvn -ntp test -pl extensions-core/histogram -am \
-Dtest='org.apache.druid.query.aggregation.histogram.**' \
-Dsurefire.failIfNoSpecifiedTests=false \
-Pskip-static-checks -Dweb.console.skip=true -T1C
```
Result: 211 tests passed; 0 failures, errors, or skips.
--
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: migrate histogram extension tests to JUnit 5 (druid)
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #19879:
URL: https://github.com/apache/druid/pull/19879#discussion_r3717425171
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramTopNQueryTest.java:
##########
@@ -36,46 +36,45 @@
import org.apache.druid.query.topn.TopNResultValue;
import org.apache.druid.segment.TestHelper;
import org.apache.druid.testing.InitializedNullHandlingTest;
-import org.junit.AfterClass;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
-@RunWith(Parameterized.class)
public class FixedBucketsHistogramTopNQueryTest extends InitializedNullHandlingTest
{
private static final Closer RESOURCE_CLOSER = Closer.create();
- @AfterClass
+ @AfterAll
public static void teardown() throws IOException
{
RESOURCE_CLOSER.close();
}
- @Parameterized.Parameters(name = "{0}")
public static Iterable<Object[]> constructorFeeder()
{
return QueryRunnerTestHelper.transformToConstructionFeeder(TopNQueryRunnerTest.queryRunners(true));
}
- private final QueryRunner runner;
+ private QueryRunner runner;
- public FixedBucketsHistogramTopNQueryTest(
+ public void initFixedBucketsHistogramTopNQueryTest(
QueryRunner runner
)
Review Comment:
The `runner` field and `initFixedBucketsHistogramTopNQueryTest` method are redundant: the test uses the method parameter `runner` (which also shadows the field), so the field assignment adds noise and can confuse readers.
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramGroupByQueryTest.java:
##########
@@ -37,45 +37,44 @@
import org.apache.druid.query.groupby.orderby.OrderByColumnSpec;
import org.apache.druid.segment.TestHelper;
import org.apache.druid.testing.InitializedNullHandlingTest;
-import org.junit.After;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
/**
*/
-@RunWith(Parameterized.class)
public class FixedBucketsHistogramGroupByQueryTest extends InitializedNullHandlingTest
{
private static final Closer RESOURCE_CLOSER = Closer.create();
private static TestGroupByBuffers BUFFER_POOLS = null;
- private final QueryRunner<ResultRow> runner;
- private final GroupByQueryRunnerFactory factory;
+ private QueryRunner<ResultRow> runner;
+ private GroupByQueryRunnerFactory factory;
Review Comment:
These instance fields appear unused after migrating to Jupiter parameterized tests: the test methods take `factory`/`runner` as parameters and do not reference `this.factory` / `this.runner`. Keeping them (and the `init...` method) adds dead state and obscures the test setup.
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramTopNQueryTest.java:
##########
@@ -36,47 +36,46 @@
import org.apache.druid.query.topn.TopNResultValue;
import org.apache.druid.segment.TestHelper;
import org.apache.druid.testing.InitializedNullHandlingTest;
-import org.junit.AfterClass;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
-@RunWith(Parameterized.class)
public class ApproximateHistogramTopNQueryTest extends InitializedNullHandlingTest
{
private static final Closer RESOURCE_CLOSER = Closer.create();
- @AfterClass
+ @AfterAll
public static void teardown() throws IOException
{
RESOURCE_CLOSER.close();
}
- @Parameterized.Parameters(name = "{0}")
public static Iterable<Object[]> constructorFeeder()
{
// Don't test with non-time-ordered segments, because results vary too much due to order-dependence.
return QueryRunnerTestHelper.transformToConstructionFeeder(TopNQueryRunnerTest.queryRunners(false));
}
- private final QueryRunner runner;
+ private QueryRunner runner;
- public ApproximateHistogramTopNQueryTest(
+ public void initApproximateHistogramTopNQueryTest(
QueryRunner runner
)
Review Comment:
The `runner` field and `initApproximateHistogramTopNQueryTest` method are redundant: the test uses the method parameter `runner` (which shadows the field), so the field assignment is dead code and makes the test harder to follow.
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramGroupByQueryTest.java:
##########
@@ -37,45 +37,44 @@
import org.apache.druid.query.groupby.orderby.OrderByColumnSpec;
import org.apache.druid.segment.TestHelper;
import org.apache.druid.testing.InitializedNullHandlingTest;
-import org.junit.After;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
/**
*/
-@RunWith(Parameterized.class)
public class ApproximateHistogramGroupByQueryTest extends InitializedNullHandlingTest
{
private static final Closer RESOURCE_CLOSER = Closer.create();
private static TestGroupByBuffers BUFFER_POOLS = null;
- private final QueryRunner<ResultRow> runner;
- private final GroupByQueryRunnerFactory factory;
+ private QueryRunner<ResultRow> runner;
+ private GroupByQueryRunnerFactory factory;
Review Comment:
These instance fields are no longer needed after switching to Jupiter parameterized tests: the test methods already receive `factory`/`runner` as parameters and do not read `this.factory` / `this.runner`. Keeping the fields plus the `init...` method adds dead state and makes the setup harder to follow.
--
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: migrate histogram extension tests to JUnit 5 (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #19879:
URL: https://github.com/apache/druid/pull/19879#discussion_r3717641807
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramTopNQueryTest.java:
##########
@@ -36,46 +36,45 @@
import org.apache.druid.query.topn.TopNResultValue;
import org.apache.druid.segment.TestHelper;
import org.apache.druid.testing.InitializedNullHandlingTest;
-import org.junit.AfterClass;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
-@RunWith(Parameterized.class)
public class FixedBucketsHistogramTopNQueryTest extends InitializedNullHandlingTest
{
private static final Closer RESOURCE_CLOSER = Closer.create();
- @AfterClass
+ @AfterAll
public static void teardown() throws IOException
{
RESOURCE_CLOSER.close();
}
- @Parameterized.Parameters(name = "{0}")
public static Iterable<Object[]> constructorFeeder()
{
return QueryRunnerTestHelper.transformToConstructionFeeder(TopNQueryRunnerTest.queryRunners(true));
}
- private final QueryRunner runner;
+ private QueryRunner runner;
- public FixedBucketsHistogramTopNQueryTest(
+ public void initFixedBucketsHistogramTopNQueryTest(
QueryRunner runner
)
Review Comment:
Thanks for flagging this. This was actionable and is already addressed by commit `1eb26ec9d8`: the redundant `runner` field and `initFixedBucketsHistogramTopNQueryTest` method were removed, and the parameterized test uses its `runner` argument directly. The four affected histogram test classes passed (80 tests, 0 failures), with Checkstyle and SpotBugs reporting 0 violations/errors.
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramTopNQueryTest.java:
##########
@@ -36,47 +36,46 @@
import org.apache.druid.query.topn.TopNResultValue;
import org.apache.druid.segment.TestHelper;
import org.apache.druid.testing.InitializedNullHandlingTest;
-import org.junit.AfterClass;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
-@RunWith(Parameterized.class)
public class ApproximateHistogramTopNQueryTest extends InitializedNullHandlingTest
{
private static final Closer RESOURCE_CLOSER = Closer.create();
- @AfterClass
+ @AfterAll
public static void teardown() throws IOException
{
RESOURCE_CLOSER.close();
}
- @Parameterized.Parameters(name = "{0}")
public static Iterable<Object[]> constructorFeeder()
{
// Don't test with non-time-ordered segments, because results vary too much due to order-dependence.
return QueryRunnerTestHelper.transformToConstructionFeeder(TopNQueryRunnerTest.queryRunners(false));
}
- private final QueryRunner runner;
+ private QueryRunner runner;
- public ApproximateHistogramTopNQueryTest(
+ public void initApproximateHistogramTopNQueryTest(
QueryRunner runner
)
Review Comment:
Thanks for flagging this. This was actionable and is already addressed by commit `1eb26ec9d8`: the redundant `runner` field and `initApproximateHistogramTopNQueryTest` method were removed, and the parameterized test uses its `runner` argument directly. The four affected histogram test classes passed (80 tests, 0 failures), with Checkstyle and SpotBugs reporting 0 violations/errors.
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramGroupByQueryTest.java:
##########
@@ -37,45 +37,44 @@
import org.apache.druid.query.groupby.orderby.OrderByColumnSpec;
import org.apache.druid.segment.TestHelper;
import org.apache.druid.testing.InitializedNullHandlingTest;
-import org.junit.After;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
/**
*/
-@RunWith(Parameterized.class)
public class FixedBucketsHistogramGroupByQueryTest extends InitializedNullHandlingTest
{
private static final Closer RESOURCE_CLOSER = Closer.create();
private static TestGroupByBuffers BUFFER_POOLS = null;
- private final QueryRunner<ResultRow> runner;
- private final GroupByQueryRunnerFactory factory;
+ private QueryRunner<ResultRow> runner;
+ private GroupByQueryRunnerFactory factory;
Review Comment:
Thanks for flagging this. This was actionable and is already addressed by commit `1eb26ec9d8`: the unused instance `factory`/`runner` fields and `initFixedBucketsHistogramGroupByQueryTest` method were removed. The parameterized method still receives the feeder values and uses the `runner` argument; SerDe registration is now performed directly in each test body. The four affected histogram test classes passed (80 tests, 0 failures), with Checkstyle and SpotBugs reporting 0 violations/errors.
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramGroupByQueryTest.java:
##########
@@ -37,45 +37,44 @@
import org.apache.druid.query.groupby.orderby.OrderByColumnSpec;
import org.apache.druid.segment.TestHelper;
import org.apache.druid.testing.InitializedNullHandlingTest;
-import org.junit.After;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
/**
*/
-@RunWith(Parameterized.class)
public class ApproximateHistogramGroupByQueryTest extends InitializedNullHandlingTest
{
private static final Closer RESOURCE_CLOSER = Closer.create();
private static TestGroupByBuffers BUFFER_POOLS = null;
- private final QueryRunner<ResultRow> runner;
- private final GroupByQueryRunnerFactory factory;
+ private QueryRunner<ResultRow> runner;
+ private GroupByQueryRunnerFactory factory;
Review Comment:
Thanks for flagging this. This was actionable and is already addressed by commit `1eb26ec9d8`: the unused instance `factory`/`runner` fields and `initApproximateHistogramGroupByQueryTest` method were removed. The parameterized method still receives the feeder values and uses the `runner` argument; SerDe registration is now performed directly in each test body. The four affected histogram test classes passed (80 tests, 0 failures), with Checkstyle and SpotBugs reporting 0 violations/errors.
--
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: migrate histogram extension tests to JUnit 5 (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #19879:
URL: https://github.com/apache/druid/pull/19879#discussion_r3718169035
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramGroupByQueryTest.java:
##########
@@ -110,26 +106,21 @@
return constructors;
}
- public ApproximateHistogramGroupByQueryTest(
- String testName,
- GroupByQueryRunnerFactory factory,
- QueryRunner runner
- )
- {
- this.factory = factory;
- this.runner = runner;
- ApproximateHistogramDruidModule.registerSerde();
- }
-
- @After
+ @AfterEach
public void teardown() throws IOException
{
RESOURCE_CLOSER.close();
}
- @Test
- public void testGroupByWithApproximateHistogramAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithApproximateHistogramAgg(
+ String testName,
+ GroupByQueryRunnerFactory factory,
Review Comment:
Fixed in commit 4872988e13. The factory remains local to constructorFeeder, where it is required to create runner, but it is no longer included in the MethodSource argument array or parameterized test signature. The affected histogram parameterized tests passed 40 tests with 0 failures; Checkstyle and SpotBugs passed with 0 violations/errors.
--
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: migrate histogram extension tests to JUnit 5 (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 merged PR #19879:
URL: https://github.com/apache/druid/pull/19879
--
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: migrate histogram extension tests to JUnit 5 (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #19879:
URL: https://github.com/apache/druid/pull/19879#discussion_r3718171796
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramGroupByQueryTest.java:
##########
@@ -191,51 +180,59 @@
TestHelper.assertExpectedObjects(expectedResults, results, "fixed-histo");
}
- @Test(expected = IllegalArgumentException.class)
- public void testGroupByWithSameNameComplexPostAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithSameNameComplexPostAgg(
+ String testName,
Review Comment:
This is intentional rather than an unused parameter. Each test is annotated @ParameterizedTest(name = "{0}"), so JUnit Jupiter consumes the first MethodSource argument as the invocation display name even though the Java method body does not reference the parameter. Removing testName would change the parameterized invocation shape and lose the descriptive display names. The affected histogram parameterized tests passed 40 tests with 0 failures; Checkstyle and SpotBugs passed with 0 violations/errors.
--
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: migrate histogram extension tests to JUnit 5 (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #19879:
URL: https://github.com/apache/druid/pull/19879#discussion_r3718171093
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramGroupByQueryTest.java:
##########
@@ -110,26 +106,21 @@
return constructors;
}
- public ApproximateHistogramGroupByQueryTest(
- String testName,
- GroupByQueryRunnerFactory factory,
- QueryRunner runner
- )
- {
- this.factory = factory;
- this.runner = runner;
- ApproximateHistogramDruidModule.registerSerde();
- }
-
- @After
+ @AfterEach
public void teardown() throws IOException
{
RESOURCE_CLOSER.close();
}
- @Test
- public void testGroupByWithApproximateHistogramAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithApproximateHistogramAgg(
+ String testName,
Review Comment:
This is intentional rather than an unused parameter. Each test is annotated @ParameterizedTest(name = "{0}"), so JUnit Jupiter consumes the first MethodSource argument as the invocation display name even though the Java method body does not reference the parameter. Removing testName would change the parameterized invocation shape and lose the descriptive display names. The affected histogram parameterized tests passed 40 tests with 0 failures; Checkstyle and SpotBugs passed with 0 violations/errors.
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramGroupByQueryTest.java:
##########
@@ -189,51 +180,59 @@
TestHelper.assertExpectedObjects(expectedResults, results, "approx-histo");
}
- @Test(expected = IllegalArgumentException.class)
- public void testGroupByWithSameNameComplexPostAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithSameNameComplexPostAgg(
+ String testName,
Review Comment:
This is intentional rather than an unused parameter. Each test is annotated @ParameterizedTest(name = "{0}"), so JUnit Jupiter consumes the first MethodSource argument as the invocation display name even though the Java method body does not reference the parameter. Removing testName would change the parameterized invocation shape and lose the descriptive display names. The affected histogram parameterized tests passed 40 tests with 0 failures; Checkstyle and SpotBugs passed with 0 violations/errors.
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramGroupByQueryTest.java:
##########
@@ -110,28 +106,21 @@
return constructors;
}
- @SuppressWarnings("unused")
- public FixedBucketsHistogramGroupByQueryTest(
- String testName,
- GroupByQueryRunnerFactory factory,
- QueryRunner runner
- )
- {
- this.factory = factory;
- this.runner = runner;
-
- ApproximateHistogramDruidModule.registerSerde();
- }
-
- @After
+ @AfterEach
public void teardown() throws IOException
{
RESOURCE_CLOSER.close();
}
- @Test
- public void testGroupByWithFixedHistogramAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithFixedHistogramAgg(
+ String testName,
Review Comment:
This is intentional rather than an unused parameter. Each test is annotated @ParameterizedTest(name = "{0}"), so JUnit Jupiter consumes the first MethodSource argument as the invocation display name even though the Java method body does not reference the parameter. Removing testName would change the parameterized invocation shape and lose the descriptive display names. The affected histogram parameterized tests passed 40 tests with 0 failures; Checkstyle and SpotBugs passed with 0 violations/errors.
--
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: migrate histogram extension 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 #19879:
URL: https://github.com/apache/druid/pull/19879#discussion_r3718295283
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramGroupByQueryTest.java:
##########
@@ -103,33 +99,27 @@
config.toString(),
runner.toString()
);
- constructors.add(new Object[]{testName, factory, runner});
+ constructors.add(new Object[]{testName, runner});
}
}
return constructors;
}
- public ApproximateHistogramGroupByQueryTest(
- String testName,
- GroupByQueryRunnerFactory factory,
- QueryRunner runner
- )
- {
- this.factory = factory;
- this.runner = runner;
- ApproximateHistogramDruidModule.registerSerde();
- }
-
- @After
+ @AfterEach
public void teardown() throws IOException
{
RESOURCE_CLOSER.close();
}
- @Test
- public void testGroupByWithApproximateHistogramAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithApproximateHistogramAgg(
+ String testName,
Review Comment:
## CodeQL / Useless parameter
The parameter 'testName' is never used.
[Show more details](https://github.com/apache/druid/security/code-scanning/11628)
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramGroupByQueryTest.java:
##########
@@ -189,51 +179,58 @@
TestHelper.assertExpectedObjects(expectedResults, results, "approx-histo");
}
- @Test(expected = IllegalArgumentException.class)
- public void testGroupByWithSameNameComplexPostAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithSameNameComplexPostAgg(
+ String testName,
Review Comment:
## CodeQL / Useless parameter
The parameter 'testName' is never used.
[Show more details](https://github.com/apache/druid/security/code-scanning/11627)
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramGroupByQueryTest.java:
##########
@@ -191,51 +179,58 @@
TestHelper.assertExpectedObjects(expectedResults, results, "fixed-histo");
}
- @Test(expected = IllegalArgumentException.class)
- public void testGroupByWithSameNameComplexPostAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithSameNameComplexPostAgg(
+ String testName,
Review Comment:
## CodeQL / Useless parameter
The parameter 'testName' is never used.
[Show more details](https://github.com/apache/druid/security/code-scanning/11629)
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramGroupByQueryTest.java:
##########
@@ -103,35 +99,27 @@
config.toString(),
runner.toString()
);
- constructors.add(new Object[]{testName, factory, runner});
+ constructors.add(new Object[]{testName, runner});
}
}
return constructors;
}
- @SuppressWarnings("unused")
- public FixedBucketsHistogramGroupByQueryTest(
- String testName,
- GroupByQueryRunnerFactory factory,
- QueryRunner runner
- )
- {
- this.factory = factory;
- this.runner = runner;
-
- ApproximateHistogramDruidModule.registerSerde();
- }
-
- @After
+ @AfterEach
public void teardown() throws IOException
{
RESOURCE_CLOSER.close();
}
- @Test
- public void testGroupByWithFixedHistogramAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithFixedHistogramAgg(
+ String testName,
Review Comment:
## CodeQL / Useless parameter
The parameter 'testName' is never used.
[Show more details](https://github.com/apache/druid/security/code-scanning/11630)
--
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: migrate histogram extension 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 #19879:
URL: https://github.com/apache/druid/pull/19879#discussion_r3717686865
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramGroupByQueryTest.java:
##########
@@ -110,26 +106,21 @@
return constructors;
}
- public ApproximateHistogramGroupByQueryTest(
- String testName,
- GroupByQueryRunnerFactory factory,
- QueryRunner runner
- )
- {
- this.factory = factory;
- this.runner = runner;
- ApproximateHistogramDruidModule.registerSerde();
- }
-
- @After
+ @AfterEach
public void teardown() throws IOException
{
RESOURCE_CLOSER.close();
}
- @Test
- public void testGroupByWithApproximateHistogramAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithApproximateHistogramAgg(
+ String testName,
+ GroupByQueryRunnerFactory factory,
Review Comment:
## CodeQL / Useless parameter
The parameter 'factory' is never used.
[Show more details](https://github.com/apache/druid/security/code-scanning/11611)
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramGroupByQueryTest.java:
##########
@@ -189,51 +180,59 @@
TestHelper.assertExpectedObjects(expectedResults, results, "approx-histo");
}
- @Test(expected = IllegalArgumentException.class)
- public void testGroupByWithSameNameComplexPostAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithSameNameComplexPostAgg(
+ String testName,
+ GroupByQueryRunnerFactory factory,
Review Comment:
## CodeQL / Useless parameter
The parameter 'factory' is never used.
[Show more details](https://github.com/apache/druid/security/code-scanning/11609)
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramGroupByQueryTest.java:
##########
@@ -110,28 +106,21 @@
return constructors;
}
- @SuppressWarnings("unused")
- public FixedBucketsHistogramGroupByQueryTest(
- String testName,
- GroupByQueryRunnerFactory factory,
- QueryRunner runner
- )
- {
- this.factory = factory;
- this.runner = runner;
-
- ApproximateHistogramDruidModule.registerSerde();
- }
-
- @After
+ @AfterEach
public void teardown() throws IOException
{
RESOURCE_CLOSER.close();
}
- @Test
- public void testGroupByWithFixedHistogramAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithFixedHistogramAgg(
+ String testName,
Review Comment:
## CodeQL / Useless parameter
The parameter 'testName' is never used.
[Show more details](https://github.com/apache/druid/security/code-scanning/11614)
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramGroupByQueryTest.java:
##########
@@ -110,26 +106,21 @@
return constructors;
}
- public ApproximateHistogramGroupByQueryTest(
- String testName,
- GroupByQueryRunnerFactory factory,
- QueryRunner runner
- )
- {
- this.factory = factory;
- this.runner = runner;
- ApproximateHistogramDruidModule.registerSerde();
- }
-
- @After
+ @AfterEach
public void teardown() throws IOException
{
RESOURCE_CLOSER.close();
}
- @Test
- public void testGroupByWithApproximateHistogramAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithApproximateHistogramAgg(
+ String testName,
Review Comment:
## CodeQL / Useless parameter
The parameter 'testName' is never used.
[Show more details](https://github.com/apache/druid/security/code-scanning/11610)
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramGroupByQueryTest.java:
##########
@@ -191,51 +180,59 @@
TestHelper.assertExpectedObjects(expectedResults, results, "fixed-histo");
}
- @Test(expected = IllegalArgumentException.class)
- public void testGroupByWithSameNameComplexPostAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithSameNameComplexPostAgg(
+ String testName,
Review Comment:
## CodeQL / Useless parameter
The parameter 'testName' is never used.
[Show more details](https://github.com/apache/druid/security/code-scanning/11612)
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramGroupByQueryTest.java:
##########
@@ -189,51 +180,59 @@
TestHelper.assertExpectedObjects(expectedResults, results, "approx-histo");
}
- @Test(expected = IllegalArgumentException.class)
- public void testGroupByWithSameNameComplexPostAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithSameNameComplexPostAgg(
+ String testName,
Review Comment:
## CodeQL / Useless parameter
The parameter 'testName' is never used.
[Show more details](https://github.com/apache/druid/security/code-scanning/11608)
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramGroupByQueryTest.java:
##########
@@ -191,51 +180,59 @@
TestHelper.assertExpectedObjects(expectedResults, results, "fixed-histo");
}
- @Test(expected = IllegalArgumentException.class)
- public void testGroupByWithSameNameComplexPostAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithSameNameComplexPostAgg(
+ String testName,
+ GroupByQueryRunnerFactory factory,
Review Comment:
## CodeQL / Useless parameter
The parameter 'factory' is never used.
[Show more details](https://github.com/apache/druid/security/code-scanning/11613)
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramGroupByQueryTest.java:
##########
@@ -110,28 +106,21 @@
return constructors;
}
- @SuppressWarnings("unused")
- public FixedBucketsHistogramGroupByQueryTest(
- String testName,
- GroupByQueryRunnerFactory factory,
- QueryRunner runner
- )
- {
- this.factory = factory;
- this.runner = runner;
-
- ApproximateHistogramDruidModule.registerSerde();
- }
-
- @After
+ @AfterEach
public void teardown() throws IOException
{
RESOURCE_CLOSER.close();
}
- @Test
- public void testGroupByWithFixedHistogramAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithFixedHistogramAgg(
+ String testName,
+ GroupByQueryRunnerFactory factory,
Review Comment:
## CodeQL / Useless parameter
The parameter 'factory' is never used.
[Show more details](https://github.com/apache/druid/security/code-scanning/11615)
--
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: migrate histogram extension tests to JUnit 5 (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #19879:
URL: https://github.com/apache/druid/pull/19879#discussion_r3718170226
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramGroupByQueryTest.java:
##########
@@ -189,51 +180,59 @@
TestHelper.assertExpectedObjects(expectedResults, results, "approx-histo");
}
- @Test(expected = IllegalArgumentException.class)
- public void testGroupByWithSameNameComplexPostAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithSameNameComplexPostAgg(
+ String testName,
+ GroupByQueryRunnerFactory factory,
Review Comment:
Fixed in commit 4872988e13. The factory remains local to constructorFeeder, where it is required to create runner, but it is no longer included in the MethodSource argument array or parameterized test signature. The affected histogram parameterized tests passed 40 tests with 0 failures; Checkstyle and SpotBugs passed with 0 violations/errors.
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramGroupByQueryTest.java:
##########
@@ -110,28 +106,21 @@
return constructors;
}
- @SuppressWarnings("unused")
- public FixedBucketsHistogramGroupByQueryTest(
- String testName,
- GroupByQueryRunnerFactory factory,
- QueryRunner runner
- )
- {
- this.factory = factory;
- this.runner = runner;
-
- ApproximateHistogramDruidModule.registerSerde();
- }
-
- @After
+ @AfterEach
public void teardown() throws IOException
{
RESOURCE_CLOSER.close();
}
- @Test
- public void testGroupByWithFixedHistogramAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithFixedHistogramAgg(
+ String testName,
+ GroupByQueryRunnerFactory factory,
Review Comment:
Fixed in commit 4872988e13. The factory remains local to constructorFeeder, where it is required to create runner, but it is no longer included in the MethodSource argument array or parameterized test signature. The affected histogram parameterized tests passed 40 tests with 0 failures; Checkstyle and SpotBugs passed with 0 violations/errors.
##########
extensions-core/histogram/src/test/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramGroupByQueryTest.java:
##########
@@ -191,51 +180,59 @@
TestHelper.assertExpectedObjects(expectedResults, results, "fixed-histo");
}
- @Test(expected = IllegalArgumentException.class)
- public void testGroupByWithSameNameComplexPostAgg()
+ @MethodSource("constructorFeeder")
+ @ParameterizedTest(name = "{0}")
+ public void testGroupByWithSameNameComplexPostAgg(
+ String testName,
+ GroupByQueryRunnerFactory factory,
Review Comment:
Fixed in commit 4872988e13. The factory remains local to constructorFeeder, where it is required to create runner, but it is no longer included in the MethodSource argument array or parameterized test signature. The affected histogram parameterized tests passed 40 tests with 0 failures; Checkstyle and SpotBugs passed with 0 violations/errors.
--
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