Skip to content

Fix flaky Hex1bNativePublishingTests: read MSBuild query results from -getResultOutputFile - #21524

Merged
Mitch Denny (mitchdenny) merged 2 commits into
mainfrom
aspireagent/fix-hex1b-native-publishing-json-parse
Nov 5, 2026
Merged

Mitch Denny (mitchdenny) merged 2 commits into
mainfrom
aspireagent/fix-hex1b-native-publishing-json-parse

Conversation

@davidfowl

Copy link
Copy Markdown
Collaborator

Description

Hex1bNativePublishingTests.NativeAssetsKeepTheirLayoutAndContents fails intermittently on Infrastructure (ubuntu-latest) with:

System.Text.Json.JsonReaderException : 'M' is invalid after a single JSON value. Expected end of data. LineNumber: 209 | BytePositionInLine: 0.

Root cause

The MSBuild JSON is not truncated. In every failing log I checked (runs 37668103720, 37782413043 and 35756256834), the complete -getItem JSON is followed on stdout by the dotnet CLI's MSBuild-server client fallback diagnostic:

MSBuild server unavailable: could not connect to the server within the timeout window; the server may have failed to start. Falling back to an in-process build.

With SDK 11, MSBuild server is used by default for invocations that run a target (-t:CopyTestFiles -getItem:...). When the runner is loaded (heartbeat showed CPU 97% in run 37668103720), the client can't connect in time, falls back to an in-process build and prints that line after the results. The tests parsed raw stdout, so JsonDocument.Parse failed on the trailing M.

Other tests in this repo already guard against this exact message: MicrosoftTestingPlatformTests uses -getResultOutputFile, and TemplateManifestTestEnvironment uses DOTNET_CLI_USE_MSBUILD_SERVER=0.

Fix

The test now reads its MSBuild query results from -getResultOutputFile:<path> instead of stdout.

  • A new RunMSBuildQueryAsync helper appends the switch and returns the file contents.
  • Every -getProperty/-getItem caller in the class now uses it. That covers the four JSON parsers and the single-property Bundle.proj Configuration check, which compared trimmed stdout and was exposed to the same noise.
  • The result file uses exactly the same format MSBuild would otherwise print: JSON, or the raw value for a single property.
  • MSBuild-server behavior is unchanged, and diagnostics stay in the captured output for failure messages.

No assertions were weakened, and no sleeps or retries were added.

Regression test

NativeAssetsIgnoreMSBuildServerFallbackDiagnostics runs the real native-asset query, with all of its existing assertions, through a dotnet wrapper (.sh on Unix, .cmd on Windows). The wrapper runs the real CLI and then writes the exact CI fallback line to stdout. The test also asserts the line was captured, so it cannot pass by skipping the noisy scenario.

Result
Before fix (stdout parsing) 3/3 failed: 'M' is invalid after a single JSON value. Expected end of data. LineNumber: 209, identical to CI
After fix 10/10 passed for the regression test, and 4× the full Hex1bNativePublishingTests class (43 tests) with 43/43 passing each time

Local runs used Linux and SDK 11.0.100-rc.2.

Denominator

5 tracked occurrences across both issues (2026-09-21 to 2026-10-08; 3 on main, 2 on PRs), against about 1,484 ci.yml runs from 2026-09-21 to 2026-10-07. Different theory rows failed (win-x64 and win-arm64, with and without single file), which fits noise from the environment rather than one data row.

I noticed that ResourceGeneratorMetadataTests also parses merged msbuild -getItem output from a script, but I left it unchanged to keep this PR focused.

Fixes #20799
Fixes #20272

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
    • No

🤖 Posted by AspireAgent (AI Agent)

Under CI load the dotnet CLI can fail to reach the MSBuild server and print
'MSBuild server unavailable: ... Falling back to an in-process build.' to
standard output after the -getItem/-getProperty JSON. The tests parsed raw
stdout, so JsonDocument.Parse failed on the trailing 'M'.

Read query results from MSBuild's -getResultOutputFile instead and add a
regression test that injects the exact fallback line through a dotnet wrapper.

Fixes #20799
Fixes #20272

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 21524

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 21524"

@aspire-repo-bot
aspire-repo-bot Bot requested a balanced review from Copilot October 31, 2026 05:17
@github-actions github-actions Bot added the area-engineering-systems infrastructure helix infra engineering repo stuff label Oct 31, 2026
@github-actions

This comment has been minimized.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The Windows regression test launches a .cmd wrapper directly with shell execution disabled, causing Process.Start to throw before MSBuild runs.

1 open issue
What changed in this PR

This PR addresses flaky MSBuild query parsing in infrastructure tests by separating query results from CLI diagnostics, targeting the failures reported in #20799 and #20272.

Changes:

  • Reads all MSBuild property and item queries in the class through -getResultOutputFile.
  • Adds a regression test that injects the MSBuild-server fallback diagnostic into stdout while preserving native-asset assertions.
  • Retains captured diagnostics for failure reporting.
File Description
tests/​Infrastructure.Tests/​Hex1bNativePublishingTests.cs Adds file-backed query results and diagnostic-injection regression coverage.

Review effort: Balanced

Address review feedback: route RunDotNetAsync through the shared
ProcessStartInfoHelper.SetCommand so .cmd wrappers run via cmd.exe with
correct argument quoting while stdout/stderr stay redirected.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

Tests selector

1 / 100 PR test projects · 0 PR jobs, from 2 changed files.

Selected PR test projects (1 / 100)

Infrastructure.Tests

Selected PR jobs (0)

none


How these were chosen — grouped by what changed

🧪 tests/Infrastructure.Tests/Hex1bNativePublishingTests.cs (changed test)
→ 1 directly: Infrastructure.Tests

🧪 tests/Infrastructure.Tests/Infrastructure.Tests.csproj (changed test)
→ 1 directly: Infrastructure.Tests

Job reasons

none


Selection computed for commit 2d31170.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues block the change to separate MSBuild query results from stdout diagnostics.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@davidfowl
David Fowler (davidfowl) marked this pull request as ready for review October 31, 2026 05:55
@mitchdenny
Mitch Denny (mitchdenny) merged commit 3539974 into main Nov 5, 2026
49 checks passed
@microsoft-github-policy-service microsoft-github-policy-service Bot added this to the 17.0 milestone Nov 5, 2026
@aspire-repo-bot

Copy link
Copy Markdown
Contributor

✅ No documentation update needed.

docs_optional → test_only. No signals triggered (signal_count == 0); the change fixes a flaky infrastructure test (Hex1bNativePublishingTests) by reading MSBuild results from -getResultOutputFile. Only test files changed, with no user-facing surface.

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

Labels

area-engineering-systems infrastructure helix infra engineering repo stuff

Projects

None yet

3 participants