Skip to content

[ISSUE #10373] Bound stats test waits and clean up executors - #11485

Open
weidong-qi wants to merge 1 commit into
apache:developfrom
weidong-qi:test/10373-stats-task-completion
Open

weidong-qi wants to merge 1 commit into
apache:developfrom
weidong-qi:test/10373-stats-task-completion

Conversation

@weidong-qi

@weidong-qi weidong-qi commented Oct 22, 2026 •

Copy link
Copy Markdown

Which Issue(s) This PR Addresses

Related to #10373, specifically the work on unnecessary sleeps and resource cleanup. This PR addresses StatsItemSetTest only.

Brief Description

StatsItemSetTest currently polls getCompletedTaskCount() without a timeout and discards the futures returned by submit(). Worker exceptions can therefore go unnoticed when the resulting values still satisfy the assertions. The scheduled executor created by each test is also not shut down.

This change:

  • Waits on the submitted futures so worker failures propagate to JUnit.
  • Limits each Future.get() call to 10 seconds. This is a per-future timeout, not a deadline for the entire test.
  • Shuts down both executors in teardown and handles an uninitialized worker executor.

The existing statistics assertions and production code are unchanged.

How Did You Test This Change?
Local environment: macOS arm64, Amazon Corretto 8u504, Maven 3.9.11.

Focused tests:
mvn -B -pl common -am -Dtest=StatsItemSetTest -Dsurefire.failIfNoSpecifiedTests=false -Djacoco.skip=true test
Result: all 3 tests passed. Checkstyle reported zero violations.

Common module tests:
mvn -B -pl common -am -Djacoco.skip=true test

Result: all 267 tests passed, with no failures, errors, or skipped tests.

Additional checks using an out-of-tree JUnitCore probe:

  • The original tests passed but left 3 non-daemon scheduled-executor threads alive. The modified tests left none.
  • Injecting IllegalStateException immediately after the moment-stat update still allowed the original tests to pass. The modified tests failed with ExecutionException retaining the original cause. The injected code is not part of this PR.
  • Running the modified class 100 times in one JVM completed 300 tests with no failures or leftover non-daemon threads.

The generated test class uses Java 8 class-file version 52. JaCoCo was skipped, consistent with the repository's macOS CI configuration. The full multi-module package build, integration tests, and Linux/Windows runs were not performed locally.

Assisted-by: OpenAI Codex

Wait on submitted task futures so worker failures reach the test and waits have a timeout. Shut down both executors in teardown to prevent scheduled-thread leaks.

Related to apache#10373.

Assisted-by: OpenAI Codex
@codecov-commenter

codecov-commenter commented Oct 22, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.73%. Comparing base (57c2dc0) to head (5f11b4a).
⚠️ Report is 8 commits behind head on develop.

Additional details and impacted files
@@              Coverage Diff              @@
##             develop   #11485      +/-   ##
=============================================
- Coverage      49.79%   49.73%   -0.06%     
+ Complexity     14387    14369      -18     
=============================================
  Files           1390     1390              
  Lines         103264   103264              
  Branches       13504    13504              
=============================================
- Hits           51419    51360      -59     
- Misses         45657    45702      +45     
- Partials        6188     6202      +14     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants