Fix TelemetryTests flake from parallel dotnet test races#8292
Fix TelemetryTests flake from parallel dotnet test races#8292Evangelink wants to merge 1 commit into
Conversation
The VSTest_RunTests_Succeeds and VSTest_DiscoverTests_Succeeds tests both invoke `dotnet test` against the same shared VSTest project path for every target framework. Running them in parallel races on MSBuild outputs such as `bin/Release/<tfm>/TelemetryVSTestProject.runtimeconfig.json` and shared `obj/` files, which causes intermittent `GenerateRuntimeConfigurationFiles` failures (file in use). Apply `[DoNotParallelize]` at the class level to serialize all tests in TelemetryTests. Coverage is unchanged and the duration impact is small. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes an intermittent flake in MSTest.Acceptance.IntegrationTests caused by MSTest’s assembly-level parallel execution running multiple target-framework instances of the VSTest telemetry tests concurrently, which races on shared bin/ and obj/ MSBuild outputs.
Changes:
- Add
[DoNotParallelize]at theTelemetryTestsclass level to prevent parallel execution of its test methods / data rows. - Document the underlying MSBuild output race causing
GenerateRuntimeConfigurationFilesfailures.
Show a summary per file
| File | Description |
|---|---|
| test/IntegrationTests/MSTest.Acceptance.IntegrationTests/TelemetryTests.cs | Serializes TelemetryTests execution via [DoNotParallelize] to avoid dotnet test output races across TFMs. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 0
Evangelink
left a comment
There was a problem hiding this comment.
Review of PR #8292 — [DoNotParallelize] on TelemetryTests
Summary: Adds [DoNotParallelize] to TelemetryTests to prevent intermittent race conditions on shared MSBuild outputs (bin/, obj/) when dotnet test is invoked against the same VSTest project path across multiple target frameworks in parallel.
Assessment
The fix is correct and well-motivated. Shared MSBuild intermediate/output directories are a well-known source of non-deterministic failures when the same project is built concurrently, and [DoNotParallelize] is the appropriate MSTest mechanism to serialize tests within a class.
No bugs, logic errors, security issues, or API surface concerns were identified. The explanatory comment accurately describes the root cause and chosen mitigation.
No changes requested.
Generated by Expert Code Review (on open) for issue #8292 · ● 1.8M
Problem
TelemetryTests.VSTest_RunTests_SucceedsandVSTest_DiscoverTests_Succeedsare parameterised by target framework via[DynamicData(TargetFrameworks.AllForDynamicData)]and both invoke:dotnet test -c Release {AssetFixture.VSTestProjectPath} --framework {tfm}against the same shared
vstest/TelemetryVSTestProject.csproj. With MSTest parallelism enabled, multiple TFM instances run concurrently and race on MSBuild outputs such asbin/Release/<tfm>/TelemetryVSTestProject.runtimeconfig.jsonand sharedobj/files.Observed CI failure:
Fix
Apply
[DoNotParallelize]at theTelemetryTestsclass level to serialize all tests in the class. This is a one-line change, preserves test coverage, and only has a small impact on suite duration.Blocked PRs
This flake has been intermittently blocking: #8284 #8259 #8234 #8268 #8270 #8256 #8255 #8189 #8262 #8237 #8231