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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
