FrankChen021 commented on code in PR #19881:
URL: https://github.com/apache/druid/pull/19881#discussion_r3717590735
##########
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/SketchAggregationWithSimpleDataTest.java:
##########
@@ -98,20 +95,20 @@ public static Collection<?> constructorFeeder()
return constructors;
}
- @Before
+ @BeforeEach
public void setup() throws Exception
{
SketchModule.registerSerde();
sm = new SketchModule();
try (
- final AggregationTestHelper toolchest =
AggregationTestHelper.createGroupByQueryAggregationTestHelper(
+ final AggregationTestHelper toolchest =
AggregationTestHelper.createGroupByQueryAggregationTestHelperWithTempDir(
sm.getJacksonModules(),
config,
tempFolder
Review Comment:
Fixed in commit
[6d8055b3f8](https://github.com/apache/druid/commit/6d8055b3f8). I removed the
mutable / lifecycle state and changed setup to , called at the start of each of
the four methods. This ensures each parameterized invocation builds its
indexes with its own config after JUnit 5 supplies the parameters; is already
available as the test method parameter for the query
context.\n\nValidation:\n\nResult: 86 tests passed, 0 failures/errors/skips.
Checkstyle and SpotBugs also passed.
##########
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchMergeAggregatorFactoryTest.java:
##########
@@ -82,50 +83,56 @@ public void setUp()
vectorFactory = new
TestVectorColumnSelectorFactory().addCapabilities(FIELD_NAME,
columnCapabilities);
}
- @Test(expected = AggregatorFactoryNotMergeableException.class)
- public void testGetMergingFactoryBadName() throws Exception
+ @Test
+ public void testGetMergingFactoryBadName()
{
- HllSketchMergeAggregatorFactory other = new
HllSketchMergeAggregatorFactory(
- NAME + "-diff",
- FIELD_NAME,
- LG_K,
- TGT_HLL_TYPE,
- STRING_ENCODING,
- SHOULD_FINALIZE,
- ROUND
- );
- targetRound.getMergingFactory(other);
+ Assertions.assertThrows(AggregatorFactoryNotMergeableException.class, ()
-> {
+ HllSketchMergeAggregatorFactory other = new
HllSketchMergeAggregatorFactory(
+ NAME + "-diff",
+ FIELD_NAME,
+ LG_K,
+ TGT_HLL_TYPE,
+ STRING_ENCODING,
+ SHOULD_FINALIZE,
+ ROUND
+ );
+ targetRound.getMergingFactory(other);
+ });
}
- @Test(expected = AggregatorFactoryNotMergeableException.class)
- public void testGetMergingFactoryBadType() throws Exception
+ @Test
+ public void testGetMergingFactoryBadType()
{
- HllSketchBuildAggregatorFactory other = new
HllSketchBuildAggregatorFactory(
- NAME,
- FIELD_NAME,
- LG_K,
- TGT_HLL_TYPE,
- STRING_ENCODING,
- SHOULD_FINALIZE,
- ROUND
- );
- targetRound.getMergingFactory(other);
+ Assertions.assertThrows(AggregatorFactoryNotMergeableException.class, ()
-> {
+ HllSketchBuildAggregatorFactory other = new
HllSketchBuildAggregatorFactory(
+ NAME,
+ FIELD_NAME,
+ LG_K,
+ TGT_HLL_TYPE,
+ STRING_ENCODING,
+ SHOULD_FINALIZE,
+ ROUND
+ );
+ targetRound.getMergingFactory(other);
+ });
}
- @Test(expected = AggregatorFactoryNotMergeableException.class)
- public void testGetMergingFactoryDifferentStringEncoding() throws Exception
+ @Test
+ public void testGetMergingFactoryDifferentStringEncoding()
{
- HllSketchMergeAggregatorFactory other = new
HllSketchMergeAggregatorFactory(
- NAME,
- FIELD_NAME,
- LG_K,
- TGT_HLL_TYPE,
- StringEncoding.UTF8,
- SHOULD_FINALIZE,
- ROUND
- );
- HllSketchAggregatorFactory result = (HllSketchAggregatorFactory)
targetRound.getMergingFactory(other);
- Assert.assertEquals(LG_K, result.getLgK());
+ Assertions.assertThrows(AggregatorFactoryNotMergeableException.class, ()
-> {
+ HllSketchMergeAggregatorFactory other = new
HllSketchMergeAggregatorFactory(
+ NAME,
+ FIELD_NAME,
+ LG_K,
+ TGT_HLL_TYPE,
+ StringEncoding.UTF8,
+ SHOULD_FINALIZE,
+ ROUND
+ );
+ HllSketchAggregatorFactory result = (HllSketchAggregatorFactory)
targetRound.getMergingFactory(other);
+ Assertions.assertEquals(LG_K, result.getLgK());
Review Comment:
Fixed in commit
[6d8055b3f8](https://github.com/apache/druid/commit/6d8055b3f8). The
unreachable assignment and assertion were removed from ; the lambda now only
invokes the method expected to throw.\n\nThe focused run passed all 18 tests.
The combined focused DataSketches command passed 86 tests, and Maven 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]
