Fix non-deterministic branch coverage in HedgingExecutionContext hedging delay tests#2997
Conversation
…ing delay tests Replace the single non-deterministic TryWaitForCompletedExecutionAsync_HedgedExecution_Ok test with two focused, deterministic tests: 1. DelayFiresFirst_ReturnsNull: primary task delay (1h) > hedging delay (5s). Advance only 10s to fire the hedging delay without completing the primary, so whenAnyHedgedTask.IsCompleted is deterministically false → returns null. 2. TaskCompletesBeforeDelay_ReturnsTask: primary task delay (5s) < hedging delay (1h). Advance only 10s to complete the primary without firing the hedging delay timer, so whenAnyHedgedTask.IsCompleted is deterministically true → returns non-null. Both tests assert the return value explicitly so mutation tests reliably fail when the condition at HedgingExecutionContext.cs:132 is inverted. Co-authored-by: martincostello <1439341+martincostello@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2997 +/- ##
=======================================
Coverage 96.15% 96.15%
=======================================
Files 310 310
Lines 7135 7135
Branches 1005 1005
=======================================
Hits 6861 6861
Misses 221 221
Partials 53 53
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Refactoring to make the tests a bit more readable and a little less repetitive.
There was a problem hiding this comment.
Pull request overview
This PR fixes flaky mutation/branch coverage around the whenAnyHedgedTask.IsCompleted check in HedgingExecutionContext<T>.TryWaitForCompletedExecutionAsync(...) by replacing a timing-racy hedging-delay test with two deterministic tests that explicitly drive each branch.
Changes:
- Replaced the previous single hedging-delay test with two deterministic tests that separately assert the “delay fires first” and “task completes first” outcomes.
- Preserved
continueOnCapturedContext = true/falsecoverage for both new tests. - Minor refactors/cleanup in the test file (consistent attribute ordering, reuse of common
TimeSpanvalues, a few additional safety assertions).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The mutation tests for
HedgingExecutionContext<T>were flaky because the condition at line 132 (if (!whenAnyHedgedTask.IsCompleted)) was not reliably exercised by both branches. The single existing test advanced the fake clock by a full day, firing both the hedging delay timer and the primary task timer simultaneously — making the outcome of theIsCompletedcheck dependent on non-deterministic async continuation scheduling.Changes
TryWaitForCompletedExecutionAsync_HedgedExecution_Okwith two deterministic, explicitly-asserted tests:DelayFiresFirst_ReturnsNull— primary task delay (1 h) exceeds hedging delay (5 s); advancing only 10 s fires the hedging delay while the primary is still running, sowhenAnyHedgedTask.IsCompletedis deterministicallyfalseand the method deterministically returnsnull(result.ShouldBeNull()).TaskCompletesBeforeDelay_ReturnsTask— primary task delay (5 s) is less than hedging delay (1 h); advancing only 10 s completes the primary before the hedging delay timer fires, sowhenAnyHedgedTask.IsCompletedis deterministicallytrueand the method deterministically returns the completed execution (result.ShouldNotBeNull()).continueOnCapturedContext = true/false, preserving the originalConfigureAwaitflag coverage.With these changes, inverting the condition at line 132 will always cause at least one test to fail.
Confirm the following
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.