Fix VSTest_RunTests_Succeeds flake by serializing TelemetryTests#8294
Fix VSTest_RunTests_Succeeds flake by serializing TelemetryTests#8294Evangelink wants to merge 1 commit into
Conversation
Acceptance tests run with assembly-level method parallelization. The two VSTest_* methods both invoke 'dotnet test' against the shared VSTestProjectPath, so concurrent invocations for the same TFM race on obj/Release/<tfm>/TelemetryVSTestProject.dll and one fails with CS2012. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a flaky failure in MSTest.Acceptance.IntegrationTests caused by method-level parallel execution running two dotnet test -c Release invocations concurrently against the same generated VSTest test asset, leading to CS2012 file-lock collisions during rebuild.
Changes:
- Serialize
TelemetryTestsexecution by applying[DoNotParallelize]at the class level to avoid concurrent builds of the sharedVSTestProjectPathasset.
Show a summary per file
| File | Description |
|---|---|
| test/IntegrationTests/MSTest.Acceptance.IntegrationTests/TelemetryTests.cs | Adds [DoNotParallelize] to prevent parallel execution of tests that rebuild the same shared VSTest asset, eliminating the file-lock race. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 0
Evangelink
left a comment
There was a problem hiding this comment.
✅ 21/21 dimensions clean — no findings.
Summary: The fix correctly identifies the root cause (two dotnet test -c Release invocations competing for obj/Release/<tfm>/TelemetryVSTestProject.dll under assembly-level method parallelization) and applies the minimal, established pattern ([DoNotParallelize]) to serialize the class. The comment on the attribute clearly documents why serialization is needed, which matches the codebase convention for similar acceptance test classes that share a single mutable generated asset.
Dimensions assessed and found clean:
- Flakiness Patterns (D12): The file-system race condition is directly eliminated — no shared mutable path accessed concurrently after this fix.
- Test Isolation (D10):
[DoNotParallelize]at the class level is the correct granularity; only the two VSTest methods truly conflict, but serializing the whole 5-method class is harmless and simpler. - Algorithmic Correctness (D1): The root cause is the parallel build race;
[DoNotParallelize]is a correct, sufficient fix. - Scope & PR Discipline (D21): Single concern, no unrelated changes, clear issue reference in the PR description.
- All other dimensions (API, threading, security, IPC, etc.) are N/A for a one-line test attribute addition.
Generated by Expert Code Review (on open) for issue #8294 · ● 3.4M
Fixes the
VSTest_RunTests_Succeeds(andVSTest_DiscoverTests_Succeeds) flake that is breakingmainon Windowsnet462and Linuxnet8.0:CSC : error CS2012: Cannot open '...\TelemetryTests\vstest\obj\Release\net462\TelemetryVSTestProject.dll' for writing -- The process cannot access the file '...' because it is being used by another process.Root cause
MSTest.Acceptance.IntegrationTestsruns with assembly-level method parallelization ([assembly: Parallelize(Scope = ExecutionScope.MethodLevel, Workers = 0)]inProgram.cs).TelemetryTestshas two methods —VSTest_RunTests_SucceedsandVSTest_DiscoverTests_Succeeds— that both invokedotnet test -c Releaseagainst the sameAssetFixture.VSTestProjectPath. When the two run in parallel for the same TFM, bothdotnet testprocesses try to (re)build intoobj/Release/<tfm>/TelemetryVSTestProject.dlland one of them loses the file-lock race, yieldingCS2012.Fix
Add
[DoNotParallelize]at theTelemetryTestsclass level so the methods that share the asset run serially. The class only has 5 tests, so the perf impact is negligible.This matches the documented pattern for acceptance test classes that share a single generated mutable asset across multiple methods.