Copy per-test result files to results directory #7452
+379
−13
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,80 @@ | ||||||||||||||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||||||||||||||
| // Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||||||||||||||
|
|
||||||||||||||
| using Microsoft.Testing.Platform.Configurations; | ||||||||||||||
| using Microsoft.Testing.Platform.Extensions; | ||||||||||||||
| using Microsoft.Testing.Platform.Extensions.Messages; | ||||||||||||||
| using Microsoft.Testing.Platform.Helpers; | ||||||||||||||
| using Microsoft.Testing.Platform.Resources; | ||||||||||||||
|
|
||||||||||||||
| namespace Microsoft.Testing.Platform.TestHost; | ||||||||||||||
|
|
||||||||||||||
| /// <summary> | ||||||||||||||
| /// Copies per-test file artifacts to the test results directory so that the | ||||||||||||||
| /// results directory is self-contained. | ||||||||||||||
| /// </summary> | ||||||||||||||
| internal sealed class FileArtifactCopyDataConsumer : IDataConsumer | ||||||||||||||
|
Member
Author
There was a problem hiding this comment. Maybe this should be enabled/disabled via CLI or option? |
||||||||||||||
| { | ||||||||||||||
| private readonly IConfiguration _configuration; | ||||||||||||||
|
|
||||||||||||||
| public FileArtifactCopyDataConsumer(IConfiguration configuration) | ||||||||||||||
| => _configuration = configuration; | ||||||||||||||
|
|
||||||||||||||
| public Type[] DataTypesConsumed => [typeof(TestNodeUpdateMessage)]; | ||||||||||||||
|
|
||||||||||||||
| public string Uid => nameof(FileArtifactCopyDataConsumer); | ||||||||||||||
|
|
||||||||||||||
| public string Version => AppVersion.DefaultSemVer; | ||||||||||||||
|
|
||||||||||||||
| public string DisplayName => PlatformResources.FileArtifactCopyDataConsumerDisplayName; | ||||||||||||||
|
|
||||||||||||||
| public string Description => PlatformResources.FileArtifactCopyDataConsumerDescription; | ||||||||||||||
|
|
||||||||||||||
| public Task<bool> IsEnabledAsync() => Task.FromResult(true); | ||||||||||||||
|
|
||||||||||||||
| public Task ConsumeAsync(IDataProducer dataProducer, IData value, CancellationToken cancellationToken) | ||||||||||||||
| { | ||||||||||||||
| if (value is not TestNodeUpdateMessage message) | ||||||||||||||
| { | ||||||||||||||
| return Task.CompletedTask; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| FileArtifactProperty[] artifacts = message.TestNode.Properties.OfType<FileArtifactProperty>(); | ||||||||||||||
| if (artifacts.Length == 0) | ||||||||||||||
| { | ||||||||||||||
| return Task.CompletedTask; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| string resultsDirectory = _configuration.GetTestResultDirectory(); | ||||||||||||||
|
|
||||||||||||||
| foreach (FileArtifactProperty artifact in artifacts) | ||||||||||||||
| { | ||||||||||||||
| CopyFileToResultsDirectory(artifact.FileInfo, resultsDirectory); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| return Task.CompletedTask; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| private static void CopyFileToResultsDirectory(FileInfo file, string resultsDirectory) | ||||||||||||||
| { | ||||||||||||||
| // If the file is already under the results directory, skip. | ||||||||||||||
| string fullResultsDirectory = Path.GetFullPath(resultsDirectory); | ||||||||||||||
| if (file.FullName.StartsWith(fullResultsDirectory, StringComparison.OrdinalIgnoreCase)) | ||||||||||||||
|
There was a problem hiding this comment. The path comparison using StartsWith with StringComparison.OrdinalIgnoreCase may not correctly handle directory separators on all platforms. For example, if resultsDirectory is "C:\results" and file.FullName is "C:\results_backup\file.txt", the StartsWith check will incorrectly consider the file to be under the results directory. The codebase has a FileUtilities.PathComparison utility (src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/FileUtilities.cs) that properly handles case-sensitivity based on the file system. Additionally, similar code in AnsiTerminal.cs checks for directory separators after the path prefix to ensure it's a true subdirectory. Consider using FileUtilities.PathComparison for the string comparison and adding a check for directory separators after the prefix, similar to the pattern in AnsiTerminal.cs lines 209-213. |
||||||||||||||
| { | ||||||||||||||
| return; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| if (!Directory.Exists(resultsDirectory)) | ||||||||||||||
|
Member
Author
There was a problem hiding this comment. If we go with this solution, I'll use file system wrapper instead to ease testing |
||||||||||||||
| { | ||||||||||||||
| Directory.CreateDirectory(resultsDirectory); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| string destination = Path.Combine(resultsDirectory, file.Name); | ||||||||||||||
| for (int nameCounter = 1; File.Exists(destination) && nameCounter <= 10; nameCounter++) | ||||||||||||||
| { | ||||||||||||||
| destination = Path.Combine(resultsDirectory, $"{Path.GetFileNameWithoutExtension(file.Name)}_{nameCounter}{Path.GetExtension(file.Name)}"); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
|
There was a problem hiding this comment. After the bounded loop completes (when nameCounter exceeds 10), the code calls File.Copy even if a file still exists at the destination path. This will throw an IOException when overwrite is false and the destination file exists. The code should check if a unique filename was found before attempting the copy, or handle the potential exception. Consider either throwing a more informative exception when all 10 attempts are exhausted, or checking File.Exists(destination) after the loop and handling that case explicitly.
Suggested change
|
||||||||||||||
| File.Copy(file.FullName, destination, overwrite: false); | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After the bounded loop completes (when nameCounter exceeds 10), the code calls CopyFileAsync even if a file still exists at the destination path. While CopyFileAsync creates a new file with FileMode.Create (which will overwrite), it would be better to verify that a unique filename was found or handle the case explicitly for clarity and consistency with error handling.