Skip to content

test: fix flaky ScheduledExecutorsTest.testscheduleWithFixedDelay - #20526

Merged
FrankChen021 merged 2 commits into
apache:masterfrom
ykisana:fix-test-ScheduledExecutorsTest.testscheduleWithFixedDelay
Oct 9, 2026
Merged

FrankChen021 merged 2 commits into
apache:masterfrom
ykisana:fix-test-ScheduledExecutorsTest.testscheduleWithFixedDelay

Conversation

@ykisana

@ykisana ykisana commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #20503.

Description

ScheduledExecutorsTest.testscheduleWithFixedDelay asserted that the first task starts strictly more than 100ms after scheduling (firstTaskStart > 100). The test measured with System.currentTimeMillis(), but ScheduledThreadPoolExecutor schedules on System.nanoTime() and can fire exactly on time. A real gap of 100.0–100.99ms truncates to 100 ms, so the strict bound failed even though the executor behaved correctly (CI: was: 100 on all 4 surefire attempts).

Fixed the flaky lower bound

  • Record task start times with System.nanoTime(), relative to the start of the test, so the test uses the same monotonic clock as the executor and isn't affected by wall-clock adjustments.
  • Make the lower bound inclusive (firstTaskStart >= 100). schedule() is called after the start time is recorded, so the measured gap can never be less than the initial delay.
  • Replace the comment that claimed the delay is always greater than 100ms "due to overhead", which was the incorrect assumption behind the flake.

The checks on the delay between later tasks are unchanged. Every start time is measured from the same origin, so the gaps between them are the same as before.


Key changed/added classes in this PR
  • ScheduledExecutorsTest

This PR has:

  • been self-reviewed.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.

@FrankChen021
FrankChen021 self-requested a review October 9, 2026 02:06
@FrankChen021
FrankChen021 merged commit 805a542 into apache:master Oct 9, 2026
27 checks passed
@github-actions github-actions Bot added this to the 39.0.0 milestone Oct 9, 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.

Flaky test: ScheduledExecutorsTest.testscheduleWithFixedDelay

2 participants