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: [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]

Reply via email to