Make Should-BeFasterThan/BeSlowerThan scriptblock tests deterministic#2855
Draft
nohwnd wants to merge 2 commits into
Draft
Make Should-BeFasterThan/BeSlowerThan scriptblock tests deterministic#2855nohwnd wants to merge 2 commits into
nohwnd wants to merge 2 commits into
Conversation
The scriptblock-based timing tests measured a real Start-Sleep against a narrow threshold, so CI scheduling noise could flip the outcome. A recent run saw a 10ms sleep measured under the 1ms threshold, so the expected assertion failure never fired. Use thresholds that timing noise cannot cross while still exercising the script block (& $Actual) + stopwatch path: a generous upper bound (10s) for the pass/throw-when-fast direction and a 0ms lower bound for the fail/throw-when-slow direction, in both assertions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…mments CI investigation (running the assertion 2000x per agent with an independent wall clock alongside the stopwatch) showed the flake is a rare per-call Stopwatch/QueryPerformanceCounter under-measurement on the virtualized Windows agents: a nominal 10ms sleep (really ~15.6ms on Windows' timer tick) is occasionally measured under 1ms, reproduced ~6/2000 on PS 5.1 / Server 2022, while the wall clock and the accumulated batch confirm the full time elapsed. It is not the sleep returning early, and not a bug in where the stopwatch starts. Comparing against the floor (0ms) and ceiling (10s) of any measurement -- never a threshold near the real run time -- makes these [scriptblock]-path tests immune to that per-call clock glitch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
96b4c7e to
53d163e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The script-block timing tests for
Should-BeFasterThan/Should-BeSlowerThanmeasured a realStart-Sleepagainst a threshold set close to that sleep's own duration, so CI timing noise could flip the result.A run flaked on the
Test PS_5_1_Windows_Server2022check (check run 86215719809, part of #2852):That test runs
{ Start-Sleep -Milliseconds 10 }and expects it to be slower than a1msthreshold, so the assertion should fail. It didn't: the stopwatch measured the sleep at under 1 ms, soShould-BeFasterThan -Expected 1mspassed and the expected failure never fired.Root cause (measured on CI, not guessed)
I instrumented the assertion and ran it 2000× per agent, measuring each
Start-Sleep -Milliseconds 10with two independent clocks —Stopwatch(QueryPerformanceCounter) andDateTime.UtcNow(wall clock) — plus a batch total, and directly reproducing the assertion. Results:Start-Sleep 10p50Two facts pin it down:
So the flake is a rare per-call
Stopwatch/QueryPerformanceCounterunder-measurement on the virtualized, 2-vCPU Windows agents: the two timestamp reads bracketing one sleep can be served by cores whose TSC/QPC are momentarily unsynchronized, yielding a near-zero delta. Over many samples it averages out (the batch is correct); a single bracketed measurement can read almost zero.It is not the sleep waking early (scheduling/coalescing only ever rounds up), and not a bug in where the stopwatch is started — the stopwatch wraps
& $Actualtightly, and the batch experiment confirms QPC accumulates correctly. There is nothing to fix in the assertion's measurement; on real hardware QPC is reliable.Fix
Keep exercising the real script block (
& $Actual) + stopwatch path, but compare against the floor and ceiling of any measurement, never a threshold near the real duration:0ms. A measured script block always takes>= 0, so the result is decided the same way on every agent no matter how the clock behaves.10s. No real script block reaches 10 s, so no measurement — even an anomalous one — can cross it.Applied symmetrically to
Should-BeFasterThanandShould-BeSlowerThan, whose script-block tests share the identical latent flake. The deterministic[timespan]-based tests are unchanged. This is the same class of flakiness previously addressed for the Because test in #2722.Test-only change. Both files pass locally (
Should-BeFasterThan16 tests,Should-BeSlowerThan5 tests).