Skip to content
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
b703a2b
Add skill writing skill
nohwnd Mar 4, 2026
a935a8e
Skill writing skill
nohwnd Mar 4, 2026
20b08d7
Make vstest-build-test skill cross-platform with OS mismatch detection
invalid-email-address Mar 4, 2026
7bc6553
Improve validate-skills skill with Windows support and ENV_ISSUE clas…
invalid-email-address Mar 4, 2026
2bd9d56
Fix build exit code propagation and skill documentation
invalid-email-address Mar 4, 2026
a64c258
Fix HTML logger parallel file collision (#15404)
invalid-email-address Mar 4, 2026
ecf5ddb
Update copilot-instructions.md and skill docs with build/test guides
invalid-email-address Mar 4, 2026
3cb268d
Fix HTML logger parallel file collision (#15404)
invalid-email-address Mar 4, 2026
1070912
Merge branch 'main' into fix/html-logger-parallel-file-collision
nohwnd Mar 5, 2026
6e26e63
Apply suggestions from code review
nohwnd Mar 5, 2026
ce0bf3e
Update .github/copilot-instructions.md
nohwnd Mar 5, 2026
4780e0e
Update .github/skills/vstest-build-test/SKILL.md
nohwnd Mar 5, 2026
87a4ba3
Update .github/skills/vstest-build-test/SKILL.md
nohwnd Mar 5, 2026
81df804
Update eng/common/tools.ps1
nohwnd Mar 5, 2026
10bb750
Apply suggestions from code review
nohwnd Mar 24, 2026
2a4f047
Merge branch 'main' into fix/html-logger-parallel-file-collision
nohwnd Mar 24, 2026
4e7ba20
Fix HtmlLogger collision handling
nohwnd Apr 10, 2026
e2f99d1
Merge origin/main into HTML logger fix
nohwnd Apr 10, 2026
4ab2a31
Fix HtmlLogger test assertion
nohwnd Apr 10, 2026
04aaf44
Merge remote-tracking branch 'origin/main' into fix/html-logger-paral…
nohwnd May 13, 2026
0b55ee4
fix: use _fileHelper.Exists instead of File.Exists in GenerateUniqueF…
nohwnd May 13, 2026
8659640
fix: correct mock setup in retry collision test
nohwnd May 13, 2026
9215fdd
Merge branch 'main' into fix/html-logger-parallel-file-collision
nohwnd May 14, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,87 @@ Anytime you add a new localization resource, you MUST:
- Add a corresponding entry in the localization resource file.
- Add an entry in all `*.xlf` files related to the modified `.resx` file.
- Do not modify existing entries in '*.xlf' files unless you are also modifying the corresponding `.resx` file.

## Build & Test Commands

### Full Build

```bash
# Windows
build.cmd
# Unix
./build.sh
```

### Building a Specific Project

```bash
dotnet build src/<ProjectName>/<ProjectName>.csproj --no-restore
```

### Running Unit Tests for a Specific Project

```bash
# Run all TFMs
dotnet test test/<TestProjectName>/<TestProjectName>.csproj --no-build

# Run a specific TFM
dotnet test test/<TestProjectName>/<TestProjectName>.csproj --no-build -f net9.0

# Run a specific test
dotnet test test/<TestProjectName>/<TestProjectName>.csproj --no-build -f net9.0 --filter "TestMethodName"
```

### Key Test Projects

| Component | Test Project |
|---|---|
| TRX Logger | `test/Microsoft.TestPlatform.Extensions.TrxLogger.UnitTests` |
| HTML Logger | `test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests` |
| Object Model | `test/Microsoft.TestPlatform.ObjectModel.UnitTests` |
| Cross-Platform Engine | `test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests` |
| Client | `test/Microsoft.TestPlatform.Client.UnitTests` |

## Repository Structure

- `src/` — Production source code
- `Microsoft.TestPlatform.Extensions.TrxLogger/` — TRX file logger (generates `.trx` test result files)
- `Microsoft.TestPlatform.Extensions.HtmlLogger/` — HTML logger (generates `.html` test reports via XML→XSLT transform)
- `Microsoft.TestPlatform.ObjectModel/` — Shared object model (TestCase, TestResult, etc.)
- `Microsoft.TestPlatform.Common/` — Common utilities
- `Microsoft.TestPlatform.CrossPlatEngine/` — Test execution engine
- `vstest.console/` — CLI console runner
- `test/` — Unit tests (mirrors `src/` structure with `.UnitTests` suffix)
- `eng/` — Build infrastructure
- `scripts/` — Helper scripts

## Logger Architecture Notes

### TRX Logger (`src/Microsoft.TestPlatform.Extensions.TrxLogger/`)

- **`TrxLogger.cs`** — Main logger class. Flow: `TestRunCompleteHandler` → compose XML DOM → `ReserveTrxFilePath` → `AdjustRunDeploymentRootForTrxSubdirectory` → `PopulateTrxFile`.
- **`Utility/Converter.cs`** — Converts VSTest object model to TRX object model. Handles file attachments.
- **`Utility/TrxFileHelper.cs`** — File path utilities. Use `MakePathRelative()` instead of `Path.GetRelativePath()` (netstandard2.0 compat).
- Tests use `TestableTrxLogger` which overrides `PopulateTrxFile` to capture TRX file path.

### HTML Logger (`src/Microsoft.TestPlatform.Extensions.HtmlLogger/`)

- **`HtmlLogger.cs`** — Main logger. Creates temp XML, transforms to HTML via XSLT, deletes XML.
- **`HtmlTransformer.cs`** — XSLT transformation from XML to HTML.
- Temp XML filenames use millisecond-precision timestamps for cross-process uniqueness. File creation uses `FileMode.CreateNew` for atomicity.
- Tests mock `IFileHelper`, `IHtmlTransformer`, and `XmlObjectSerializer`.

## Analyzer Rules

The codebase enforces strict code analysis rules as errors:
- **CA1305**: Always provide `IFormatProvider` (e.g., `CultureInfo.InvariantCulture`) for `ToString()` calls.
- **CA1837**: Use `Environment.ProcessId` instead of `Process.GetCurrentProcess().Id` (net5.0+ only; suppress for net48 targets).
- Source projects target `netstandard2.0`/`net462` — many modern APIs are unavailable. Always check TFM compatibility.

## Task Completion Jingle

When you finish a task (right before calling `task_complete`), play a short celebratory jingle so the user knows you're done:

```powershell
[console]::beep(500,300); [console]::beep(600,300); [console]::beep(700,300); [console]::beep(600,300); [console]::beep(800,500)
```
23 changes: 23 additions & 0 deletions .github/skills/vstest-build-test/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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}.xml` — timestamp has millisecond precision; on collisions a `[n]` suffix is appended
- **File creation:** Uses `FileMode.CreateNew` for atomic creation with retry on collision
- **Tests:** Mock `IFileHelper`, `IHtmlTransformer`, and `XmlObjectSerializer`

### Analyzer Pitfalls

- **CA1305:** Always provide `IFormatProvider` (e.g., `CultureInfo.InvariantCulture`) for `ToString()` calls
- **CA1837 (Environment.ProcessId):** Prefer `Environment.ProcessId` on net5.0+; for this repo (targeting `netstandard2.0`), continue using the existing PID retrieval (e.g., `Process.GetCurrentProcess().Id`) and suppress CA1837 with a brief justification comment.
- Source targets `netstandard2.0` — many modern APIs (`Path.GetRelativePath`, `Environment.ProcessId`) are NOT available here; plan any migrations accordingly.
22 changes: 12 additions & 10 deletions src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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))
Comment thread
nohwnd marked this conversation as resolved.
Outdated

Copilot AI Apr 10, 2026

Copy link

Choose a reason for hiding this comment

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

GenerateUniqueFilePath retries on IOException but uses static File.Exists(fullFilePath) in the catch filter. Since this class already abstracts file operations via IFileHelper, consider using _fileHelper.Exists(fullFilePath) (or checking the specific IOException/HResult for file-exists) so the logic stays consistent and easier to unit test without touching the real filesystem.

Suggested change
catch (IOException) when (File.Exists(fullFilePath))
catch (IOException) when (_fileHelper.Exists(fullFilePath))

Copilot uses AI. Check for mistakes.
{
Comment on lines +360 to 366
// File already exists (another process created it), try next iteration.
Comment thread
nohwnd marked this conversation as resolved.
Outdated
}
}

Expand All @@ -365,7 +367,7 @@ private string GenerateUniqueFilePath(string fileName, string fileExtension)

private static string FormatDateTimeForRunName(DateTime timeStamp)
{
return timeStamp.ToString("yyyyMMdd_HHmmss", DateTimeFormatInfo.InvariantInfo);
return timeStamp.ToString("yyyyMMdd_HHmmss.fffffff", DateTimeFormatInfo.InvariantInfo);
Comment thread
nohwnd marked this conversation as resolved.
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,45 @@ public void TestCompleteHandlerShouldNotDivideByZeroWhenThereAre0TestResults()
Assert.AreEqual(0, _htmlLogger.TestRunDetails.Summary.PassPercentage);
}

[TestMethod]
public void XmlFilePathShouldContainMillisecondTimestampForCrossProcessUniqueness()
{
// Verifies fix for https://github.com/microsoft/vstest/issues/15404
// The temp XML file name should use millisecond-precision timestamps to avoid
// collisions when multiple vstest processes run in parallel.
string? createdFilePath = null;
try
{
_htmlLogger.TestRunCompleteHandler(new object(), new TestRunCompleteEventArgs(null, false, true, null, null, null, TimeSpan.Zero));

Assert.IsNotNull(_htmlLogger.XmlFilePath);
createdFilePath = _htmlLogger.XmlFilePath;

Copilot AI Apr 10, 2026

Copy link

Choose a reason for hiding this comment

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

XmlFilePathShouldContainMillisecondTimestampForCrossProcessUniqueness calls TestRunCompleteHandler without setting up IFileHelper.GetStream for the new FileMode.CreateNew call, so the mock returns null Streams and the test doesn’t reflect real file creation. Consider setting up GetStream to return a disposable Stream (e.g., MemoryStream) and optionally verifying the CreateNew overload is invoked to keep the test robust.

Copilot uses AI. Check for mistakes.

// The filename should match the pattern with milliseconds: yyyyMMdd_HHmmssfff
// e.g., TestResult_user_MACHINE_20260304_153012345.xml
// The time part (HHmmssfff) should be 9 chars; previously it was 6 chars (HHmmss).
var fileName = Path.GetFileNameWithoutExtension(_htmlLogger.XmlFilePath);
var parts = fileName.Split('_');
// The last part is the time portion, possibly with an iteration suffix like [7]
var timePart = parts[parts.Length - 1];
var bracketIdx = timePart.IndexOf('[');
if (bracketIdx >= 0)
{
timePart = timePart.Substring(0, bracketIdx);
}

Assert.AreEqual(9, timePart.Length,
$"Time portion of timestamp should be 9 chars (HHmmssfff) but was '{timePart}' (length {timePart.Length}) in path: '{_htmlLogger.XmlFilePath}'");
}
finally
{
if (!string.IsNullOrEmpty(createdFilePath) && File.Exists(createdFilePath))
{
File.Delete(createdFilePath);
}
}
}

private static TestCase CreateTestCase(string testCaseName)
{
return new TestCase(testCaseName, new Uri("some://uri"), "DummySourceFileName");
Expand Down