Repository navigation
Serialize Infrastructure.Tests classes that build Aspire.TestTools - #21543
Merged
Eric Erhardt (eerhardt) merged 2 commits intoNov 5, 2026
Merged
Eric Erhardt (eerhardt) merged 2 commits into
Eric Erhardt (eerhardt) merged 2 commits into
Conversation
GenerateTestSummaryFixture runs 'dotnet build' on GenerateTestSummary (ProjectReference to Aspire.TestTools) while DownloadFailingJobLogsToolTests runs 'dotnet run' on a file-based app with '#:project ../Aspire.TestTools'. The two test classes were in different xunit collections and ran in parallel, so both MSBuild processes could write artifacts/bin/Aspire.TestTools/.../Aspire.TestTools.deps.json at the same time and GenerateDepsFile failed with an IOException. Put both classes in one collection and add a guard test. Fixes microsoft#19637 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 21543Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 21543" |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new theory checks hard-coded collection metadata rather than whether builds overlap, violating the infrastructure tests’ behavioral-testing requirement.
1 open issue
What changed in this PR
Addresses #19637 by serializing the Infrastructure.Tests classes that build Aspire.TestTools, preventing concurrent writes to shared build outputs.
Changes:
- Places both tool test classes in the same xUnit collection.
- Documents the shared-output contention behind the intermittent failure.
- Adds a reflection-based check of collection membership.
| File | Description |
|---|---|
| tests/Infrastructure.Tests/GenerateTestSummary/GenerateTestSummaryToolTests.cs | Joins the shared build collection. |
| tests/Infrastructure.Tests/DownloadFailingJobLogs/DownloadFailingJobLogsToolTests.cs | Joins the same collection to prevent overlapping builds. |
| tests/Infrastructure.Tests/AspireTestToolsBuildCollection.cs | Defines the collection, explains the contention, and checks membership. |
Review effort: Balanced
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
David Fowler (davidfowl)
marked this pull request as ready for review
October 31, 2026 08:21
Eric Erhardt (eerhardt)
approved these changes
Nov 5, 2026
This was referenced Nov 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Fixes #19637
Root cause
Two Infrastructure.Tests classes build
tools/Aspire.TestToolsout of process, and they used to run in parallel:GenerateTestSummaryToolTestsdotnet build tools/GenerateTestSummary/GenerateTestSummary.csproj --restore, which has a ProjectReference to Aspire.TestTools.DownloadFailingJobLogsToolTestsdotnet run tools/scripts/DownloadFailingJobLogs.cs, a file-based app with#:project ../Aspire.TestTools/Aspire.TestTools.csproj.The two classes were in separate xunit collections, so their builds could overlap. Both builds write the same outputs under
artifacts/bin/Aspire.TestTools/Debug/net10.0/. WhenAspire.TestTools.deps.jsonneeds regenerating, both MSBuild processes open it for exclusive write. The loser fails:That is the failure in CI job 97605592624, which made DownloadFailingJobLogsToolTests'
dotnet runexit 1.The other tool fixtures (
CreateFailingTestIssue,ExtractTestPartitions) don't build Aspire.TestTools as a project. CreateFailingTestIssue compiles the shared sources into itself.GenerateTestSummaryToolTestsinvokes the tool with--no-build.Fix
Put both classes in one new xunit collection,
AspireTestToolsBuild. Xunit runs classes in the same collection sequentially, so the fixture build and thedotnet runbuilds can never overlap. There are no retries or sleeps, and test behavior is unchanged.Evidence
Exact failure reproduced deterministically.
Aspire.TestTools.deps.jsonopen withFileShare.None, as a concurrent GenerateDepsFile writer does. The file was stale, so MSBuild had to regenerate it.dotnet build tools/GenerateTestSummary/GenerateTestSummary.csproj --restorefailed with the CI error:error MSB4018: The "GenerateDepsFile" task failed unexpectedly. System.IO.IOException: The process cannot access the file '.../Aspire.TestTools.deps.json' because it is being used by another process.Overlap measured in the real test run. I ran both classes with the real test runner (
--filter-class *GenerateTestSummaryToolTests --filter-class *DownloadFailingJobLogsToolTests). Before each run, I forced a TestTools rebuild by touching a source file and deleting deps.json. A sampler checkedpsevery 100 ms for the fixture'sdotnet buildand adotnet run DownloadFailingJobLogs.csalive at the same time.The IOException itself is a narrow window (the deps.json write takes milliseconds). Locally I forced 40 concurrent build/run pairs and none hit it, which fits the rare CI rate. The overlap that makes it possible was present on every run before the fix and is gone after.
No metadata-only guard. Following
.github/instructions/infrastructure-tests.instructions.md, this PR adds no test that asserts collection attributes. Such a test would restate the source, not observe behavior. Behavioral coverage of the race isn't practical in a unit test: it needs two real MSBuild processes and a stale deps.json, and the conflicting write window is milliseconds. The real-runner evidence above is the verification. Both affected classes pass after the change (10/10 tests on every run).🤖 Posted by AspireAgent (AI Agent)