[fix] Fix TRX attachment paths when LogFileName contains a subdirectory by nohwnd · Pull Request #15791 · microsoft/vstest

nohwnd · 2026-05-15T13:21:12Z

🤖 This is an automated fix generated by the Issue Triage agent.

Root Cause

When --logger trx;LogFileName=subdir/results.trx is used, the TRX file is placed at <TestRunDirectory>/subdir/results.trx. However, the internal _testResultsDirPath (used to organize test attachments) remained set to the original TestRunDirectory.

TRX viewers resolve the runDeploymentRoot attribute relative to the directory containing the TRX file. So a viewer opening subdir/results.trx expects attachments at subdir/<runDeploymentRoot>/In/..., but they were actually placed at <runDeploymentRoot>/In/... (one level up) — making them unreachable.

Fix

In Initialize(events, parameters), detect when LogFileName has a directory component and pre-adjust _testResultsDirPath to point to the directory that will contain the TRX file. This ensures both the TRX and its attachment folder are co-located.

In GetTrxFilePath, use Path.GetFileName(logFileNameValue) instead of the full logFileNameValue so the directory portion isn't appended twice (it's already in _testResultsDirPath).

The fix is backward-compatible: when LogFileName is a plain filename with no directory component, Path.GetDirectoryName returns an empty string and no adjustment is made.

Test

Added CustomTrxFileNameWithSubdirectoryShouldPlaceTrxAndAttachmentsUnderSameDirectory which verifies that when LogFileName=subdir/results.trx, the TRX file lands at <TestRunDirectory>/subdir/results.trx.

🔍 Triaged by Issue Repro Triage & Auto-Fix 🔍

When LogFileName is set to a path like 'subdir/results.trx', the TRX file is placed at '<TestRunDirectory>/subdir/results.trx'. However, attachments were still organized relative to the original TestRunDirectory. Since TRX viewers resolve RunDeploymentRootDirectory relative to the TRX file's parent folder, attachments became unreachable. Fix: adjust _testResultsDirPath in Initialize to include the directory component of LogFileName so that both the TRX file and its attachments land under the same parent directory. Also update GetTrxFilePath to use only the filename portion of LogFileName (since the directory is now already baked into _testResultsDirPath). Fixes #15271 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR updates the TRX logger so LogFileName values containing a subdirectory place the TRX file and attachment deployment directory under the same relative folder, addressing attachment resolution for TRX viewers.

Changes:

Adjusts the logger’s test results directory when LogFileName includes a directory component.
Prevents duplicating the directory portion when constructing the final TRX file path.
Adds a unit test for TRX path placement with a subdirectory in LogFileName.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
`src/Microsoft.TestPlatform.Extensions.TrxLogger/TrxLogger.cs`	Updates initialization and TRX path construction to account for subdirectories in `LogFileName`.
`test/Microsoft.TestPlatform.Extensions.TrxLogger.UnitTests/TrxLoggerTests.cs`	Adds coverage for TRX file path placement when `LogFileName` includes a subdirectory.

+        var parameters = new Dictionary<string, string?>
+        {
+            [DefaultLoggerParameterNames.TestRunDirectory] = DefaultTestRunDirectory,
+            [TrxLoggerConstants.LogFileNameKey] = Path.Combine(subDir, fileName),
+        };
+        logger.Initialize(_events.Object, parameters);
+
+        MakeTestRunComplete(logger);
+
+        try
+        {
+            // The TRX file must be at <TestRunDirectory>/<subDir>/<fileName>
+            var expectedTrxPath = Path.Combine(DefaultTestRunDirectory, subDir, fileName);
+            Assert.AreEqual(expectedTrxPath, logger.TrxFile, "TRX file should be in the specified subdirectory.");
+        }
+        finally
+        {
+            if (!string.IsNullOrEmpty(logger.TrxFile) && File.Exists(logger.TrxFile))
+            {
+                File.Delete(logger.TrxFile);
+            }


nohwnd

Code Review

Dimensions activated: Null Safety & Boundary Validation, Backward Compatibility & Rollback Safety, Error Reporting & Diagnostic Clarity

Summary

The fix correctly addresses the root cause. When LogFileName contains a directory component, _testResultsDirPath is now pre-adjusted to include that subdirectory, ensuring both the TRX file and its attachment folder land in the same directory. The AcquireTrxFileNamePath change correctly uses Path.GetFileName to avoid double-inclusion of the subdirectory.

Correctness Analysis

Path edge cases — all handled correctly:

Plain filename (results.trx): Path.GetDirectoryName returns "", which is null-or-whitespace → no adjustment. ✓
Absolute path (/tmp/results.trx): Path.Combine(testRunDirectory, "/tmp") → absolute path wins, same behavior as before. ✓
Nested subdirectories (dir1/dir2/results.trx): Path.GetDirectoryName returns dir1/dir2 → correctly combined. ✓
LogFilePrefix guard: the adjustment is skipped when isLogFilePrefixParameterExists is true, matching the AcquireTrxFileNamePath branch logic. ✓

runDeploymentRoot in TRX: CreateTestRun derives runDeploymentRoot from the run name (not _testResultsDirPath), so it's always a relative folder name. The fix ensures this relative folder resolves correctly from the TRX file's directory because _testResultsDirPath now points to the same directory as the TRX file. ✓

One Minor Note

The new test doesn't clean up the subdir directory it creates under Path.GetTempPath() — only the TRX file itself is deleted. However, this is consistent with the existing test style in this file (e.g., CustomTrxFileNameShouldConstructFromLogFileParameter performs no cleanup at all), and using the system temp path means it's not harmful. The new test is actually more careful than its peers by having a try/finally block at all.

No blocking issues. The fix is correct and backward-compatible.

🧠 Reviewed by Expert Code Reviewer

🧠 Reviewed by Expert Code Reviewer 🧠

Verifies that when LogFileName contains a path component (e.g. subdir/results.trx), the TRX file is placed in the expected subdirectory under the results directory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

nohwnd · 2026-05-19T05:59:38Z

Commit pushed: 69eced6

🔧 Iterated by PR Iteration Agent 🔧

nohwnd

Code Review

Dimensions activated: Null Safety & Boundary Validation, Backward Compatibility & Rollback Safety, Acceptance Test Coverage Design, Cross-TFM & Framework Resolution

Summary

No blocking issues. The fix is correct and backward-compatible. This is a re-review pass — the head commit (69eced6) is only a CI retrigger with no code changes since the previous review.

Dimension Findings

Null Safety & Boundary Validation ✓

logFileNameInit.IsNullOrWhiteSpace() correctly guards the TryGetValue result before Path.GetDirectoryName.
logFileDir.IsNullOrWhiteSpace() correctly skips the adjustment for plain filenames (where Path.GetDirectoryName returns "").
Path.GetFileName(logFileNameValue!) is sound — it's called inside the isLogFileNameParameterExists branch, which guarantees the value was set.

Backward Compatibility & Rollback Safety ✓

Plain filename (results.trx): Path.GetDirectoryName returns "" → IsNullOrWhiteSpace is true → no adjustment → identical behavior to before. ✓
Subdirectory (subdir/results.trx): adjustment applied, attachments land co-located with the TRX. ✓
Absolute path (/tmp/results.trx): Path.Combine(testRunDirectory, "/tmp") → absolute path wins on both platforms → _testResultsDirPath = /tmp; TRX file = /tmp/results.trx — same final location as before, and attachments are now also correctly co-located. ✓
The !isLogFilePrefixParameterExists guard correctly mirrors the LogFilePrefix vs LogFileName branch logic in AcquireTrxFileNamePath. ✓

Acceptance Test Coverage Design ✓

Test covers both NetFullTargetFrameworkDataSource and NetCoreTargetFrameworkDataSource — good TFM breadth.
Uses TempDirectory.Path for the results directory, so cleanup is automatic.
Asserts both file existence and XML validity, providing meaningful failure messages including a glob of all TRX files found.

Cross-TFM & Framework Resolution ✓

All APIs used (Path.GetDirectoryName, Path.GetFileName, Path.Combine, string.IsNullOrWhiteSpace) are available on all vstest target TFMs (net462 / netstandard2.0 / net8.0+).

🧠 Reviewed by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

+    [TestMethod]
+    public void CustomTrxFileNameWithSubdirectoryShouldPlaceTrxAndAttachmentsUnderSameDirectory()
+    {
+        // Arrange: LogFileName contains a subdirectory component
+        var logger = new TestableTrxLogger();


+    [TestMethod]
+    [NetFullTargetFrameworkDataSource]
+    [NetCoreTargetFrameworkDataSource]
+    public void TrxLoggerShouldPlaceTrxFileInSubdirectoryWhenLogFileNameContainsPath(RunnerInfo runnerInfo)
+    {
+        // Regression test for https://github.com/microsoft/vstest/issues/15271
+        // When LogFileName contains a subdirectory (e.g. "subdir/results.trx"),
+        // the TRX file and its attachments should be placed under that subdirectory.
+        SetTestEnvironment(_testEnvironment, runnerInfo);


Copilot AI review requested due to automatic review settings May 15, 2026 13:21

nohwnd added the Area: Logger label May 15, 2026

Copilot started reviewing on behalf of nohwnd May 15, 2026 13:24 View session

Copilot AI reviewed May 15, 2026

View reviewed changes

nohwnd commented May 15, 2026

View reviewed changes

This was referenced May 16, 2026

Daily Maintenance Digest — 2026-05-17 #15768

Closed

Daily Maintenance Digest — 2026-05-20 #16036

Closed

nohwnd and others added 2 commits May 18, 2026 12:17

ci: retrigger macOS build

69eced6

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

nohwnd commented May 19, 2026

View reviewed changes

This was referenced May 21, 2026

Daily Maintenance Digest — 2026-05-25 #16049

Closed

Daily Maintenance Digest — 2026-06-01 #16066

Open

nohwnd added the 🚢 Ship it! Add to PRs where owner approves automated PR, but cannot approve because they "wrote it". label May 29, 2026

nohwnd marked this pull request as ready for review May 29, 2026 15:24

Copilot AI review requested due to automatic review settings May 29, 2026 15:24

Copilot started reviewing on behalf of nohwnd May 29, 2026 15:24 View session

Copilot AI reviewed May 29, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fix] Fix TRX attachment paths when LogFileName contains a subdirectory#15791

[fix] Fix TRX attachment paths when LogFileName contains a subdirectory#15791
nohwnd wants to merge 3 commits into
mainfrom
fix/issue-15271-f17c7eff60d1c399

nohwnd commented May 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

nohwnd left a comment

Uh oh!

nohwnd commented May 19, 2026

Uh oh!

nohwnd left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

nohwnd commented May 15, 2026

Root Cause

Fix

Test

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

nohwnd left a comment

Choose a reason for hiding this comment

Code Review

Summary

Correctness Analysis

One Minor Note

Uh oh!

nohwnd commented May 19, 2026

Uh oh!

nohwnd left a comment

Choose a reason for hiding this comment

Code Review

Summary

Dimension Findings

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants