Skip to content

Copy per-test result files to results directory in TRX reports #7449

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.

Sign up for GitHub

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

Draft
Evangelink wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Copy per-test result files to results directory in TRX reports #7449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
53 changes: 36 additions & 17 deletions src/Platform/Microsoft.Testing.Extensions.TrxReport/TrxReportEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ public TrxReportEngine(IFileSystem fileSystem, ITestApplicationModuleInfo testAp
isFileNameExplicitlyProvided = false;
}

AddResults(testAppModule, testRun, out XElement testDefinitions, out XElement testEntries, out string uncategorizedTestId, out bool hasFailedTests);
(XElement testDefinitions, XElement testEntries, string uncategorizedTestId, bool hasFailedTests) = await AddResultsAsync(testAppModule, testRun, runDeploymentRoot).ConfigureAwait(false);
testRun.Add(testDefinitions);
testRun.Add(testEntries);
AddTestLists(testRun, uncategorizedTestId);
Expand Down Expand Up @@ -346,25 +346,40 @@ private async Task AddResultSummaryAsync(XElement testRun, string resultSummaryO
await AddArtifactsToCollectionAsync(_artifactsByExtension, collectorDataEntries, runDeploymentRoot).ConfigureAwait(false);
}

private async Task<string> CopyTestResultFileAndReturnRelativePathAsync(FileInfo artifact, string runDeploymentRoot, string executionId)
Copy link
Member Author

@Evangelink Evangelink Feb 22, 2026

Choose a reason for hiding this comment

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

We could also consider the copy function to be part of the core platform (or vstest bridge) so that the feature is also available outside of trx being enabled

{
string testResultDirectory = Path.Combine(_configuration.GetTestResultDirectory(), runDeploymentRoot, "In", executionId, _environment.MachineName);
if (!Directory.Exists(testResultDirectory))
{
Directory.CreateDirectory(testResultDirectory);
}
Comment on lines +351 to +355
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

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

CopyTestResultFileAndReturnRelativePathAsync creates directories and probes the real filesystem even when _isCopyingFileAllowed is false. This makes “copy disabled” mode still have side effects (and can break unit tests / restricted environments). Consider short-circuiting early when copying is disabled (return the relative path without touching disk).

Copilot uses AI. Check for mistakes.

string fileName = artifact.Name;

string destination = Path.Combine(testResultDirectory, fileName);

// If the file already exists, append a number to the end of the file name
for (int nameCounter = 1; File.Exists(destination) && nameCounter <= 10; nameCounter++)
{
destination = Path.Combine(testResultDirectory, $"{Path.GetFileNameWithoutExtension(fileName)}_{nameCounter}{Path.GetExtension(fileName)}");
}
Comment on lines +361 to +365
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

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

The bounded name-collision loop can still leave destination pointing to an existing file after 10 attempts. Because CopyFileAsync uses FileMode.Create, this will overwrite an existing artifact and silently lose data. After the loop, handle the “still exists” case explicitly (e.g., generate a GUID-based name or throw) and/or use an atomic CreateNew strategy to avoid overwrites.

This issue also appears on line 379 of the same file.

Copilot uses AI. Check for mistakes.

await CopyFileAsync(artifact, new FileInfo(destination)).ConfigureAwait(false);

return Path.Combine(_environment.MachineName, Path.GetFileName(destination));
}

private async Task<string> CopyArtifactIntoTrxDirectoryAndReturnHrefValueAsync(FileInfo artifact, string runDeploymentRoot)
{
string artifactDirectory = CreateOrGetTrxArtifactDirectory(runDeploymentRoot);
string fileName = artifact.Name;

string destination = Path.Combine(artifactDirectory, fileName);
int nameCounter = 0;

// If the file already exists, append a number to the end of the file name
while (true)
for (int nameCounter = 1; File.Exists(destination) && nameCounter <= 10; nameCounter++)
{
if (File.Exists(destination))
{
nameCounter++;
destination = Path.Combine(artifactDirectory, $"{Path.GetFileNameWithoutExtension(fileName)}_{nameCounter}{Path.GetExtension(fileName)}");
continue;
}

break;
destination = Path.Combine(artifactDirectory, $"{Path.GetFileNameWithoutExtension(fileName)}_{nameCounter}{Path.GetExtension(fileName)}");
}

await CopyFileAsync(artifact, new FileInfo(destination)).ConfigureAwait(false);
Expand Down Expand Up @@ -412,17 +427,17 @@ private static void AddTestLists(XElement testRun, string uncategorizedTestId)
testRun.Add(testLists);
}

private void AddResults(string testAppModule, XElement testRun, out XElement testDefinitions, out XElement testEntries, out string uncategorizedTestId, out bool hasFailedTests)
private async Task<(XElement TestDefinitions, XElement TestEntries, string UncategorizedTestId, bool HasFailedTests)> AddResultsAsync(string testAppModule, XElement testRun, string runDeploymentRoot)
{
var results = new XElement("Results");

// Duplicate test ids are not allowed inside the TestDefinitions element.
testDefinitions = new XElement("TestDefinitions");
var testDefinitions = new XElement("TestDefinitions");
var uniqueTestDefinitionTestIds = new HashSet<string>();

testEntries = new XElement("TestEntries");
uncategorizedTestId = "8C84FA94-04C1-424b-9868-57A2D4851A1D";
hasFailedTests = false;
var testEntries = new XElement("TestEntries");
string uncategorizedTestId = "8C84FA94-04C1-424b-9868-57A2D4851A1D";
bool hasFailedTests = false;
foreach (TestNodeUpdateMessage nodeMessage in _testNodeUpdatedMessages)
{
TestNode testNode = nodeMessage.TestNode;
Expand Down Expand Up @@ -534,9 +549,11 @@ private void AddResults(string testAppModule, XElement testRun, out XElement tes
foreach (FileArtifactProperty testFileArtifact in testNode.Properties.OfType<FileArtifactProperty>())
{
resultFiles ??= new XElement("ResultFiles");
string relativePath = await CopyTestResultFileAndReturnRelativePathAsync(
testFileArtifact.FileInfo, runDeploymentRoot, executionId).ConfigureAwait(false);
resultFiles.Add(new XElement(
"ResultFile",
new XAttribute("path", testFileArtifact.FileInfo.FullName)));
new XAttribute("path", relativePath)));
Comment on lines +552 to +556
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

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

New behavior copies per-test result files into the results directory and changes <ResultFile path=...> to be relative. There’s no unit test that exercises the real copy path (all tests construct TrxReportEngine with isCopyingFileAllowed: false), so regressions in directory layout / naming / copy failures won’t be caught. Add a test that enables copying and asserts the file is created under the expected {runDeploymentRoot}/In/{executionId}/{MachineName}/ layout and that the TRX references it correctly.

Copilot uses AI. Check for mistakes.
}

if (resultFiles is not null)
Expand Down Expand Up @@ -666,6 +683,8 @@ private void AddResults(string testAppModule, XElement testRun, out XElement tes
}

testRun.Add(results);

return (testDefinitions, testEntries, uncategorizedTestId, hasFailedTests);
}

private static string AddTestSettings(XElement testRun, string testRunName)
Expand Down
4 changes: 2 additions & 2 deletions test/UnitTests/Microsoft.Testing.Extensions.UnitTests/TrxTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,11 +473,11 @@ public async Task TrxReportEngine_GenerateReportAsync_WithArtifactsByTestNode_Tr
string trxContentsPattern = @"
<UnitTestResult .* testName=""TestMethod"" .* outcome=""Passed"" .*>
<ResultFiles>
<ResultFile path=.*fileName"" />
<ResultFile path=""MachineName[/\\]fileName"" />
</ResultFiles>
</UnitTestResult>
";
Assert.IsTrue(Regex.IsMatch(trxContent, trxContentsPattern));
Assert.IsTrue(Regex.IsMatch(trxContent, trxContentsPattern), trxContent);
}

[TestMethod]
Expand Down