Skip to content

test(processing): stabilize flaky IndexIOTest execution - #19859

Open
FrankChen021 wants to merge 3 commits into
apache:masterfrom
FrankChen021:codex/indexio-parameterized-test
Open

test(processing): stabilize flaky IndexIOTest execution#19859
FrankChen021 wants to merge 3 commits into
apache:masterfrom
FrankChen021:codex/indexio-parameterized-test

Conversation

@FrankChen021

@FrankChen021 FrankChen021 commented Aug 3, 2026

Copy link
Copy Markdown
Member

Description

IndexIOTest used JUnit 5 @ParameterizedClass, which caused each of its 5,925 method-source tuples to be reported as a separate Surefire test set. The resulting stream of Running and Tests run: 1 events can overwhelm the Surefire fork communication channel; the failing master run timed out after 60 minutes while the fork was blocked in that channel.

This patch:

  • converts IndexIOTest to one @ParameterizedTest while preserving all generated cases and assertions;
  • creates fresh incremental indexes for each invocation.

The change does not alter Druid runtime behavior or reduce test coverage.

CI findings

The failure on the latest master push was a CI execution flake, not a failing assertion in the commit's changed test. The push commit only changed IncrementalIndexCursorFactoryTest; the failing job was the JDK 25 I*,A*,U* shard running IndexIOTest as part of the unit-test matrix.

  • Failed run, failed job
  • The Execute step timed out after 60 minutes.
  • The log repeatedly emitted Running org.apache.druid.segment.IndexIOTest and Tests run: 1 without an assertion failure.
  • The fork JVM's diagnostic stack was blocked in Surefire's Channels$1.write while completing a test set; Maven's parent process was waiting for the fork. This indicates Surefire event-channel backpressure from the per-parameter test-set reporting.
  • The same shard completed successfully in roughly three minutes on the preceding run, supporting the infrastructure/reporting-flake classification.

IndexIOTest is a functional correctness test, not a benchmark. It generates 5,925 combinations of input maps and expected validation outcomes. The refactor keeps those cases and assertions intact while reducing the number of Surefire test sets from one per combination to one parameterized test method. The standard Surefire XML report still records the individual invocations without adding custom name formatting.

Performance comparison

Measured locally on the same checkout with JDK 25 and the same Maven command:

mvn -B -pl processing -Dtest=IndexIOTest \
  -Dsurefire.failIfNoSpecifiedTests=false \
  -Dweb.console.skip=true -P skip-static-checks test
Configuration Tests Surefire test time Wall-clock time
@ParameterizedClass baseline 5,925 passed 189.5 s 3:31.95
@ParameterizedTest 5,925 passed 3.559 s 26.6 s

The method-level parameterization reduced the local Surefire test phase by roughly 53x. The report contains all 5,925 testcase entries using the standard Surefire naming.

Verification

  • Ran the complete IndexIOTest parameter stream: 5,925 passed.
  • Confirmed the standard Surefire XML report contains 5,925 testcase entries.
  • Ran git diff --check.
  • Self-reviewed the complete diff against master.

Key changed/added classes in this PR
  • processing/src/test/java/org/apache/druid/segment/IndexIOTest.java

This PR has:

  • been self-reviewed.
  • added or modified existing unit tests to preserve coverage.

@FrankChen021 FrankChen021 changed the title test(processing): reduce IndexIOTest Surefire overhead test(processing): stabilize flaky IndexIOTest execution Aug 3, 2026
@FrankChen021
FrankChen021 marked this pull request as ready for review August 3, 2026 12:29
Copilot AI review requested due to automatic review settings August 3, 2026 12:29
@FrankChen021
FrankChen021 marked this pull request as draft August 3, 2026 12:31
@FrankChen021
FrankChen021 marked this pull request as ready for review August 3, 2026 12: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

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Refactors IndexIOTest to avoid Surefire/JUnit 5 parameterized-class testset explosion that can overwhelm fork communication, while preserving the full parameter stream and assertions.

Changes:

  • Convert IndexIOTest from @ParameterizedClass to a single @ParameterizedTest driven by the same @MethodSource.
  • Create fresh IncrementalIndex instances per parameterized invocation to avoid cross-case coupling.
  • Configure Surefire to emit phrased parameterized testcase names in XML output.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
processing/src/test/java/org/apache/druid/segment/IndexIOTest.java Moves from class-level to method-level parameterization and makes index construction per-invocation.
pom.xml Enables JUnit 5 XML 3.0 stateless reporter with phrased testcase names for parameterized tests.

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

Comment thread processing/src/test/java/org/apache/druid/segment/IndexIOTest.java Outdated
Comment thread processing/src/test/java/org/apache/druid/segment/IndexIOTest.java Outdated
Comment thread processing/src/test/java/org/apache/druid/segment/IndexIOTest.java
Comment thread pom.xml Outdated
Comment thread pom.xml Outdated
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.

2 participants