Skip to content

Isolate the MSBuild debug-log directory per test process - #15005

Merged
AR-May merged 3 commits into
dotnet:mainfrom
AR-May:ar-may-isolate-test-debug-path
Sep 18, 2026
Merged

AR-May merged 3 commits into
dotnet:mainfrom
AR-May:ar-may-isolate-test-debug-path

Conversation

@AR-May

@AR-May AR-May commented Sep 11, 2026

Copy link
Copy Markdown
Member

Related issue(s):

Context

BuildFailureLogInvariant flakes in msbuild-pr CI, in both directions (Expected: 1, Actual: 2 and Expected: 2, Actual: 1).

It scans three directories for MSBuild_*.txt. Two are process-private; the third — FrameworkDebugUtils.DebugPath, added by #14224 — resolves to the single build-wide folder Arcade sets via MSBUILDDEBUGPATH. Test assemblies run in parallel as separate processes, and AssertInvariant deletes files it considers new before applying its benign-file filters, so they destroy each other's files.

Changes Made

Give each test process a test_<pid> subfolder, applying the isolation pattern already used for the temp path just below it.

Nested under the Arcade path so the logs are still published as a CI artifact. No-op when the variable is unset.

BuildFailureLogInvariant is deliberately unchanged: once all three scanned directories are process-private, its existing logic is correct as written.

@AR-May
AR-May marked this pull request as ready for review September 11, 2026 14:52
Copilot AI lite review requested due to automatic review settings September 11, 2026 14:52
@AR-May
AR-May deployed to copilot-pat-pool September 11, 2026 14:52 — with GitHub Actions Active
@AR-May
AR-May deployed to copilot-pat-pool September 11, 2026 14:53 — with GitHub Actions Active

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

Path normalization and best-effort error handling still need correction.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This pull request isolates MSBuild debug logs per test process while preserving CI artifact collection.

Changes:

  • Adds a test_<pid> debug-log subdirectory.
  • Refreshes the cached framework debug path.
File summaries
File Summary
src/Shared/UnitTests/TestAssemblyInfo.cs Configures process-specific debug logging during test startup.
Review details

Suppressed comments (1)

src/Shared/UnitTests/TestAssemblyInfo.cs:261

  • SetDebugPath() can throw when the configured path is invalid, inaccessible, or becomes too long after adding test_<pid>; its static constructor explicitly catches these failures so MSBuild can continue. Calling it directly here makes test-process startup fail for those environments, so keep this re-resolution best-effort and preserve a safe fallback if nested-directory creation fails.
                // Lets re-resolve FrameworkDebugUtils.DebugPath, which is cached, so it picks up new MSBUILDDEBUGPATH
                FrameworkDebugUtils.SetDebugPath();
  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Shared/UnitTests/TestAssemblyInfo.cs Outdated
Comment thread src/Shared/UnitTests/TestAssemblyInfo.cs Outdated
AR-May and others added 3 commits September 11, 2026 17:43
Arcade sets MSBUILDDEBUGPATH once per build (eng/common/tools.ps1) to
$LogDir/MsbuildDebugLogs. Since dotnet#14224 BuildFailureLogInvariant also scans
FrameworkDebugUtils.DebugPath, which resolves to that single shared folder in
every test process. Test assemblies run in parallel, and the invariant deletes
the files it considers new, so parallel processes saw and destroyed each
other's MSBuild_*.txt files - producing flaky failures in both directions.

Give each test process its own subfolder, nested under the Arcade path so the
logs are still published as a CI artifact.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
SetDebugPath reads the variable through FileUtilities.TrimAndStripAnyQuotes.
Reading it raw here appended the suffix after a trailing quote, stranding that
quote mid-path where Trim can no longer reach it, and let a whitespace-only
value through as a relative path. Normalize first so both cases are handled.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@AR-May
AR-May force-pushed the ar-may-isolate-test-debug-path branch from cfed17f to e95783b Compare September 11, 2026 15:45
Comment thread src/Shared/UnitTests/TestAssemblyInfo.cs
@AR-May

AR-May commented Sep 11, 2026

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 2 pipeline(s).

@AR-May
AR-May enabled auto-merge (squash) September 11, 2026 16:47
@AR-May

AR-May commented Sep 18, 2026

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 2 pipeline(s).

@AR-May
AR-May merged commit b54e8e6 into dotnet:main Sep 18, 2026
14 checks passed
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.

3 participants