Skip to content

test: migrate datasketches extension tests to JUnit 5 - #19881

Merged
FrankChen021 merged 7 commits into
apache:masterfrom
FrankChen021:codex/junit5-migration-ext-core-7
Aug 6, 2026
Merged

test: migrate datasketches extension tests to JUnit 5#19881
FrankChen021 merged 7 commits into
apache:masterfrom
FrankChen021:codex/junit5-migration-ext-core-7

Conversation

@FrankChen021

Copy link
Copy Markdown
Member

Description

Migrates the extensions-core/datasketches test suite from JUnit 4 to JUnit 5.

This is Batch 7 of the extensions-core migration and is part of #13948.

Depends on the earlier migration batches in #19875, #19876, #19877, #19878, #19879, and #19880.

The migration replaces JUnit 4 assertions, lifecycle methods, and temporary-folder rules with Jupiter equivalents. The shared AggregationTestHelper supports the migrated @TempDir callers while retaining its legacy overload for the remaining migration work.

Tests

mvn -ntp test -pl extensions-core/datasketches -am \
  -Dtest='org.apache.druid.query.aggregation.datasketches.**,org.apache.druid.segment.DatasketchesProjectionTest' \
  -Dsurefire.failIfNoSpecifiedTests=false \
  -Pskip-static-checks -Dweb.console.skip=true -T1C

Result: 1,049 tests passed; 0 failures, errors, or skips.

@FrankChen021 FrankChen021 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have reviewed the code for correctness, edge cases, concurrency, and integration risks; no issues found.

Reviewed 80 of 80 changed files.


This is an automated review by Codex GPT-5.6-Sol

@FrankChen021 FrankChen021 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have reviewed the updated code for correctness, edge cases, concurrency, and integration risks; no issues found.

Reviewed 80 of 80 changed files. The shared temporary-folder helper update preserves both legacy and Jupiter callers.


This is an automated review by Codex GPT-5.6-Sol

@FrankChen021

FrankChen021 commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

Fixed in 690542e: removed the unnecessary throws IOException declaration and import from BaseSketchBuildSegmentMetadataQueryTest.newFolder(...). Error Prone reported CheckedExceptionNotThrown during strict compilation because FileUtils.createTempDirInLocation(...) does not throw a checked exception. Targeted strict compilation for extensions-core/datasketches and its required modules passed with BUILD SUCCESS.

@FrankChen021
FrankChen021 marked this pull request as ready for review August 5, 2026 02:33
Copilot AI lite review requested due to automatic review settings August 5, 2026 02:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates the extensions-core/datasketches module’s test suite from JUnit 4 to JUnit 5 (Jupiter), updating assertions, parameterized tests, and temp-directory handling. It also extends the shared processing test-jar AggregationTestHelper to support JUnit 5 @TempDir callers while keeping JUnit 4 compatibility for remaining migrations.

Changes:

  • Add JUnit 5-friendly temp-directory support to AggregationTestHelper via a provider abstraction and new *WithTempDir factory methods.
  • Migrate DataSketches extension tests from JUnit 4 APIs (rules, lifecycle annotations, assertions) to JUnit 5 equivalents.
  • Remove junit-jupiter-migrationsupport and the Vintage engine from the DataSketches module POM, while retaining the JUnit 4 API dependency for compatibility with shared test-jar signatures.

Reviewed changes

Copilot reviewed 80 out of 80 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
processing/src/test/java/org/apache/druid/query/aggregation/AggregationTestHelper.java Introduces a temp-folder provider and adds *WithTempDir helpers for Jupiter @TempDir.
extensions-core/datasketches/src/test/java/org/apache/druid/segment/DatasketchesProjectionTest.java Converts parameterized projections test from JUnit 4 to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/util/ToObjectVectorColumnProcessorFactoryTest.java Migrates lifecycle + assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchTTestPostAggregatorTest.java Replaces ExpectedException with assertThrows and JUnit 5 assertions.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchToVariancesPostAggregatorTest.java JUnit 5 migration (assertThrows, assertions).
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchToStringPostAggregatorTest.java JUnit 5 migration (assertThrows, assertions).
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchToQuantilesSketchPostAggregatorTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchToNumEntriesPostAggregatorTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchToMetricsSumEstimatePostAggregatorTest.java Migrates assertions / assertThrows to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchToMeansPostAggregatorTest.java JUnit 5 migration (assertThrows, assertions).
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchToEstimatePostAggregatorTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchToEstimateAndBoundsPostAggregatorTest.java JUnit 5 migration (assertThrows, assertions).
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchToBase64StringPostAggregatorTest.java Migrates assertions / assertThrows to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchSetOpPostAggregatorTest.java JUnit 5 migration (assertThrows, assertions).
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchOperationsTest.java Migrates assertions / assertThrows to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchObjectStrategyTest.java Migrates assertions / assertThrows to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchConstantPostAggregatorTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchAggregatorFactoryTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/ThetaSketchBuildSegmentMetadataQueryTest.java Migrates segment metadata assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaSketchSqlAggregatorTest.java Migrates assertions/assertThrows to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/SketchToStringPostAggregatorTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/SketchSetPostAggregatorTest.java JUnit 5 migration (assertThrows, assertions).
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/SketchHolderTest.java Migrates assertions/assertThrows to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/SketchHolderObjectStrategyTest.java Migrates assertions/assertThrows to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/SketchEstimateWithErrorBoundsTest.java Migrates serde test assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/SketchAggregatorFactoryTest.java Migrates lifecycle + assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/SketchAggregationWithSimpleDataTest.java Converts parameterized ingestion/query tests to JUnit 5 + @TempDir.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/oldapi/OldApiSketchAggregationTest.java Converts old-API aggregation parameterized tests to JUnit 5 + @TempDir.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/BufferHashGrouperUsingSketchMergeAggregatorFactoryTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchSqlAggregatorTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToStringPostAggregatorTest.java JUnit 5 migration (assertThrows, assertions).
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToRankPostAggregatorTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregatorTest.java JUnit 5 migration (assertThrows, assertions).
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilePostAggregatorTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregatorTest.java JUnit 5 migration (assertThrows, assertions).
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToCDFPostAggregatorTest.java JUnit 5 migration (assertThrows, assertions).
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchOperationsTest.java Migrates assertions/assertThrows to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchMergeAggregatorFactoryTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchComplexMetricSerdeTest.java Migrates assertions/assertThrows to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchAggregatorFactoryTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllFloatsSketchToStringPostAggregatorTest.java JUnit 5 migration (assertThrows, assertions).
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllFloatsSketchToRankPostAggregatorTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllFloatsSketchToQuantilesPostAggregatorTest.java JUnit 5 migration (assertThrows, assertions).
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllFloatsSketchToQuantilePostAggregatorTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllFloatsSketchToHistogramPostAggregatorTest.java JUnit 5 migration (assertThrows, assertions).
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllFloatsSketchToCDFPostAggregatorTest.java JUnit 5 migration (assertThrows, assertions).
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllFloatsSketchOperationsTest.java Migrates assertions/assertThrows to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllFloatsSketchMergeAggregatorFactoryTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllFloatsSketchComplexMetricSerdeTest.java Migrates assertions/assertThrows to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllFloatsSketchAggregatorFactoryTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllDoublesSketchToStringPostAggregatorTest.java JUnit 5 migration (assertThrows, assertions).
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllDoublesSketchToRankPostAggregatorTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllDoublesSketchToQuantilesPostAggregatorTest.java JUnit 5 migration (assertThrows, assertions).
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllDoublesSketchToQuantilePostAggregatorTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllDoublesSketchToHistogramPostAggregatorTest.java JUnit 5 migration (assertThrows, assertions).
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllDoublesSketchToCDFPostAggregatorTest.java JUnit 5 migration (assertThrows, assertions).
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllDoublesSketchOperationsTest.java Migrates assertions/assertThrows to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllDoublesSketchMergeAggregatorFactoryTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllDoublesSketchComplexMetricSerdeTest.java Migrates assertions/assertThrows to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/kll/KllDoublesSketchAggregatorFactoryTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchSqlAggregatorTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchUnionPostAggregatorTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchToStringPostAggregatorTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchToEstimateWithBoundsPostAggregatorTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchToEstimatePostAggregatorTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchMergeAggregatorFactoryTest.java Migrates lifecycle + exception tests to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchHolderTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchHolderObjectStrategyTest.java Migrates assertions/assertThrows to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchBuildUtilTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchBuildSegmentMetadataQueryTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchBuildAggregatorFactoryTest.java Migrates assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchAggregatorFactoryTest.java Migrates lifecycle + assertions to JUnit 5.
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/BaseSketchBuildSegmentMetadataQueryTest.java Migrates to JUnit 5 @TempDir and assertions; updates temp dir creation.
extensions-core/datasketches/pom.xml Removes Vintage/migrationsupport; retains JUnit 4 API dependency for shared test-jar signature compatibility.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 103 to 107
try (
final AggregationTestHelper toolchest = AggregationTestHelper.createGroupByQueryAggregationTestHelper(
final AggregationTestHelper toolchest = AggregationTestHelper.createGroupByQueryAggregationTestHelperWithTempDir(
sm.getJacksonModules(),
config,
tempFolder

@FrankChen021 FrankChen021 Aug 5, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 6d8055b3f8. I removed the mutable config/vectorize lifecycle state and changed setup to setup(GroupByQueryConfig config), called at the start of each of the four @ParameterizedTest methods. Each parameterized invocation now builds its indexes with its own config after JUnit 5 supplies the parameters; vectorize remains the test method parameter used for the query context.

Validation:

mvn -ntp test -pl extensions-core/datasketches -am -Dtest='org.apache.druid.query.aggregation.datasketches.theta.SketchAggregationWithSimpleDataTest,org.apache.druid.query.aggregation.datasketches.theta.oldapi.OldApiSketchAggregationTest,org.apache.druid.query.aggregation.datasketches.hll.HllSketchMergeAggregatorFactoryTest' -Dsurefire.failIfNoSpecifiedTests=false -Pskip-static-checks -Dweb.console.skip=true -T1C

Result: 86 tests passed, 0 failures/errors/skips. Checkstyle and SpotBugs also passed.

Comment on lines +91 to 95
@AfterEach
public void teardown() throws IOException
{
helper.close();
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 6d8055b3f8. teardown() now checks helper != null, closes it when initialized, and clears the field afterward. This prevents cleanup from masking an initialization failure and avoids retaining a closed helper.

The focused OldApiSketchAggregationTest run passed all 36 parameterized tests. The combined focused command covered this class and the other two reviewed classes and passed 86 tests; Maven Checkstyle and SpotBugs both reported no violations or errors.

Comment on lines +133 to +134
HllSketchAggregatorFactory result = (HllSketchAggregatorFactory) targetRound.getMergingFactory(other);
Assertions.assertEquals(LG_K, result.getLgK());

@FrankChen021 FrankChen021 Aug 5, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 6d8055b3f8. The unreachable result assignment and assertion were removed from testGetMergingFactoryDifferentStringEncoding; the lambda now only invokes the method expected to throw.

The focused HllSketchMergeAggregatorFactoryTest run passed all 18 tests. The combined focused DataSketches command passed 86 tests, and Maven Checkstyle and SpotBugs passed with 0 violations/errors.

@FrankChen021 FrankChen021 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have reviewed the updated code for correctness, edge cases, concurrency, and integration risks; no issues found.

Reviewed 80 of 80 changed files. The later commits preserve test-count parity and correctly handle strict-compilation cleanup, parameterized setup ordering, and teardown null safety.


This is an automated review by Codex GPT-5.6-Sol

@FrankChen021
FrankChen021 merged commit ec95071 into apache:master Aug 6, 2026
44 of 45 checks passed
@FrankChen021
FrankChen021 deleted the codex/junit5-migration-ext-core-7 branch August 6, 2026 12:44
@github-actions github-actions Bot added this to the 39.0.0 milestone Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants