Refactor: Extract shared cooperative-cancellation timeout helper to eliminate duplication#9308
Open
Copilot wants to merge 2 commits into
Open
Refactor: Extract shared cooperative-cancellation timeout helper to eliminate duplication#9308Copilot wants to merge 2 commits into
Copilot wants to merge 2 commits into
Conversation
…r to eliminate code duplication Closes #9306 The cooperative-cancellation timeout block (create a timeout CancellationTokenSource, chain it to the existing token, run the action, catch OperationCanceledException, distinguish timeout from cancellation) was copy-pasted — comment and all — between FixtureMethodRunner and TestMethodInfo.Execution. This change: - Adds `FixtureMethodRunner.RunWithCooperativeCancellationAsync<T>`, a generic internal helper that encapsulates the shared pattern. - Updates the private RunWithCooperativeCancellationAsync to delegate to the generic helper. - Updates TestMethodInfo.Execution.ExecuteInternalWithTimeoutAsync to use the shared helper, removing ~40 lines of duplicated code. - Fixes an inconsistency: the TestMethodInfo path previously caught bare OperationCanceledException; the shared helper now consistently uses IsOperationCanceledExceptionFromToken in both paths. Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Refactor duplicate cooperative cancellation timeout blocks
Refactor: Extract shared cooperative-cancellation timeout helper to eliminate duplication
Jun 21, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors duplicated “cooperative cancellation timeout” logic by extracting a shared helper into FixtureMethodRunner, and then reusing it from TestMethodInfo.Execution to unify behavior and reduce copy/paste.
Changes:
- Add a generic
FixtureMethodRunner.RunWithCooperativeCancellationAsync<T>helper to encapsulate cooperative-timeout token wiring and cancellation handling. - Replace the duplicated cooperative-cancellation block in
TestMethodInfo.Execution.ExecuteInternalWithTimeoutAsyncwith a call to the new helper. - Tighten cancellation handling in the
TestMethodInfo.Executionpath to only treatOperationCanceledExceptionas a timeout/cancel when it originates from the expected token (viaIsOperationCanceledExceptionFromToken).
Show a summary per file
| File | Description |
|---|---|
| src/Adapter/MSTestAdapter.PlatformServices/Helpers/FixtureMethodRunner.cs | Introduces a shared generic cooperative-cancellation helper and updates the fixture-specific helper to delegate to it. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.Execution.cs | Replaces local duplicated logic with a call into the shared helper, aligning cancellation filtering behavior. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 2
Comment on lines
291
to
+296
| if (TimeoutInfo.CooperativeCancellation) | ||
| { | ||
| CancellationTokenSource? timeoutTokenSource = null; | ||
| try | ||
| { | ||
| timeoutTokenSource = new(TimeoutInfo.Timeout); | ||
| timeoutTokenSource.Token.Register(TestContext.Context.CancellationTokenSource.Cancel); | ||
| if (timeoutTokenSource.Token.IsCancellationRequested) | ||
| return await FixtureMethodRunner.RunWithCooperativeCancellationAsync<TestResult>( | ||
| TestContext.Context.CancellationTokenSource, | ||
| TimeoutInfo.Timeout, | ||
| timeoutCts => ExecuteInternalAsync(arguments, timeoutCts), |
Comment on lines
99
to
108
| try | ||
| { | ||
| await ExecutionContextHelpers.RunOnContextAsync(executionContext, action).ConfigureAwait(false); | ||
| return null; | ||
| return await action(timeoutTokenSource).ConfigureAwait(false); | ||
| } | ||
| catch (Exception ex) when (ex.IsOperationCanceledExceptionFromToken(cancellationTokenSource.Token)) | ||
| { | ||
| // Ideally we would like to check that the token of the exception matches cancellationTokenSource but TestContext | ||
| // instances are not well defined so we have to handle the exception entirely. | ||
| return new( | ||
| UnitTestOutcome.Timeout, | ||
| timeoutTokenSource.Token.IsCancellationRequested | ||
| ? string.Format( | ||
| CultureInfo.InvariantCulture, | ||
| methodTimedOutMessageFormat, | ||
| methodInfo.DeclaringType!.FullName, | ||
| methodInfo.Name, | ||
| timeout) | ||
| : string.Format( | ||
| CultureInfo.InvariantCulture, | ||
| methodCanceledMessageFormat, | ||
| methodInfo.DeclaringType!.FullName, | ||
| methodInfo.Name)); | ||
| return onCancelled(timeoutTokenSource.Token.IsCancellationRequested); | ||
| } |
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.
The cooperative-cancellation timeout block (create timeout
CancellationTokenSource, link it to the parent token, run the action, catchOperationCanceledException, distinguish timeout vs. user-cancel) was copy-pasted verbatim — including the multi-sentence explanatory comment — betweenFixtureMethodRunnerandTestMethodInfo.Execution. The two sites also had an inconsistency:FixtureMethodRunnerfiltered withIsOperationCanceledExceptionFromToken, whileTestMethodInfo.Executioncaught bareOperationCanceledException.Changes
FixtureMethodRunner.cs— Addsinternal static async Task<T> RunWithCooperativeCancellationAsync<T>encapsulating the shared pattern. Callers supplyonAlreadyCancelledandonCancelled(bool isTimeout)delegates to produce the appropriate result type:The private
RunWithCooperativeCancellationAsync(fixture-specific) now delegates to this generic helper.TestMethodInfo.Execution.cs— Replaces the ~40-line duplicated cooperative-cancellation block inExecuteInternalWithTimeoutAsyncwith a call toFixtureMethodRunner.RunWithCooperativeCancellationAsync<TestResult>.Bug fix: The
TestMethodInfo.Executionpath now consistently usesIsOperationCanceledExceptionFromToken(matchingFixtureMethodRunner) instead of indiscriminately catching allOperationCanceledExceptioninstances.