Repository navigation
Fix flaky StampedeTests and harden related test waits - #7572
Merged
jeffhandley merged 3 commits intoJun 15, 2026
Merged
Conversation
Replaces the bare 'await Task.Delay(500)' before the cancelled-caller asserts in MultipleCallsShareExecution_EveryoneCancels and MultipleCallsShareExecution_MostCancel with a bounded WaitForCompletionAsync(results[i], 5_000) call. The 500ms warmup is retained because the shared underlying task needs that window to enter its semaphore wait before the test releases the semaphore for the handshake. - Add private static WaitForCompletionAsync(Task, int timeoutMs) helper using Task.WhenAny(task, Task.Delay(timeoutMs)) with a CallerArgumentExpression-driven TimeoutException, suppressing VSTHRD003 to match the AssertAwaitingTaskCompleted helper in FakeLogCollectorTests.LogEnumeration.cs. - In each test, await WaitForCompletionAsync(result, 5_000) immediately before Assert.Equal(TaskStatus.Canceled, result.Status) so the assertion no longer races the worker-thread cancellation transition. Fixes dotnet#6836 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaces the bare 'await Task.Delay(500)' between the cache populate loop and the KeyTimeToLiveAsync assertion with a bounded poll that calls KeyTimeToLiveAsync up to 50 times with 100ms between attempts, exiting as soon as a non-null TTL is returned. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaces the two bare 'await Task.Delay(100)' waits in FakeHostTests: - Host_ShutsDownAfterTimeout: polls host.Services.GetService<IHost>() in a try/catch loop (up to 50 attempts of 100ms) and captures the ObjectDisposedException, asserting the exception was observed instead of asserting it on a single direct call. - StartAsync_NoTokenProvided_UsesDefaultTimeout: polls hostMock.Invocations.Count (up to 50 attempts of 100ms) before calling hostMock.VerifyAll(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens several timing-sensitive tests by replacing fixed Task.Delay(...) waits with bounded polling/timeout-based waiting, addressing CI flakiness in the HybridCache stampede cancellation tests (Issue #6836) and applying the same pattern to a couple of other tests that asserted on eventually-consistent background state.
Changes:
- Updated
StampedeTestscancellation scenarios to wait (with a timeout) for caller tasks to complete before assertingTaskStatus.Canceled, and introduced a localWaitForCompletionAsynchelper. - Replaced a fixed delay in
RedisTestswith a bounded poll for a Redis TTL to become observable after an async L2 write. - Replaced fixed delays in
FakeHostTestswith bounded polling to observe host disposal and Moq invocation recording.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/Libraries/Microsoft.Extensions.Hosting.Testing.Tests/FakeHostTests.cs | Replaces fixed delays with bounded polling to reduce timing-related flakiness in host shutdown and invocation assertions. |
| test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/StampedeTests.cs | Fixes the reported flake by waiting for cancellation completion with a timeout before asserting task status; adds a helper for bounded waits. |
| test/Libraries/Microsoft.Extensions.Caching.Hybrid.Tests/RedisTests.cs | Replaces a fixed delay with a bounded poll for TTL visibility after background Redis writes. |
jozkee
approved these changes
Jun 15, 2026
Collaborator
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1465264&view=codecoverage-tab |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes #6836
StampedeTests.MultipleCallsShareExecution_MostCancel(and its sibling_EveryoneCancels) assertedresult.Status == TaskStatus.Canceledafter a fixedTask.Delay(500). On loaded CI agents, the HybridCache stampede coordinator hadn't always transitioned every caller task toCanceledwithin that window, producing intermittent failures.This PR replaces that pure-delay assertion with a bounded poll on the caller's task and opportunistically applies the same hardening to two other tests that used fixed delays before asserting on background-thread state.
StampedeTests (the actual flake fix)
WaitForCompletionAsync(Task, int timeoutMs, [CallerArgumentExpression] string?)helper modeled onAssertAwaitingTaskCompletedinFakeLogCollectorTests.LogEnumeration.cs. ThetimeoutMshas no default; the 5-second budget is passed explicitly at each call site.MultipleCallsShareExecution_EveryoneCancelsand_MostCancelnowawait WaitForCompletionAsync(result, 5_000)before assertingTaskStatus.Canceled.Task.Delaywarmup is retained with an updated comment explaining its actual purpose: letting the shared underlying lambda enter itssemaphore.WaitAsync(...)before the main thread completes the handshake. A first attempt that removed it entirely failed 25/25 on net462 atAssert.Equal(1, executeCount)because the test reuses a singleSemaphoreSlimfor handshakes in both directions andSemaphoreSlimdoes not guarantee FIFO ordering.Stress-verified locally: 25/25 pass on net8/9/10/462 for both methods after the fix.
Opportunistic improvements to related "pure delay before assert" tests
RedisTests: replacedawait Task.Delay(500)before readingKeyTimeToLiveAsyncwith a bounded poll (50 attempts of 100ms = 5 seconds total) on the TTL becoming non-null. The test isIClassFixture<RedisFixture>and skipped without a Redis instance, so it is not in default CI -- this is purely defense-in-depth for the codebase's reusable wait pattern.FakeHostTests:Host_ShutsDownAfterTimeout: replaced a fixed delay with a try/catch poll (50 x 100ms = 5 seconds total) capturingObjectDisposedException, then asserting it surfaced.StartAsync_NoTokenProvided_UsesDefaultTimeout: replaced a fixed delay with a poll onhostMock.Invocations.Count(50 x 100ms = 5 seconds total) beforehostMock.VerifyAll().Stress-verified locally: 25/25 pass on net8/9/10/462 for both
FakeHostTestsmethods.Microsoft Reviewers: Open in CodeFlow
Microsoft Reviewers: Open in CodeFlow