fix: stabilize flaky ParamFlowDefaultCheckerTest by using mocked time - #3626
fix: stabilize flaky ParamFlowDefaultCheckerTest by using mocked time#3626EvanYao826 wants to merge 1 commit into
Conversation
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
left a comment
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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.
Summary
The test
testParamFlowDefaultCheckSingleValueCheckQpsMultipleThreadswas flaky because it used real time (TimeUnit.sleepand 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:Fix
MockedStatic<TimeUtil>(consistent with all other tests in this class)Fixes #2426