Implement global command-line options for dotnet test (MTP): --timeout and --maximum-failed-tests - #55458
Conversation
DangerousFileDetectorTests was migrated to MSTest.Sdk, which runs test methods in parallel (MethodLevel). ItShouldDetectFileWithMarkOfTheWeb and WhenThereIsNoFileItReturnsFalse now call DangerousFileDetector.IsDangerous concurrently, exposing an unsynchronized static-init race: one thread sets s_attemptedLoad=true before s_classFactory is populated, so the other thread reads s_classFactory==null and returns false, causing the intermittent 'Expected True, but found False' failure seen across many PRs. Serialize the lazy class-factory creation behind a lock. Also convert the leftover xUnit [Fact] usages in TransientSdkResolutionErrorDetectorTests to MSTest [TestClass]/[TestMethod], which were breaking the test build. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add run-level maximum-failure and timeout policies for dotnet test, including cooperative TestFx server-control cancellation and per-app passthrough after --. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 231e4f5f-653d-4a96-8ed2-206c484a357b
When test-application parallelism is greater than one, OnTestApplicationExited can drive the remaining timeout non-positive a moment before it flips the cancellation reason to Timeout. A test application starting in that window previously constructed a Timer with a negative due time, which throws ArgumentOutOfRangeException, spuriously fails the module, and leaks the active-application count. Clamp the due time to zero so the timer fires immediately and OnTimeout performs the cancellation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 37e6377f-fdab-42f4-bc63-0763a85772e5
Code review summaryThree review passes plus one independent Pass 1 — self, core files ( Pass 2 — self, remaining logic ( Pass 3 — code-review subagent (independent): found one real Medium bug — negative Findings & disposition
Tests re-run green after the fix (incremental build)
Branch commits
Coverage caveatThe two e2e tests validate SDK-side policy + exit codes but would also pass on the |
There was a problem hiding this comment.
Pull request overview
Implements remaining run-level (“global”) dotnet test options for Microsoft.Testing.Platform (MTP) by adding a thread-safe run policy (timeout + max failed tests), wiring cooperative session cancellation via a reverse control pipe, and updating CLI parsing/help/localization and tests.
Changes:
- Add
--timeoutand--maximum-failed-testsas global MTPdotnet testoptions with validation and localized help. - Introduce a run-level coordinator (
TestRunPolicy) and a new IPC reverse-control channel to cooperatively cancel test sessions. - Add unit + end-to-end tests and update MTP dependency version to 2.4 preview line.
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.NET.TestFramework/Constants.cs | Add new exit codes for timeout abort and max-failures policy. |
| test/Microsoft.DotNet.Cli.Utils.Tests/TransientSdkResolutionErrorDetectorTests.cs | Convert tests to MSTest attributes to match test project SDK. |
| test/dotnet.Tests/CommandTests/Test/TestRunPolicyTests.cs | New unit tests for TestRunPolicy behavior and concurrency regression. |
| test/dotnet.Tests/CommandTests/Test/TestCommandParserTests.cs | Add parsing/validation tests for new global options and -- forwarding. |
| test/dotnet.Tests/CommandTests/Test/ServerControlMessageSerializerTests.cs | New serializer contract tests for reverse-control IPC messages. |
| test/dotnet.Tests/CommandTests/Test/GivenDotnetTestBuildsAndRunsTests.cs | Add E2E coverage for exit codes from max-failures and timeout. |
| src/Cli/Microsoft.DotNet.Cli.Utils/DangerousFileDetector.cs | Make COM factory initialization thread-safe to avoid race-induced false negatives. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.zh-Hant.xlf | Localization updates for new/updated dotnet test option strings. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.zh-Hans.xlf | Localization updates for new/updated dotnet test option strings. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.tr.xlf | Localization updates for new/updated dotnet test option strings. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.ru.xlf | Localization updates for new/updated dotnet test option strings. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.pt-BR.xlf | Localization updates for new/updated dotnet test option strings. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.pl.xlf | Localization updates for new/updated dotnet test option strings. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.ko.xlf | Localization updates for new/updated dotnet test option strings. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.ja.xlf | Localization updates for new/updated dotnet test option strings. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.it.xlf | Localization updates for new/updated dotnet test option strings. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.fr.xlf | Localization updates for new/updated dotnet test option strings. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.es.xlf | Localization updates for new/updated dotnet test option strings. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.de.xlf | Localization updates for new/updated dotnet test option strings. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/xlf/CommandDefinitionStrings.cs.xlf | Localization updates for new/updated dotnet test option strings. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/Commands/Test/TestCommandDefinition.MicrosoftTestingPlatform.cs | Define new options, add validators, and implement --timeout parsing. |
| src/Cli/Microsoft.DotNet.Cli.Definitions/CommandDefinitionStrings.resx | Add/adjust option descriptions and validation error strings. |
| src/Cli/dotnet/Commands/Test/MTP/TestRunPolicy.cs | New run-level policy state machine for timeout + maximum failed tests. |
| src/Cli/dotnet/Commands/Test/MTP/TestApplicationHandler.cs | Feed failure counts into the run policy from test result messages. |
| src/Cli/dotnet/Commands/Test/MTP/TestApplicationActionQueue.cs | Stop scheduling new test apps when run-level cancellation is requested. |
| src/Cli/dotnet/Commands/Test/MTP/TestApplication.cs | Add reverse control pipe, advertise capability in handshake, and request cooperative cancellation. |
| src/Cli/dotnet/Commands/Test/MTP/Terminal/TerminalTestReporter.cs | Track cancellation state for policy-triggered cancellation reporting. |
| src/Cli/dotnet/Commands/Test/MTP/MicrosoftTestingPlatformTestCommand.cs | Create and wire TestRunPolicy, link cancellation, and map reasons to exit codes. |
| src/Cli/dotnet/Commands/Test/MTP/IPC/Serializers/WaitForServerControlRequestSerializer.cs | New serializer for control-channel request. |
| src/Cli/dotnet/Commands/Test/MTP/IPC/Serializers/ServerControlMessageSerializer.cs | New serializer for control-channel cancellation message. |
| src/Cli/dotnet/Commands/Test/MTP/IPC/Serializers/RegisterSerializers.cs | Register new IPC serializers for control channel. |
| src/Cli/dotnet/Commands/Test/MTP/IPC/ObjectFieldIds.cs | Add message/field ids for new control-channel contract. |
| src/Cli/dotnet/Commands/Test/MTP/IPC/Models/WaitForServerControlRequest.cs | New request model for waiting on server control messages. |
| src/Cli/dotnet/Commands/Test/MTP/IPC/Models/ServerControlMessage.cs | New response model for server control messages. |
| src/Cli/dotnet/Commands/Test/MTP/ExitCode.cs | Add max-failed-tests policy exit code constant. |
| src/Cli/dotnet/Commands/Test/CliConstants.cs | Add handshake property id and server-control kind constant. |
| eng/Version.Details.xml | Update dependency version/SHA for Microsoft.Testing.Platform. |
| eng/Version.Details.props | Bump MicrosoftTestingPlatformPackageVersion to 2.4 preview. |
Copilot's findings
- Files reviewed: 38/38 changed files
- Comments generated: 3
… timeout error Addresses Copilot review comments on dotnet#55458: - TestApplicationHandler no longer allocates a fallback TestRunPolicy (which owns a CancellationTokenSource and was never disposed, leaking in tests that omit the policy). The field is now nullable and ReportFailedTests is called null-conditionally; production always supplies a policy owned/disposed by MicrosoftTestingPlatformTestCommand. - CmdTestInvalidTimeout now notes that longer unit forms (seconds/minutes/hours/days) are also accepted, matching TimeoutPattern. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 37e6377f-fdab-42f4-bc63-0763a85772e5
Resolve conflicts from main's artifact-post-processing feature and xUnit->MSTest test migration. TestRunPolicy is now optional/nullable on TestApplication and TestApplicationHandler so post-processing tool invocations don't require a policy. Regenerated .xlf for the updated CmdTestInvalidTimeout string. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 37e6377f-fdab-42f4-bc63-0763a85772e5
ServerControlMessageSerializerTests and TestRunPolicyTests were added on this branch using xUnit ([Fact]) but main migrated the dotnet.Tests project to MSTest. Convert them to [TestClass]/[TestMethod] and use MSTest's TestContext.CancellationToken. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 37e6377f-fdab-42f4-bc63-0763a85772e5
|
Rebased onto latest
The 3 review comments above are addressed in Validation after merge: |
|
Azure Pipelines: Successfully started running 1 pipeline(s). 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 37e6377f-fdab-42f4-bc63-0763a85772e5
|
/backport to release/11.0.1xx-preview7 |
|
Started backporting to |
|
@Evangelink backporting to |
|
/backport to release/11.0.1xx-preview7 |
|
Started backporting to |
|
@Evangelink backporting to git am output$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix thread-safety race in DangerousFileDetector and unblock test build
Using index info to reconstruct a base tree...
M src/Cli/Microsoft.DotNet.Cli.Utils/DangerousFileDetector.cs
M test/Microsoft.DotNet.Cli.Utils.Tests/TransientSdkResolutionErrorDetectorTests.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Cli/Microsoft.DotNet.Cli.Utils/DangerousFileDetector.cs
CONFLICT (content): Merge conflict in src/Cli/Microsoft.DotNet.Cli.Utils/DangerousFileDetector.cs
Auto-merging test/Microsoft.DotNet.Cli.Utils.Tests/TransientSdkResolutionErrorDetectorTests.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Fix thread-safety race in DangerousFileDetector and unblock test build
Error: The process '/usr/bin/git' failed with exit code 128 |
|
/backport to release/11.0.1xx-preview7 |
|
Started backporting to |
|
@Evangelink backporting to git am output$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix thread-safety race in DangerousFileDetector and unblock test build
Using index info to reconstruct a base tree...
M src/Cli/Microsoft.DotNet.Cli.Utils/DangerousFileDetector.cs
M test/Microsoft.DotNet.Cli.Utils.Tests/TransientSdkResolutionErrorDetectorTests.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Cli/Microsoft.DotNet.Cli.Utils/DangerousFileDetector.cs
CONFLICT (content): Merge conflict in src/Cli/Microsoft.DotNet.Cli.Utils/DangerousFileDetector.cs
Auto-merging test/Microsoft.DotNet.Cli.Utils.Tests/TransientSdkResolutionErrorDetectorTests.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Fix thread-safety race in DangerousFileDetector and unblock test build
Error: The process '/usr/bin/git' failed with exit code 128 |
|
Manual backport to |
…t and --maximum-failed-tests (#55458)
Implements the remaining "global" (run-level) command-line options for
dotnet testunder Microsoft.Testing.Platform, as requested in #49709. Options placed before--are treated as global and apply to the whole run; options after--continue to be forwarded per test application.New global options
--timeout <duration>— aborts the run if total test-application execution time exceeds the given duration (e.g.90s,5m,100ms). Timing starts when the first test app begins and only accrues while at least one app is running. On timeout the run returnsTestSessionAborted(exit code 3), matching native MTP behavior.--maximum-failed-tests <n>— stops the run once the number of failed/errored/timed-out/cancelled test results reachesn, returning exit code 13 (TestExecutionStoppedForMaxFailedTests). Counting is independent of retry bookkeeping.How cancellation works
The SDK now advertises a
ServerControlPipeNamecapability in the handshake reply and opens a reverse control pipe. When a run-level policy trips, the SDK sends a cooperativeCancelSessionmessage to each test app over that pipe (the TestFx side landed in microsoft/testfx#9549). This requires the MTP 2.4 line, soMicrosoftTestingPlatformPackageVersionis bumped from 2.3.0 to 2.4.0-preview.Key changes
TestRunPolicy(thread-safe timeout + max-failure state machine).ServerControlMessage/WaitForServerControlRequestmodels + serializers, field ids, and registration.TestApplication/TestApplicationActionQueue/TestApplicationHandlerwire the policy, reverse control pipe, and linked cancellation..resx+.xlf).Tests
TestRunPolicyTests(8, incl. a concurrency regression test for the timeout race),ServerControlMessageSerializerTests(3),TestApplicationHandlerTests(19),TestCommandDefinitionTestsincl. new global-option parsing/validation cases (66).GivenDotnetTestBuildsAndRunsTests:RunMTPSolutionWithMaximumFailedTestsReturnsPolicyExitCode(exit 13) andRunMTPProjectWithGlobalTimeoutReturnsTestSessionAborted(exit 3).Fixes #49709.