Skip to content

Fix VSTest_RunTests_Succeeds flake by serializing TelemetryTests#8294

Open
Evangelink wants to merge 1 commit into
mainfrom
copilot/fix-telemetry-vstest-parallelize
Open

Fix VSTest_RunTests_Succeeds flake by serializing TelemetryTests#8294
Evangelink wants to merge 1 commit into
mainfrom
copilot/fix-telemetry-vstest-parallelize

Conversation

@Evangelink
Copy link
Copy Markdown
Member

Fixes the VSTest_RunTests_Succeeds (and VSTest_DiscoverTests_Succeeds) flake that is breaking main on Windows net462 and Linux net8.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.IntegrationTests runs with assembly-level method parallelization ([assembly: Parallelize(Scope = ExecutionScope.MethodLevel, Workers = 0)] in Program.cs).

TelemetryTests has two methods — VSTest_RunTests_Succeeds and VSTest_DiscoverTests_Succeeds — that both invoke dotnet test -c Release against the same AssetFixture.VSTestProjectPath. When the two run in parallel for the same TFM, both dotnet test processes try to (re)build into obj/Release/<tfm>/TelemetryVSTestProject.dll and one of them loses the file-lock race, yielding CS2012.

Fix

Add [DoNotParallelize] at the TelemetryTests class 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.

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>
Copilot AI review requested due to automatic review settings May 16, 2026 14:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 TelemetryTests execution by applying [DoNotParallelize] at the class level to avoid concurrent builds of the shared VSTestProjectPath asset.
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

Copy link
Copy Markdown
Member Author

@Evangelink Evangelink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants