[fix] Fix HTML logger parallel file collision#15435
Merged
Merged
Conversation
Add pre-build environment setup section that detects when .dotnet contains binaries for the wrong OS and cleans .dotnet, .packages, and artifacts for a fresh bootstrap. Update all build/test commands to show both Linux/macOS and Windows equivalents with a quick-reference table. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sification - Add PowerShell OS detection alongside bash - Add guidance for extracting OS-specific commands from tables - Add placeholder substitution and cross-platform adaptation rules - Use SQL tracking table instead of markdown table - Distinguish ENV_ISSUE from ERROR in result classification - Improve Fix or Flag section with actionable steps per status Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- eng/build.ps1: Forward exit code from Arcade build script using \�xit \0\ to ensure non-zero exit codes are propagated to the caller - eng/common/tools.ps1: Add diagnostic log message before exiting so the exit code is visible in build output - .github/skills/vstest-build-test/SKILL.md: Fix Windows test filter flag from \-p\ to \-projects\ since \-p\ is ambiguous in PowerShell and does not work as expected Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When multiple vstest processes run in parallel with the HTML logger, temporary XML result files could collide because the timestamp-based filename only has second-level precision, and the in-process lock doesn't prevent cross-process races. Changes: - Include process ID in the temp XML filename for cross-process uniqueness - Replace File.Exists + File.Create with atomic FileMode.CreateNew to handle any remaining race conditions gracefully via retry - Remove unused FileCreateLockObject (cross-process lock was ineffective) Fixes: #15404 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add build commands, test project mappings, repository structure, logger architecture notes, and analyzer pitfalls to copilot-instructions.md. Update vstest-build-test skill with logger-specific notes covering TRX and HTML logger architecture, test patterns, and common analyzer issues. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
nohwnd
marked this pull request as draft
March 4, 2026 14:58
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes cross-process temp XML filename collisions in the HTML logger when multiple vstest.console instances run in parallel.
Changes:
- Adds PID to the temp XML filename and switches to
FileMode.CreateNewfor atomic “create if not exists”. - Adds a unit test to validate the PID is present in the generated XML path.
- Updates build/doc tooling and authoring docs for repo build/test guidance.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs | Adds a unit test asserting PID is included in the temp XML filename. |
| src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs | Adds PID to filename and uses FileMode.CreateNew for atomic file creation; removes ineffective in-proc lock. |
| eng/common/tools.ps1 | Adds an extra console message when exiting with an exit code. |
| eng/build.ps1 | Attempts to forward the exit code from the invoked Arcade build script. |
| .github/skills/vstest-build-test/SKILL.md | Adds repo-specific build/test instructions and logger notes. |
| .github/skills/validate-skills/SKILL.md | Adds a procedure for validating that documented skill commands work. |
| .github/skills/creating-skills/SKILL.md | Adds guidance for creating skill files and frontmatter. |
| .github/copilot-instructions.md | Adds build/test command guidance and repository structure notes. |
When multiple vstest processes run in parallel with the HTML logger, temporary XML result files could collide because the timestamp-based filename only has second-level precision, and the in-process lock doesn't prevent cross-process races. Changes: - Add millisecond precision to temp XML filename timestamp (HHmmss → HHmmssfff) to greatly reduce collision likelihood across parallel processes - Replace File.Exists + File.Create with atomic FileMode.CreateNew to handle any remaining race conditions gracefully via retry - Remove unused FileCreateLockObject (cross-process lock was ineffective) Fixes: #15404 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
nohwnd
force-pushed
the
fix/html-logger-parallel-file-collision
branch
from
March 4, 2026 15:04
1d29994 to
3cb268d
Compare
Member
Author
|
needs merge of #15434 |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Comment on lines
+160
to
+161
| - **Temp XML naming:** `TestResult_{user}_{machine}_{timestamp}_{pid}.xml` — PID ensures cross-process uniqueness | ||
| - **File creation:** Uses `FileMode.CreateNew` for atomic creation with retry on collision |
There was a problem hiding this comment.
Same issue as in copilot-instructions.md: the documented filename pattern includes {pid} but the actual code does not add a PID to the filename. The uniqueness strategy relies on millisecond timestamps plus FileMode.CreateNew with retry. This documentation should be corrected to match the actual implementation.
Suggested change
| - **Temp XML naming:** `TestResult_{user}_{machine}_{timestamp}_{pid}.xml` — PID ensures cross-process uniqueness | |
| - **File creation:** Uses `FileMode.CreateNew` for atomic creation with retry on collision | |
| - **Temp XML naming:** `TestResult_{user}_{machine}_{timestamp}.xml` — timestamp-based name only (no PID) | |
| - **File creation:** Uses `FileMode.CreateNew` for atomic creation with retry on collision; millisecond timestamps + `CreateNew` retries provide uniqueness |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Youssef1313
reviewed
Mar 6, 2026
Youssef1313
reviewed
Mar 6, 2026
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Youssef Victor <youssefvictor00@gmail.com>
This was referenced Jul 18, 2026
Merged
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.
Summary
Fixes #15404
When multiple vstest.console processes run in parallel with the HTML logger, temporary XML result files can collide because:
yyyyMMdd_HHmmss) only has second-level precisionlock (FileCreateLockObject)doesn't prevent cross-process racesChanges
src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.csHHmmsstoHHmmssfff, greatly reducing collision likelihood across parallel processesFileMode.CreateNewfor atomic file creation - replaces theFile.Exists+File.Createpattern with an atomic create-if-not-exists operation that retries onIOExceptionFileCreateLockObject- the cross-process lock was ineffectivetest/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.csXmlFilePathShouldContainMillisecondTimestampForCrossProcessUniquenesstestDocumentation
.github/copilot-instructions.mdwith build/test commands, repo structure, and logger architecture.github/skills/vstest-build-test/SKILL.mdwith logger-specific notesTesting
All 34 HTML logger unit tests pass on both net9.0 and net48.