Skip to content

fix: stabilize flaky ParamFlowDefaultCheckerTest by using mocked time - #3626

Open
EvanYao826 wants to merge 1 commit into
alibaba:1.8from
EvanYao826:fix/flaky-param-flow-test-2426
Open

fix: stabilize flaky ParamFlowDefaultCheckerTest by using mocked time#3626
EvanYao826 wants to merge 1 commit into
alibaba:1.8from
EvanYao826:fix/flaky-param-flow-test-2426

Conversation

@EvanYao826

Copy link
Copy Markdown

Summary

The test testParamFlowDefaultCheckSingleValueCheckQpsMultipleThreads was flaky because it used real time (TimeUnit.sleep and wall-clock-based loops) for concurrent assertions. This caused race conditions when the test ran in isolation vs. after other tests.

Root Cause

The test's second phase used TimeUtil.currentTimeMillis() in a busy loop with real thread sleep, making the assertion result depend on:

  • Exact thread scheduling timing
  • Whether a previous test's time mock was still active
  • Wall-clock drift during the 2.5-second loop

Fix

  • Wrapped the entire test in MockedStatic<TimeUtil> (consistent with all other tests in this class)
  • Replaced the real-time busy loop with a clean second round of concurrent requests after advancing the mock clock past the duration window
  • This makes the test deterministic regardless of thread scheduling or test execution order

Fixes #2426

The test testParamFlowDefaultCheckSingleValueCheckQpsMultipleThreads
was flaky because it used real time (TimeUnit.sleep and wall-clock
loops) for concurrent assertions. This caused race conditions when
the test ran in isolation vs. after other tests.

Fixed by:
- Using MockedStatic<TimeUtil> to control time deterministically
- Replacing the real-time busy loop with a clean second round of
  concurrent requests after advancing the mock clock past the
  duration window

Fixes alibaba#2426

@oss-sentinel-ai oss-sentinel-ai left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Summary

The PR replaces the wall-clock delay with mocked time, but the thread-local static mock does not affect the concurrent checks, so the revised test is not correct or deterministic.


Automated review by github-manager-bot

successCount.set(0);

// Advance time past the duration window to reset counters
sleep(mocked, rule.getDurationInSec() * 1000);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

MockedStatic scopes stubbing to the thread that created it, so the 40 worker threads still call the real TimeUtil.currentTimeMillis() inside ParamFlowChecker. Advancing this mock therefore does not advance their token-counter timestamps; the second round runs less than three real seconds after the first and is rejected rather than allowing threshold requests. Use a time source that is shared by the worker threads (or install the static mock within each worker) before removing the real wait.

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.

[bug] test case ParamFlowDefaultCheckerTest#testParamFlowDefaultCheckSingleValueCheckQpsMultipleThreads() assert error

2 participants