-
Notifications
You must be signed in to change notification settings - Fork 355
[fix] Fix HTML logger parallel file collision #15435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 10 commits
b703a2b
a935a8e
20b08d7
7bc6553
2bd9d56
a64c258
ecf5ddb
3cb268d
1070912
6e26e63
ce0bf3e
4780e0e
87a4ba3
81df804
10bb750
2a4f047
4e7ba20
e2f99d1
4ab2a31
04aaf44
0b55ee4
8659640
9215fdd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -143,3 +143,26 @@ After building with `--pack` / `-pack`, validate vstest.console changes by unzip | |||||||||
| - If build fails asking for .NET 4.6 targeting pack, install it from [Microsoft Downloads](https://www.microsoft.com/download/details.aspx?id=48136) | ||||||||||
| - Enable verbose diagnostics: see `docs/diagnose.md` | ||||||||||
| - For debugging, add `Debugger.Launch` at process entry points (testhost.exe, vstest.console.exe) | ||||||||||
|
|
||||||||||
| ## Logger-Specific Notes | ||||||||||
|
|
||||||||||
| ### TRX Logger (`src/Microsoft.TestPlatform.Extensions.TrxLogger/`) | ||||||||||
|
|
||||||||||
| - **Key files:** `TrxLogger.cs` (main), `Utility/Converter.cs` (attachment handling), `Utility/TrxFileHelper.cs` (path utils) | ||||||||||
| - **Flow:** `TestRunCompleteHandler` → compose XML DOM → `ReserveTrxFilePath` → `AdjustRunDeploymentRootForTrxSubdirectory` → `PopulateTrxFile` | ||||||||||
| - **Test helper:** `TestableTrxLogger` overrides `PopulateTrxFile` to capture TRX file path. Call `MakeTestRunComplete()` to trigger the full flow. | ||||||||||
| - **Important:** Targets netstandard2.0 — use `TrxFileHelper.MakePathRelative()` instead of `Path.GetRelativePath()` | ||||||||||
|
|
||||||||||
| ### HTML Logger (`src/Microsoft.TestPlatform.Extensions.HtmlLogger/`) | ||||||||||
|
|
||||||||||
| - **Key files:** `HtmlLogger.cs` (main), `HtmlTransformer.cs` (XSLT) | ||||||||||
| - **Flow:** `TestRunCompleteHandler` → `PopulateHtmlFile` → create temp XML → serialize → XSLT to HTML → delete XML | ||||||||||
| - **Temp XML naming:** `TestResult_{user}_{machine}_{timestamp}_{pid}.xml` — PID ensures cross-process uniqueness | ||||||||||
|
nohwnd marked this conversation as resolved.
Outdated
|
||||||||||
| - **File creation:** Uses `FileMode.CreateNew` for atomic creation with retry on collision | ||||||||||
|
||||||||||
| - **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 |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -33,7 +33,6 @@ public class HtmlLogger : ITestLoggerWithParameters | |||||
| private readonly IFileHelper _fileHelper; | ||||||
| private readonly XmlObjectSerializer _xmlSerializer; | ||||||
| private readonly IHtmlTransformer _htmlTransformer; | ||||||
| private static readonly object FileCreateLockObject = new(); | ||||||
| private Dictionary<string, string?>? _parametersDictionary; | ||||||
|
|
||||||
| public HtmlLogger() | ||||||
|
|
@@ -345,18 +344,21 @@ private void PopulateHtmlFile() | |||||
|
|
||||||
| private string GenerateUniqueFilePath(string fileName, string fileExtension) | ||||||
| { | ||||||
| string fullFilePath; | ||||||
| for (short i = 0; i < short.MaxValue; i++) | ||||||
| { | ||||||
| var fileNameWithIter = i == 0 ? fileName : Path.GetFileNameWithoutExtension(fileName) + $"[{i}]"; | ||||||
| fullFilePath = Path.Combine(TestResultsDirPath!, $"TestResult_{fileNameWithIter}.{fileExtension}"); | ||||||
| lock (FileCreateLockObject) | ||||||
| var fullFilePath = Path.Combine(TestResultsDirPath!, $"TestResult_{fileNameWithIter}.{fileExtension}"); | ||||||
|
|
||||||
| try | ||||||
| { | ||||||
| if (!File.Exists(fullFilePath)) | ||||||
| { | ||||||
| using var _ = File.Create(fullFilePath); | ||||||
| return fullFilePath; | ||||||
| } | ||||||
| // Use FileMode.CreateNew for atomic "create if not exists" to avoid | ||||||
| // cross-process race conditions when multiple vstest processes run in parallel. | ||||||
| using var _ = new FileStream(fullFilePath, FileMode.CreateNew, FileAccess.Write, FileShare.None); | ||||||
| return fullFilePath; | ||||||
| } | ||||||
| catch (IOException) when (File.Exists(fullFilePath)) | ||||||
|
nohwnd marked this conversation as resolved.
Outdated
|
||||||
| catch (IOException) when (File.Exists(fullFilePath)) | |
| catch (IOException) when (_fileHelper.Exists(fullFilePath)) |
Uh oh!
There was an error while loading. Please reload this page.