Pre-build VSTest project and use --no-build for TelemetryTests#8312
Conversation
The VSTest_RunTests_Succeeds and VSTest_DiscoverTests_Succeeds tests were still flaking on SDK 11 preview with: error MSB4018: The `GenerateRuntimeConfigurationFiles` task failed unexpectedly. System.IO.IOException: The process cannot access the file '...bin/Release/<tfm>/TelemetryVSTestProject.runtimeconfig.json' because it is being used by another process. Even though [DoNotParallelize] (PR #8292) serializes the test methods, `dotnet test` re-runs the build (and `GenerateRuntimeConfigurationFiles`) for each invocation, and the shared bin/<tfm>/ outputs were racing with leftover handles (MSBuild server / lingering testhost) across serialized invocations. Build the VSTest project lazily once per TFM via a SemaphoreSlim+HashSet on the fixture, then run `dotnet test ... --no-build --no-restore` so the file-writing build targets do not run at test time at all. Keep [DoNotParallelize] as defense in depth. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR reduces TelemetryTests flakiness by pre-building the shared VSTest test asset per target framework and running VSTest invocations without rebuilding/restoring during each test.
Changes:
- Adds a fixture-level pre-build helper guarded by a semaphore and per-TFM cache.
- Updates VSTest run and discovery tests to call the pre-build helper and use
--no-build --no-restore. - Keeps class-level serialization as defense in depth for shared build outputs.
Show a summary per file
| File | Description |
|---|---|
test/IntegrationTests/MSTest.Acceptance.IntegrationTests/TelemetryTests.cs |
Pre-builds the VSTest asset once per TFM and updates dotnet test calls to avoid per-test build/restore races. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 0
Evangelink
left a comment
There was a problem hiding this comment.
The fix correctly addresses the GenerateRuntimeConfigurationFiles race by pre-building once per TFM via a SemaphoreSlim+HashSet guard and then passing --no-build --no-restore. One logic bug: the ExitCode != 0 guard in EnsureVSTestProjectBuiltAsync is unreachable dead code because DotnetCli.RunAsync defaults to failIfReturnValueIsNotZero: true and throws before the caller can inspect the exit code. See inline comment.
Generated by Expert Code Review (on open) for issue #8312 · ● 3.9M
- Remove unreachable ExitCode!=0 check (DotnetCli.RunAsync already throws on non-zero exit by default) - Use collection expression [with(comparer)] syntax for HashSet to fix IDE0028 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Follow-up to #8292 which only added
[DoNotParallelize]toTelemetryTests.The
VSTest_RunTests_SucceedsandVSTest_DiscoverTests_Succeedscases were still flaking on SDK 11 preview (11.0.100-preview.5.26227.104) with:Even though
[DoNotParallelize]serializes the test methods inside the class, everydotnet testinvocation still re-runs the build (andGenerateRuntimeConfigurationFiles), and the sharedbin/<tfm>/outputs were racing with leftover MSBuild / testhost handles across serialized invocations.Fix
EnsureVSTestProjectBuiltAsync(tfm, ct)on the fixture, gated by aSemaphoreSlim+HashSet<string>so the VSTest project is built exactly once per TFM. The TFM is only cached after a successful build to avoid poisoning on cancellation/failure.VSTest_*tests to call the pre-build then rundotnet test ... --no-build --no-restoreso the build/restore targets that write the locked file never run during the actual tests.[DoNotParallelize]as defense in depth.Verification
dotnet buildofMSTest.Acceptance.IntegrationTests.csprojsucceeds.