From b703a2bc0e54117df7e73064cb710bdbbbd119e9 Mon Sep 17 00:00:00 2001 From: nohwnd Date: Wed, 4 Mar 2026 11:41:56 +0100 Subject: [PATCH 01/18] Add skill writing skill --- .github/skills/creating-skills/SKILL.md | 166 ++++++++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 .github/skills/creating-skills/SKILL.md diff --git a/.github/skills/creating-skills/SKILL.md b/.github/skills/creating-skills/SKILL.md new file mode 100644 index 0000000000..2ed42681bc --- /dev/null +++ b/.github/skills/creating-skills/SKILL.md @@ -0,0 +1,166 @@ +--- +name: creating-skills +description: Create custom agent capabilities when discovering novel tools, receiving task-agnostic tips from reviewers, or after researching specialized workflows not covered in existing instructions. Teaches structure, YAML optimization for LLM discoverability, and token efficiency. +--- + +# Creating GitHub Copilot Agent Skills + +This skill teaches you how to create effective GitHub Copilot Agent Skills for this repository. + +## Pre-Check: Avoid Duplication + +**STOP** and check before creating a new skill: + +1. **Does it exist already?** + - List all skills: `ls -la .github/skills/` + - Read existing skill frontmatter and content + - If semantically similar skill exists, STOP + +2. **Should an existing skill be expanded?** + - If frontmatter semantically matches your use case → Update existing skill's description + - Add keywords to improve discoverability rather than creating duplicate + +3. **Should existing skill content change?** + - If frontmatter matches but content incomplete → Add section to existing skill + - Enhance with additional examples, procedures, or troubleshooting + - Update frontmatter only if significantly broadening scope + +**Only create new skill if:** +- No semantic overlap with existing skills +- Addresses distinct problem domain +- Has unique triggering conditions + +## Skill Structure + +### Directory Placement + +Skills should be placed in `.github/skills/` directory: +- **Project skills** (repository-specific): `.github/skills/skill-name/` + +Each skill must have its own subdirectory with a lowercase, hyphenated name that matches the `name` field in the frontmatter. + +### File Requirements + +Every skill directory must contain a `SKILL.md` file (case-sensitive) with: + +1. **YAML Frontmatter** (required): + +2. **Markdown Body** with clear instructions, examples, procedures, guidelines, and references + +### Additional Resources + +Skills can include: +- Scripts (e.g., `.sh`, `.fsx`, `.ps1`) +- Example files +- Templates +- Reference documentation + +## YAML Frontmatter Best Practices + +The frontmatter is critical for skill discoverability and token efficiency: + +### Required Fields + +- **name** (string): Unique identifier, lowercase with hyphens + - Must match the directory name + - Should be descriptive but concise + - Example: `hypothesis-driven-debugging`, `github-actions-failure-debugging` + +- **description** (string): When and why to use this skill + - Should be 1-2 sentences + - Include trigger keywords that help the AI recognize when to load the skill + - Example: "Guide for debugging failing GitHub Actions workflows. Use this when asked to debug failing GitHub Actions workflows." + - **SEO-like optimization for LLMs**: Include key terms that would appear in user requests + +### Optional Fields + +- **license** (string): License for the skill (e.g., MIT, Apache-2.0) + +### Description Guidelines + +The description is crucial for skill discoverability. Think of it like SEO for LLMs: + +✅ **Good descriptions** (specific, actionable, keyword-rich): +- "Guide for debugging failing GitHub Actions workflows. Use this when asked to debug failing GitHub Actions workflows." +- "Systematic approach to investigating F# compiler performance issues using traces, dumps, and benchmarks." +- "Step-by-step process for analyzing test failures using hypothesis-driven debugging." + +❌ **Poor descriptions** (vague, generic): +- "Helps with debugging" +- "Tool for testing" +- "Useful utility" + +### Token Efficiency + +Skills should be concise to avoid wasting context tokens: +- Keep instructions focused and relevant +- Use bullet points and numbered lists +- Avoid redundant information +- Reference external resources rather than duplicating content +- The agent will only load skills when relevant, so clear descriptions help prevent unnecessary loading + +## Skill Content Best Practices + +### Structure + +1. **Title and Overview**: Brief introduction +2. **When to Use**: Clear triggering conditions +3. **Prerequisites**: Required tools, setup, or knowledge +4. **Step-by-Step Instructions**: Numbered procedures +5. **Examples**: Concrete use cases +6. **Troubleshooting**: Common issues +7. **References**: Links to related documentation + +### Writing Style + +- Use imperative mood ("Run the test", not "You should run the test") +- Be specific and actionable +- Include command examples with expected output +- Use code blocks with language identifiers +- Highlight warnings and critical information +- Reference tools and APIs that the agent has access to + +### Examples + +Always include concrete examples: +- Command invocations with flags and arguments +- Expected output and how to interpret it +- Common variations and edge cases +- Links to real-world usage in the repository + +## Testing Your Skill + +After creating a skill: + +1. Verify the file structure: + ```bash + ls -la .github/skills/your-skill-name/ + # Should show SKILL.md and any additional resources + ``` + +2. Validate YAML frontmatter: + - Ensure proper YAML syntax + - Required fields are present + - Name matches directory name + +3. Test skill invocation: + - Ask Copilot a question that should trigger the skill + - Verify the skill is loaded (check response for skill-specific guidance) + - Ensure instructions are clear and actionable + +4. Iterate based on usage: + - Monitor how often the skill is used + - Refine description for better discoverability + - Update instructions based on feedback + +## Examples from This Repository + +See existing skills in `.github/skills/` for reference: +- `hypothesis-driven-debugging`: Systematic failure investigation +- Additional skills may be added over time + +## References + +- [GitHub Copilot Agent Skills Documentation](https://docs.github.com/en/copilot/concepts/agents/about-agent-skills) +- [Agent Skills Open Standard](https://github.com/agentskills/agentskills) +- [Community Skills Collection](https://github.com/github/awesome-copilot) \ No newline at end of file From a935a8e335d362afdf3f841fcdc9f441ebf871f8 Mon Sep 17 00:00:00 2001 From: nohwnd Date: Wed, 4 Mar 2026 12:45:29 +0100 Subject: [PATCH 02/18] Skill writing skill --- .github/skills/vstest-build-test/SKILL.md | 86 +++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 .github/skills/vstest-build-test/SKILL.md diff --git a/.github/skills/vstest-build-test/SKILL.md b/.github/skills/vstest-build-test/SKILL.md new file mode 100644 index 0000000000..e452d502d9 --- /dev/null +++ b/.github/skills/vstest-build-test/SKILL.md @@ -0,0 +1,86 @@ +--- +name: vstest-build-test +description: Build, test, and validate changes in the vstest repository. Use when building vstest projects, running unit tests, smoke tests, or acceptance tests, or when deploying locally built vstest.console for manual testing. +--- + +# Building and Testing vstest + +## Build + +### Full Build (Recommended) + +For projects with many cross-project dependencies (e.g., HtmlLogger, TrxLogger, vstest.console): + +```powershell +./build.cmd -pack +``` + +This produces NuGet packages under `artifacts/packages/Debug/Shipping/`. + +### Single Project Build + +For isolated projects with few dependencies: + +```powershell +./build.cmd -project +``` + +> **Warning:** This does NOT work for projects like HtmlLogger that have many transitive dependencies. Use `-pack` instead. + +### Release Configuration + +```powershell +./build.cmd -c Release -pack +``` + +## Test + +### Unit Tests (Default) + +```powershell +./test.cmd +``` + +### Specific Test Assembly + +Use `-p` to filter by assembly name pattern: + +```powershell +./test.cmd -p htmllogger # HTML logger tests +./test.cmd -p trxlogger # TRX logger tests +./test.cmd -p datacollector # Data collector tests +./test.cmd -p smoke # Smoke tests +``` + +### Specific Test by Name + +```powershell +./test.cmd -bl -c release /p:TestRunnerAdditionalArguments="'--filter TestName'" -Integration +``` + +## Manual Validation with vstest.console + +After `./build.cmd -pack`, validate vstest.console changes by unzipping the built package: + +1. Locate the package: `artifacts/packages/Debug/Shipping/Microsoft.TestPlatform.-dev.nupkg` +2. Unzip it (`.nupkg` files are ZIP archives) +3. Run the local vstest.console against a test project + +### Alternative: Direct Artifact Paths + +- **xplat (netcoreapp):** `artifacts//netcoreapp1.0/vstest.console.dll` +- **Windows desktop:** `artifacts//net46/win7-x64/vstest.console.exe` + +## Test Categories + +| Category | Speed | What it tests | Filter | +|---|---|---|---| +| Unit tests | Fast | Individual units | `./test.cmd` (default) | +| Smoke tests | Slow | P0 end-to-end scenarios | `./test.cmd -p smoke` | +| Acceptance tests | Slowest | Extensive coverage | `-Integration` flag | + +## Troubleshooting + +- 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) From 20b08d7d83a5fcf3c5175832171c9f9ef98e617e Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 4 Mar 2026 11:57:44 +0000 Subject: [PATCH 03/18] Make vstest-build-test skill cross-platform with OS mismatch detection 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> --- .github/skills/vstest-build-test/SKILL.md | 99 ++++++++++++++++++----- 1 file changed, 79 insertions(+), 20 deletions(-) diff --git a/.github/skills/vstest-build-test/SKILL.md b/.github/skills/vstest-build-test/SKILL.md index e452d502d9..1b21c0e574 100644 --- a/.github/skills/vstest-build-test/SKILL.md +++ b/.github/skills/vstest-build-test/SKILL.md @@ -5,13 +5,60 @@ description: Build, test, and validate changes in the vstest repository. Use whe # Building and Testing vstest +## Pre-Build: Environment Setup + +Before building, verify the `.dotnet` toolchain matches the current OS. The repo bootstraps its own .NET SDK into `.dotnet/`. + +### Detect OS vs .dotnet Mismatch + +Run this check **before every first build in a session**: + +```bash +# Determine current OS +OS=$(uname -s) # "Linux", "Darwin" (macOS), or contains "MINGW"/"MSYS" (Windows/Git Bash) + +if [ -d ".dotnet" ]; then + if [ "$OS" = "Linux" ] || [ "$OS" = "Darwin" ]; then + # On Linux/macOS the dotnet binary must be an ELF/Mach-O executable, not .exe + if [ -f ".dotnet/dotnet.exe" ] && [ ! -f ".dotnet/dotnet" ]; then + echo "MISMATCH: .dotnet contains Windows binaries but OS is $OS" + rm -rf .dotnet .packages artifacts + echo "Cleaned .dotnet, .packages, and artifacts for fresh bootstrap" + fi + else + # On Windows the dotnet binary should be dotnet.exe + if [ -f ".dotnet/dotnet" ] && [ ! -f ".dotnet/dotnet.exe" ]; then + echo "MISMATCH: .dotnet contains Linux/macOS binaries but OS is Windows" + rm -rf .dotnet .packages artifacts + echo "Cleaned .dotnet, .packages, and artifacts for fresh bootstrap" + fi + fi +fi +``` + +After cleanup (or if `.dotnet` doesn't exist), the build script automatically downloads the correct SDK version from `global.json`. + ## Build +### Platform Commands + +| Action | Windows | Linux / macOS | +|---|---|---| +| Restore + Build | `./build.cmd` | `./build.sh` | +| Restore only | `./restore.cmd` | `./restore.sh` | +| Build + Pack | `./build.cmd -pack` | `./build.sh --pack` | +| Release config | `./build.cmd -c Release -pack` | `./build.sh -c Release --pack` | +| Single project | `./build.cmd -project ` | `./build.sh --projects ` | + ### Full Build (Recommended) For projects with many cross-project dependencies (e.g., HtmlLogger, TrxLogger, vstest.console): -```powershell +```bash +# Linux / macOS +./build.sh --pack + +# Windows ./build.cmd -pack ``` @@ -21,23 +68,25 @@ This produces NuGet packages under `artifacts/packages/Debug/Shipping/`. For isolated projects with few dependencies: -```powershell +```bash +# Linux / macOS +./build.sh --projects + +# Windows ./build.cmd -project ``` -> **Warning:** This does NOT work for projects like HtmlLogger that have many transitive dependencies. Use `-pack` instead. - -### Release Configuration - -```powershell -./build.cmd -c Release -pack -``` +> **Warning:** This does NOT work for projects like HtmlLogger that have many transitive dependencies. Use `--pack` / `-pack` instead. ## Test ### Unit Tests (Default) -```powershell +```bash +# Linux / macOS +./test.sh + +# Windows ./test.cmd ``` @@ -45,22 +94,31 @@ For isolated projects with few dependencies: Use `-p` to filter by assembly name pattern: -```powershell -./test.cmd -p htmllogger # HTML logger tests -./test.cmd -p trxlogger # TRX logger tests -./test.cmd -p datacollector # Data collector tests -./test.cmd -p smoke # Smoke tests +```bash +# Linux / macOS +./test.sh -p htmllogger # HTML logger tests +./test.sh -p trxlogger # TRX logger tests +./test.sh -p datacollector # Data collector tests +./test.sh -p smoke # Smoke tests + +# Windows +./test.cmd -p htmllogger +./test.cmd -p smoke ``` ### Specific Test by Name -```powershell +```bash +# Windows ./test.cmd -bl -c release /p:TestRunnerAdditionalArguments="'--filter TestName'" -Integration + +# Linux / macOS +./test.sh -bl -c release /p:TestRunnerAdditionalArguments="'--filter TestName'" --integrationTest ``` ## Manual Validation with vstest.console -After `./build.cmd -pack`, validate vstest.console changes by unzipping the built package: +After building with `--pack` / `-pack`, validate vstest.console changes by unzipping the built package: 1. Locate the package: `artifacts/packages/Debug/Shipping/Microsoft.TestPlatform.-dev.nupkg` 2. Unzip it (`.nupkg` files are ZIP archives) @@ -75,12 +133,13 @@ After `./build.cmd -pack`, validate vstest.console changes by unzipping the buil | Category | Speed | What it tests | Filter | |---|---|---|---| -| Unit tests | Fast | Individual units | `./test.cmd` (default) | -| Smoke tests | Slow | P0 end-to-end scenarios | `./test.cmd -p smoke` | -| Acceptance tests | Slowest | Extensive coverage | `-Integration` flag | +| Unit tests | Fast | Individual units | `./test.sh` / `./test.cmd` (default) | +| Smoke tests | Slow | P0 end-to-end scenarios | `-p smoke` | +| Acceptance tests | Slowest | Extensive coverage | `--integrationTest` / `-Integration` flag | ## Troubleshooting +- **OS mismatch errors:** If you see SDK load failures, run the mismatch detection script above to clean and re-bootstrap. - 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) From 7bc6553b9c0e6a45ad6a9a0f2659392bde24ceb2 Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 4 Mar 2026 13:26:30 +0100 Subject: [PATCH 04/18] Improve validate-skills skill with Windows support and ENV_ISSUE classification - 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> --- .github/skills/validate-skills/SKILL.md | 98 +++++++++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 .github/skills/validate-skills/SKILL.md diff --git a/.github/skills/validate-skills/SKILL.md b/.github/skills/validate-skills/SKILL.md new file mode 100644 index 0000000000..07a5a9ff09 --- /dev/null +++ b/.github/skills/validate-skills/SKILL.md @@ -0,0 +1,98 @@ +--- +name: validate-skills +description: Validate that commands documented in skill files actually work. Use when creating, updating, or reviewing skills to ensure all documented commands exit with code 0. +--- + +# Validating Skills + +Verify every executable command in a skill runs successfully on the current OS. + +## When to Use + +- After creating or updating a skill that contains executable commands +- During skill review to catch stale or broken instructions +- When switching OS (e.g. Windows → Linux) to confirm cross-platform commands + +## Procedure + +### 1. Detect Current OS + +Determine which platform commands to extract: + +```powershell +# PowerShell (Windows) +$os = "Windows" +``` + +```bash +# Bash (Linux / macOS) +OS=$(uname -s) # "Linux" or "Darwin" +``` + +### 2. Extract Commands + +Parse the target skill's `SKILL.md` and list every shell command for the detected OS: + +- Many skills document commands in tables with **Windows** and **Linux / macOS** columns. Pick the column matching your OS. +- If a command contains comments like `# Windows` or `# Linux / macOS`, only run the one for your OS. +- **Placeholder substitution:** Replace obvious placeholders (e.g. ``, ``) with real values from the repo. If no sensible value exists, skip the command. +- **Adapt cross-platform commands:** Commands like `ls -la` should be adapted to PowerShell equivalents (`Get-ChildItem`) on Windows when no native Windows command is documented. + +### 3. Track Results + +Use the SQL tool to create a tracking table: + +```sql +CREATE TABLE skill_commands ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + skill TEXT NOT NULL, + command TEXT NOT NULL, + expected_exit INTEGER DEFAULT 0, + actual_exit INTEGER, + status TEXT DEFAULT 'pending', + notes TEXT +); +``` + +### 4. Run Each Command + +For every extracted command: + +1. Run it from the repo root +2. Record the exit code +3. Classify the result: + - **Exit 0 → PASS** + - **Non-zero + environment issue (missing SDK, no internet) → ENV_ISSUE** + - **Non-zero + command/docs wrong → ERROR** + +### 5. Safety Rules + +> **CRITICAL:** Never run unfiltered integration/acceptance tests. They take hours. +> - `test.sh --integrationTest` or `test.cmd -Integration` **MUST** include a `--filter` or `-p` flag. +> - `test.sh -p smoke` is acceptable (scoped to smoke tests), but expect it to be slow. + +### 6. Report + +After all commands finish, print a summary: + +``` +=== Skill Validation Report === +Skill: +OS: +Commands tested: N +PASS: X +ENV_ISSUE: Y (list with reasons) +ERROR: Z (list failed commands with exit codes) +``` + +### 7. Fix or Flag + +- **ERROR (documentation bug):** Update the skill's `SKILL.md` to fix the command. +- **ENV_ISSUE:** Add a troubleshooting note to the skill if the environment prerequisite is not already documented. +- **PASS:** No action needed. + +## Ordering Tips + +- Run restore/build before tests (tests depend on build output) +- Run the cheapest commands first to fail fast +- Batch independent test commands in parallel when possible From 2bd9d56b27b73f5c341b2c9fe624be49cde817da Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 4 Mar 2026 14:38:31 +0100 Subject: [PATCH 05/18] Fix build exit code propagation and skill documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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> --- .github/skills/vstest-build-test/SKILL.md | 6 +++--- eng/build.ps1 | 4 +++- eng/common/tools.ps1 | 1 + 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/skills/vstest-build-test/SKILL.md b/.github/skills/vstest-build-test/SKILL.md index 1b21c0e574..3c061b2c28 100644 --- a/.github/skills/vstest-build-test/SKILL.md +++ b/.github/skills/vstest-build-test/SKILL.md @@ -101,9 +101,9 @@ Use `-p` to filter by assembly name pattern: ./test.sh -p datacollector # Data collector tests ./test.sh -p smoke # Smoke tests -# Windows -./test.cmd -p htmllogger -./test.cmd -p smoke +# Windows (-p is ambiguous in PowerShell; use -projects) +./test.cmd -projects htmllogger +./test.cmd -projects smoke ``` ### Specific Test by Name diff --git a/eng/build.ps1 b/eng/build.ps1 index 3f17a9cf7d..0d4d2b915e 100644 --- a/eng/build.ps1 +++ b/eng/build.ps1 @@ -38,4 +38,6 @@ Param( # Call the build script provided by Arcade -& $PSScriptRoot/common/build.ps1 @PSBoundParameters \ No newline at end of file +& $PSScriptRoot/common/build.ps1 @PSBoundParameters +# Forward exit code of the parent script +exit $LastExitCode diff --git a/eng/common/tools.ps1 b/eng/common/tools.ps1 index 01296ee601..8b3ae7cd43 100644 --- a/eng/common/tools.ps1 +++ b/eng/common/tools.ps1 @@ -747,6 +747,7 @@ function ExitWithExitCode([int] $exitCode) { if ($ci -and $prepareMachine) { Stop-Processes } + Write-Host "exiting with exit code: $exitCode" exit $exitCode } From a64c2580717e33acaa8df35556553186d1e9340f Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 4 Mar 2026 15:22:12 +0100 Subject: [PATCH 06/18] Fix HTML logger parallel file collision (#15404) 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: https://github.com/microsoft/vstest/issues/15404 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../HtmlLogger.cs | 26 +++++++++++-------- .../HtmlLoggerTests.cs | 21 +++++++++++++++ 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs b/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs index bf994ee5e2..e965b8d94f 100644 --- a/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs +++ b/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.IO; @@ -33,7 +34,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? _parametersDictionary; public HtmlLogger() @@ -305,9 +305,10 @@ private void PopulateHtmlFile() { try { - var fileName = string.Format(CultureInfo.CurrentCulture, "{0}_{1}_{2}", + var fileName = string.Format(CultureInfo.CurrentCulture, "{0}_{1}_{2}_{3}", Environment.GetEnvironmentVariable("UserName"), Environment.MachineName, - FormatDateTimeForRunName(DateTime.Now)); + FormatDateTimeForRunName(DateTime.Now), + Process.GetCurrentProcess().Id); XmlFilePath = GenerateUniqueFilePath(fileName, HtmlLoggerConstants.XmlFileExtension); @@ -345,18 +346,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) + { + // File already exists (another process created it), try next iteration. } } diff --git a/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs b/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs index 821093aa91..a2010d6726 100644 --- a/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs +++ b/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.IO; using System.Linq; using System.Runtime.Serialization; @@ -591,6 +592,26 @@ public void TestCompleteHandlerShouldNotDivideByZeroWhenThereAre0TestResults() Assert.AreEqual(0, _htmlLogger.TestRunDetails.Summary.PassPercentage); } + [TestMethod] + public void XmlFilePathShouldContainProcessIdForCrossProcessUniqueness() + { + // Verifies fix for https://github.com/microsoft/vstest/issues/15404 + // The temp XML file name should include the process ID to avoid collisions + // when multiple vstest processes run in parallel. + _mockFileHelper.Setup(x => x.GetStream(It.IsAny(), FileMode.OpenOrCreate, FileAccess.ReadWrite)) + .Returns(new Mock().Object); + + _htmlLogger.TestRunCompleteHandler(new object(), new TestRunCompleteEventArgs(null, false, true, null, null, null, TimeSpan.Zero)); + +#pragma warning disable CA1837 // Use Environment.ProcessId - not available on net48 + var pid = System.Diagnostics.Process.GetCurrentProcess().Id.ToString(CultureInfo.InvariantCulture); +#pragma warning restore CA1837 + Assert.IsNotNull(_htmlLogger.XmlFilePath); + Assert.IsTrue( + _htmlLogger.XmlFilePath.Contains($"_{pid}"), + $"XmlFilePath should contain process ID '{pid}' for cross-process uniqueness, but was: '{_htmlLogger.XmlFilePath}'"); + } + private static TestCase CreateTestCase(string testCaseName) { return new TestCase(testCaseName, new Uri("some://uri"), "DummySourceFileName"); From ecf5ddbfff776493c85e180d1a09baf8b43c3bae Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 4 Mar 2026 15:23:43 +0100 Subject: [PATCH 07/18] Update copilot-instructions.md and skill docs with build/test guides 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> --- .github/copilot-instructions.md | 76 +++++++++++++++++++++++ .github/skills/vstest-build-test/SKILL.md | 23 +++++++ 2 files changed, 99 insertions(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 19be10e2fa..4989d8a45a 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -28,3 +28,79 @@ 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//.csproj --no-restore +``` + +### Running Unit Tests for a Specific Project + +```bash +# Run all TFMs +dotnet test test//.csproj --no-build + +# Run a specific TFM +dotnet test test//.csproj --no-build -f net9.0 + +# Run a specific test +dotnet test test//.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 include PID 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. diff --git a/.github/skills/vstest-build-test/SKILL.md b/.github/skills/vstest-build-test/SKILL.md index 3c061b2c28..fa96929953 100644 --- a/.github/skills/vstest-build-test/SKILL.md +++ b/.github/skills/vstest-build-test/SKILL.md @@ -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 +- **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:** Use `Environment.ProcessId` on net5.0+; suppress with `#pragma warning disable CA1837` for net48 targets +- Source targets `netstandard2.0` — many modern APIs (`Path.GetRelativePath`, `Environment.ProcessId`) are NOT available From 3cb268db58af2c9d3adb8155d8a091308f43d7d8 Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 4 Mar 2026 15:28:52 +0100 Subject: [PATCH 08/18] Fix HTML logger parallel file collision (#15404) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/microsoft/vstest/issues/15404 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 8 ++++++ .../HtmlLogger.cs | 8 ++---- .../HtmlLoggerTests.cs | 28 ++++++++++++------- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 4989d8a45a..677a6f04d5 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -104,3 +104,11 @@ 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) +``` diff --git a/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs b/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs index e965b8d94f..6c6ce7e199 100644 --- a/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs +++ b/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; -using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.IO; @@ -305,10 +304,9 @@ private void PopulateHtmlFile() { try { - var fileName = string.Format(CultureInfo.CurrentCulture, "{0}_{1}_{2}_{3}", + var fileName = string.Format(CultureInfo.CurrentCulture, "{0}_{1}_{2}", Environment.GetEnvironmentVariable("UserName"), Environment.MachineName, - FormatDateTimeForRunName(DateTime.Now), - Process.GetCurrentProcess().Id); + FormatDateTimeForRunName(DateTime.Now)); XmlFilePath = GenerateUniqueFilePath(fileName, HtmlLoggerConstants.XmlFileExtension); @@ -369,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_HHmmssfff", DateTimeFormatInfo.InvariantInfo); } /// diff --git a/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs b/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs index a2010d6726..0571b06763 100644 --- a/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs +++ b/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Globalization; using System.IO; using System.Linq; using System.Runtime.Serialization; @@ -593,23 +592,32 @@ public void TestCompleteHandlerShouldNotDivideByZeroWhenThereAre0TestResults() } [TestMethod] - public void XmlFilePathShouldContainProcessIdForCrossProcessUniqueness() + public void XmlFilePathShouldContainMillisecondTimestampForCrossProcessUniqueness() { // Verifies fix for https://github.com/microsoft/vstest/issues/15404 - // The temp XML file name should include the process ID to avoid collisions - // when multiple vstest processes run in parallel. + // The temp XML file name should use millisecond-precision timestamps to avoid + // collisions when multiple vstest processes run in parallel. _mockFileHelper.Setup(x => x.GetStream(It.IsAny(), FileMode.OpenOrCreate, FileAccess.ReadWrite)) .Returns(new Mock().Object); _htmlLogger.TestRunCompleteHandler(new object(), new TestRunCompleteEventArgs(null, false, true, null, null, null, TimeSpan.Zero)); -#pragma warning disable CA1837 // Use Environment.ProcessId - not available on net48 - var pid = System.Diagnostics.Process.GetCurrentProcess().Id.ToString(CultureInfo.InvariantCulture); -#pragma warning restore CA1837 Assert.IsNotNull(_htmlLogger.XmlFilePath); - Assert.IsTrue( - _htmlLogger.XmlFilePath.Contains($"_{pid}"), - $"XmlFilePath should contain process ID '{pid}' for cross-process uniqueness, but was: '{_htmlLogger.XmlFilePath}'"); + // 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}'"); } private static TestCase CreateTestCase(string testCaseName) From 6e26e6381e470c2b646df7d1a1a51a81676a3fa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Thu, 5 Mar 2026 16:22:09 +0100 Subject: [PATCH 09/18] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs b/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs index 6c6ce7e199..466a5a4af6 100644 --- a/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs +++ b/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs @@ -356,7 +356,7 @@ private string GenerateUniqueFilePath(string fileName, string fileExtension) using var _ = new FileStream(fullFilePath, FileMode.CreateNew, FileAccess.Write, FileShare.None); return fullFilePath; } - catch (IOException) + catch (IOException) when (File.Exists(fullFilePath)) { // File already exists (another process created it), try next iteration. } From ce0bf3e6919305348089d02696283e2e0c09728e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Thu, 5 Mar 2026 16:42:52 +0100 Subject: [PATCH 10/18] Update .github/copilot-instructions.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 677a6f04d5..1fec2dc960 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -95,7 +95,7 @@ dotnet test test//.csproj --no-build -f net9.0 - **`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 include PID for cross-process uniqueness. File creation uses `FileMode.CreateNew` for atomicity. +- 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 From 4780e0e40d546da912d1698aa49bb8852bf8f973 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Thu, 5 Mar 2026 16:43:10 +0100 Subject: [PATCH 11/18] Update .github/skills/vstest-build-test/SKILL.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .github/skills/vstest-build-test/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/skills/vstest-build-test/SKILL.md b/.github/skills/vstest-build-test/SKILL.md index fa96929953..565b19e0d1 100644 --- a/.github/skills/vstest-build-test/SKILL.md +++ b/.github/skills/vstest-build-test/SKILL.md @@ -157,7 +157,7 @@ After building with `--pack` / `-pack`, validate vstest.console changes by unzip - **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 +- **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` From 87a4ba3dbab58324e86b7a491a767ab79fbd1d14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Thu, 5 Mar 2026 16:43:30 +0100 Subject: [PATCH 12/18] Update .github/skills/vstest-build-test/SKILL.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .github/skills/vstest-build-test/SKILL.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/skills/vstest-build-test/SKILL.md b/.github/skills/vstest-build-test/SKILL.md index 565b19e0d1..9a03e8f3bb 100644 --- a/.github/skills/vstest-build-test/SKILL.md +++ b/.github/skills/vstest-build-test/SKILL.md @@ -164,5 +164,5 @@ After building with `--pack` / `-pack`, validate vstest.console changes by unzip ### Analyzer Pitfalls - **CA1305:** Always provide `IFormatProvider` (e.g., `CultureInfo.InvariantCulture`) for `ToString()` calls -- **CA1837:** Use `Environment.ProcessId` on net5.0+; suppress with `#pragma warning disable CA1837` for net48 targets -- Source targets `netstandard2.0` — many modern APIs (`Path.GetRelativePath`, `Environment.ProcessId`) are NOT available +- **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. From 81df804bf848942b2996433b0446a567dd1f79b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Thu, 5 Mar 2026 16:44:12 +0100 Subject: [PATCH 13/18] Update eng/common/tools.ps1 --- eng/common/tools.ps1 | 1 - 1 file changed, 1 deletion(-) diff --git a/eng/common/tools.ps1 b/eng/common/tools.ps1 index 8b3ae7cd43..01296ee601 100644 --- a/eng/common/tools.ps1 +++ b/eng/common/tools.ps1 @@ -747,7 +747,6 @@ function ExitWithExitCode([int] $exitCode) { if ($ci -and $prepareMachine) { Stop-Processes } - Write-Host "exiting with exit code: $exitCode" exit $exitCode } From 10bb75040658cb6b3e7fb412496c62c4c9edd2ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Tue, 24 Mar 2026 07:54:22 +0100 Subject: [PATCH 14/18] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Youssef Victor --- .../HtmlLogger.cs | 2 +- .../HtmlLoggerTests.cs | 46 +++++++++++-------- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs b/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs index 466a5a4af6..aa947242af 100644 --- a/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs +++ b/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs @@ -367,7 +367,7 @@ private string GenerateUniqueFilePath(string fileName, string fileExtension) private static string FormatDateTimeForRunName(DateTime timeStamp) { - return timeStamp.ToString("yyyyMMdd_HHmmssfff", DateTimeFormatInfo.InvariantInfo); + return timeStamp.ToString("yyyyMMdd_HHmmss.fffffff", DateTimeFormatInfo.InvariantInfo); } /// diff --git a/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs b/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs index 0571b06763..544285cc44 100644 --- a/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs +++ b/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs @@ -597,27 +597,37 @@ public void XmlFilePathShouldContainMillisecondTimestampForCrossProcessUniquenes // 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. - _mockFileHelper.Setup(x => x.GetStream(It.IsAny(), FileMode.OpenOrCreate, FileAccess.ReadWrite)) - .Returns(new Mock().Object); - - _htmlLogger.TestRunCompleteHandler(new object(), new TestRunCompleteEventArgs(null, false, true, null, null, null, TimeSpan.Zero)); + 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; + + // 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.IsNotNull(_htmlLogger.XmlFilePath); - // 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) + 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 { - timePart = timePart.Substring(0, bracketIdx); + if (!string.IsNullOrEmpty(createdFilePath) && File.Exists(createdFilePath)) + { + File.Delete(createdFilePath); + } } - - Assert.AreEqual(9, timePart.Length, - $"Time portion of timestamp should be 9 chars (HHmmssfff) but was '{timePart}' (length {timePart.Length}) in path: '{_htmlLogger.XmlFilePath}'"); } private static TestCase CreateTestCase(string testCaseName) From 4e7ba20052ff46e1d766ac2d0582af9f0a600583 Mon Sep 17 00:00:00 2001 From: Jakub Jares Date: Fri, 10 Apr 2026 14:55:53 +0200 Subject: [PATCH 15/18] Fix HtmlLogger collision handling Tighten the HTML logger collision fix by restoring the intended millisecond timestamp format, keeping atomic create-new file allocation testable through IFileHelper, dropping unrelated docs churn from the PR, and adding coverage for retry-on-collision behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 84 ------------------- .github/skills/vstest-build-test/SKILL.md | 23 ----- .../HtmlLogger.cs | 6 +- .../HtmlLoggerTests.cs | 40 +++++++++ 4 files changed, 43 insertions(+), 110 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index ff1a199db4..5da0bc8573 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -57,90 +57,6 @@ Anytime you add a new localization resource, you MUST: - 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//.csproj --no-restore -``` - -### Running Unit Tests for a Specific Project - -```bash -# Run all TFMs -dotnet test test//.csproj --no-build - -# Run a specific TFM -dotnet test test//.csproj --no-build -f net9.0 - -# Run a specific test -dotnet test test//.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) -``` - ## Unattended Work Instructions When working autonomously on issues (e.g. from a milestone), follow this workflow: diff --git a/.github/skills/vstest-build-test/SKILL.md b/.github/skills/vstest-build-test/SKILL.md index 9a03e8f3bb..3c061b2c28 100644 --- a/.github/skills/vstest-build-test/SKILL.md +++ b/.github/skills/vstest-build-test/SKILL.md @@ -143,26 +143,3 @@ 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. diff --git a/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs b/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs index 8e1fe4c168..527d0f155d 100644 --- a/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs +++ b/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs @@ -310,7 +310,7 @@ private void PopulateHtmlFile() { try { - var fileName = string.Format(CultureInfo.CurrentCulture, "{0}_{1}_{2}", + var fileName = string.Format(CultureInfo.InvariantCulture, "{0}_{1}_{2}", Environment.GetEnvironmentVariable("UserName"), Environment.MachineName, FormatDateTimeForRunName(DateTime.Now)); @@ -359,7 +359,7 @@ private string GenerateUniqueFilePath(string fileName, string fileExtension) { // 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); + using var _ = _fileHelper.GetStream(fullFilePath, FileMode.CreateNew, FileAccess.Write, FileShare.None); return fullFilePath; } catch (IOException) when (File.Exists(fullFilePath)) @@ -373,7 +373,7 @@ private string GenerateUniqueFilePath(string fileName, string fileExtension) private static string FormatDateTimeForRunName(DateTime timeStamp) { - return timeStamp.ToString("yyyyMMdd_HHmmss.fffffff", DateTimeFormatInfo.InvariantInfo); + return timeStamp.ToString("yyyyMMdd_HHmmssfff", DateTimeFormatInfo.InvariantInfo); } /// diff --git a/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs b/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs index 7bb5c55009..dec3b29781 100644 --- a/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs +++ b/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs @@ -630,6 +630,46 @@ public void XmlFilePathShouldContainMillisecondTimestampForCrossProcessUniquenes } } + [TestMethod] + public void TestCompleteHandlerShouldRetryUniqueFileNameWhenCreateNewCollides() + { + string? collidedPath = null; + var collisionInjected = false; + + _mockFileHelper.Setup(x => x.GetStream(It.IsAny(), FileMode.CreateNew, FileAccess.Write, FileShare.None)) + .Returns((path, _, _, _) => + { + if (!collisionInjected) + { + collisionInjected = true; + collidedPath = path; + File.WriteAllText(path, string.Empty); + throw new IOException("collision"); + } + + return new Mock().Object; + }); + + _mockFileHelper.Setup(x => x.GetStream(It.IsAny(), FileMode.OpenOrCreate, FileAccess.ReadWrite)) + .Returns(new Mock().Object); + + try + { + _htmlLogger.TestRunCompleteHandler(new object(), new TestRunCompleteEventArgs(null, false, true, null, null, null, TimeSpan.Zero)); + + Assert.IsNotNull(_htmlLogger.XmlFilePath); + StringAssert.Contains(_htmlLogger.XmlFilePath, "[1].xml"); + _mockFileHelper.Verify(x => x.GetStream(It.IsAny(), FileMode.CreateNew, FileAccess.Write, FileShare.None), Times.AtLeast(2)); + } + finally + { + if (!string.IsNullOrEmpty(collidedPath) && File.Exists(collidedPath)) + { + File.Delete(collidedPath); + } + } + } + private static TestCase CreateTestCase(string testCaseName) { return new TestCase(testCaseName, new Uri("some://uri"), "DummySourceFileName"); From 4ab2a31372750c63d3f5c50a3af6fa604dcfa8f6 Mon Sep 17 00:00:00 2001 From: Jakub Jares Date: Fri, 10 Apr 2026 15:21:42 +0200 Subject: [PATCH 16/18] Fix HtmlLogger test assertion Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../HtmlLoggerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs b/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs index c9c6979f56..1ab0bd797a 100644 --- a/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs +++ b/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs @@ -632,7 +632,7 @@ public void TestCompleteHandlerShouldRetryUniqueFileNameWhenCreateNewCollides() _htmlLogger.TestRunCompleteHandler(new object(), new TestRunCompleteEventArgs(null, false, true, null, null, null, TimeSpan.Zero)); Assert.IsNotNull(_htmlLogger.XmlFilePath); - StringAssert.Contains(_htmlLogger.XmlFilePath, "[1].xml"); + Assert.Contains("[1].xml", _htmlLogger.XmlFilePath); _mockFileHelper.Verify(x => x.GetStream(It.IsAny(), FileMode.CreateNew, FileAccess.Write, FileShare.None), Times.AtLeast(2)); } finally From 0b55ee497529dcbcec42e5e254d1872c66e7ef91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Wed, 13 May 2026 17:12:30 +0200 Subject: [PATCH 17/18] fix: use _fileHelper.Exists instead of File.Exists in GenerateUniqueFilePath Address review feedback: the exception filter was calling File.Exists() directly, bypassing the injected IFileHelper abstraction. Replace with _fileHelper.Exists() for consistency and testability. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs b/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs index 99465cdffc..95081e1eb2 100644 --- a/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs +++ b/src/Microsoft.TestPlatform.Extensions.HtmlLogger/HtmlLogger.cs @@ -362,7 +362,7 @@ private string GenerateUniqueFilePath(string fileName, string fileExtension) using var _ = _fileHelper.GetStream(fullFilePath, FileMode.CreateNew, FileAccess.Write, FileShare.None); return fullFilePath; } - catch (IOException) when (File.Exists(fullFilePath)) + catch (IOException) when (_fileHelper.Exists(fullFilePath)) { // File already exists (another process created it), try next iteration. } From 865964012c7eb4167ca686d5cdfb041d0ca13d5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Wed, 13 May 2026 17:27:34 +0200 Subject: [PATCH 18/18] fix: correct mock setup in retry collision test Add _mockFileHelper.Exists setup returning true so the IOException catch filter when (_fileHelper.Exists(fullFilePath)) evaluates to true and the exception is swallowed, allowing the retry loop to proceed. Remove real-filesystem File.WriteAllText side effect and simplify cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../HtmlLoggerTests.cs | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs b/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs index 1ab0bd797a..0f7d6dd2b3 100644 --- a/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs +++ b/test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/HtmlLoggerTests.cs @@ -607,17 +607,16 @@ public void XmlFilePathShouldContainFractionalSecondsForCrossProcessUniqueness() [TestMethod] public void TestCompleteHandlerShouldRetryUniqueFileNameWhenCreateNewCollides() { - string? collidedPath = null; var collisionInjected = false; + _mockFileHelper.Setup(x => x.Exists(It.IsAny())).Returns(true); + _mockFileHelper.Setup(x => x.GetStream(It.IsAny(), FileMode.CreateNew, FileAccess.Write, FileShare.None)) .Returns((path, _, _, _) => { if (!collisionInjected) { collisionInjected = true; - collidedPath = path; - File.WriteAllText(path, string.Empty); throw new IOException("collision"); } @@ -627,21 +626,11 @@ public void TestCompleteHandlerShouldRetryUniqueFileNameWhenCreateNewCollides() _mockFileHelper.Setup(x => x.GetStream(It.IsAny(), FileMode.OpenOrCreate, FileAccess.ReadWrite)) .Returns(new Mock().Object); - try - { - _htmlLogger.TestRunCompleteHandler(new object(), new TestRunCompleteEventArgs(null, false, true, null, null, null, TimeSpan.Zero)); + _htmlLogger.TestRunCompleteHandler(new object(), new TestRunCompleteEventArgs(null, false, true, null, null, null, TimeSpan.Zero)); - Assert.IsNotNull(_htmlLogger.XmlFilePath); - Assert.Contains("[1].xml", _htmlLogger.XmlFilePath); - _mockFileHelper.Verify(x => x.GetStream(It.IsAny(), FileMode.CreateNew, FileAccess.Write, FileShare.None), Times.AtLeast(2)); - } - finally - { - if (!string.IsNullOrEmpty(collidedPath) && File.Exists(collidedPath)) - { - File.Delete(collidedPath); - } - } + Assert.IsNotNull(_htmlLogger.XmlFilePath); + Assert.Contains("[1].xml", _htmlLogger.XmlFilePath); + _mockFileHelper.Verify(x => x.GetStream(It.IsAny(), FileMode.CreateNew, FileAccess.Write, FileShare.None), Times.AtLeast(2)); } private static TestCase CreateTestCase(string testCaseName)