add unit tests for the artifact post processing

src/Microsoft.TestPlatform.CoreUtilities/Helpers/FileHelper.cs

@@ -24,40 +24,27 @@ public class FileHelper : IFileHelper
 
     /// <inheritdoc/>
     public DirectoryInfo CreateDirectory(string path)
-    {
-        return Directory.CreateDirectory(path);
-    }
+        => Directory.CreateDirectory(path);
 
     /// <inheritdoc/>
     public string GetCurrentDirectory()
-    {
-        return Directory.GetCurrentDirectory();
-    }
+        => Directory.GetCurrentDirectory();
 
     /// <inheritdoc/>
     public bool Exists(string path)
-    {
-        return File.Exists(path);
-    }
+        => File.Exists(path);
 
     /// <inheritdoc/>
     public bool DirectoryExists(string path)
-    {
-        return Directory.Exists(path);
-    }
+        => Directory.Exists(path);
 
     /// <inheritdoc/>
     public Stream GetStream(string filePath, FileMode mode, FileAccess access = FileAccess.ReadWrite)
-    {
-        return new FileStream(filePath, mode, access);
-    }
+        => new FileStream(filePath, mode, access);
 
     /// <inheritdoc/>
     public Stream GetStream(string filePath, FileMode mode, FileAccess access, FileShare share)
-    {
-        return new FileStream(filePath, mode, access, share);
-    }
-
+        => new FileStream(filePath, mode, access, share);
 
     /// <inheritdoc/>
     public IEnumerable<string> EnumerateFiles(
@@ -79,40 +66,29 @@ public IEnumerable<string> EnumerateFiles(
 
     /// <inheritdoc/>
     public FileAttributes GetFileAttributes(string path)
-    {
-        return new FileInfo(path).Attributes;
-    }
+        => new FileInfo(path).Attributes;
 
     /// <inheritdoc/>
     public Version GetFileVersion(string path)
-    {
-        var currentFileVersion = FileVersionInfo.GetVersionInfo(path)?.FileVersion;
-        return Version.TryParse(currentFileVersion, out var currentVersion) ? currentVersion : DefaultFileVersion;
-    }
+        => Version.TryParse(FileVersionInfo.GetVersionInfo(path)?.FileVersion, out var currentVersion) ?
+            currentVersion :
+            DefaultFileVersion;
 
     /// <inheritdoc/>
     public void CopyFile(string sourcePath, string destinationPath)
-    {
-        File.Copy(sourcePath, destinationPath);
-    }
+        => File.Copy(sourcePath, destinationPath);
 
     /// <inheritdoc/>
     public void MoveFile(string sourcePath, string destinationPath)
-    {
-        File.Move(sourcePath, destinationPath);
-    }
+        => File.Move(sourcePath, destinationPath);
 
     /// <inheritdoc/>
     public void WriteAllTextToFile(string filePath, string content)
-    {
-        File.WriteAllText(filePath, content);
-    }
+        => File.WriteAllText(filePath, content);
 
     /// <inheritdoc/>
     public string GetFullPath(string path)
-    {
-        return Path.GetFullPath(path);
-    }
+        => Path.GetFullPath(path);
 
     /// <inheritdoc/>
     public void DeleteEmptyDirectroy(string dirPath)
@@ -133,20 +109,20 @@ public void DeleteEmptyDirectroy(string dirPath)
 
     /// <inheritdoc/>
     public string[] GetFiles(string path, string searchPattern, SearchOption searchOption)
-    {
-        return Directory.GetFiles(path, searchPattern, searchOption);
-    }
+        => Directory.GetFiles(path, searchPattern, searchOption);
 
     /// <inheritdoc/>
     public void Delete(string path)
-    {
-        File.Delete(path);
-    }
+        => File.Delete(path);
 
     public void DeleteDirectory(string directoryPath, bool recursive)
-    {
-        Directory.Delete(directoryPath, recursive);
-    }
+        => Directory.Delete(directoryPath, recursive);
+
+    public string GetTempPath()
+        => Path.GetTempPath();
+
+    public long GetFileLength(string path)
+        => new FileInfo(path).Length;
 }
 
 #endif

src/Microsoft.TestPlatform.CoreUtilities/Helpers/Interfaces/IFileHelper.cs

@@ -157,4 +157,16 @@ public interface IFileHelper
     /// </summary>
     /// <param name="path"></param>
     void Delete(string path);
+
+    /// <summary>
+    /// Get temporary file path
+    /// </summary>
+    /// <param name="path"></param>
+    public string GetTempPath();
+
+    /// <summary>
+    /// Get file length
+    /// </summary>
+    /// <param name="path"></param>
+    public long GetFileLength(string path);
 }

src/Microsoft.TestPlatform.CoreUtilities/PublicAPI/PublicAPI.Shipped.txt

@@ -71,6 +71,8 @@ Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces.IFileHelper.Cop
 Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces.IFileHelper.Delete(string path) -> void
 Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces.IFileHelper.DeleteEmptyDirectroy(string directoryPath) -> void
 Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces.IFileHelper.DeleteDirectory(string directoryPath, bool recursive) -> void
+Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces.IFileHelper.GetTempPath() -> string
+Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces.IFileHelper.GetFileLength(string path) -> long
 Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces.IFileHelper.DirectoryExists(string path) -> bool
 Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces.IFileHelper.Exists(string path) -> bool
 Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces.IFileHelper.GetCurrentDirectory() -> string

src/Microsoft.TestPlatform.CoreUtilities/PublicAPI/net/PublicAPI.Shipped.txt

@@ -57,6 +57,8 @@ Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.FileHelper.CreateDirectory
 Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.FileHelper.Delete(string path) -> void
 Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.FileHelper.DeleteEmptyDirectroy(string dirPath) -> void
 Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.FileHelper.DeleteDirectory(string directoryPath, bool recursive) -> void
+Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.FileHelper.GetTempPath() -> string
+Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.FileHelper.GetFileLength(string path) -> long
 Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.FileHelper.DirectoryExists(string path) -> bool
 Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.FileHelper.EnumerateFiles(string directory, System.IO.SearchOption searchOption, params string[] endsWithSearchPatterns) -> System.Collections.Generic.IEnumerable<string>
 Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.FileHelper.Exists(string path) -> bool

src/Microsoft.TestPlatform.CrossPlatEngine/PostProcessing/ArtifactProcessingManager.cs

@@ -1,8 +1,6 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
-#nullable disable
-
 namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.ArtifactProcessing;
 
 using System;
@@ -31,11 +29,11 @@ internal class ArtifactProcessingManager : IArtifactProcessingManager
     private const string RunsettingsFileName = "runsettings.xml";
     private const string ExecutionCompleteFileName = "executionComplete.json";
 
-    private readonly string _testSessionCorrelationId;
+    private readonly string? _testSessionCorrelationId;
     private readonly IFileHelper _fileHelper;
     private readonly ITestRunAttachmentsProcessingManager _testRunAttachmentsProcessingManager;
-    private readonly string _testSessionProcessArtifactFolder;
-    private readonly string _processArtifactFolder;
+    private readonly string? _testSessionProcessArtifactFolder;
+    private readonly string? _processArtifactFolder;
     private readonly IDataSerializer _dataSerialized;
     private readonly ITestRunAttachmentsProcessingEventsHandler _testRunAttachmentsProcessingEventsHandler;
     private readonly IFeatureFlag _featureFlag;
@@ -49,28 +47,29 @@ public ArtifactProcessingManager(string testSessionCorrelationId) :
             FeatureFlag.Instance)
     { }
 
-    public ArtifactProcessingManager(string testSessionCorrelationId,
-        IFileHelper fileHelper,
-        ITestRunAttachmentsProcessingManager testRunAttachmentsProcessingManager,
-        IDataSerializer dataSerialized,
-        ITestRunAttachmentsProcessingEventsHandler testRunAttachmentsProcessingEventsHandler,
-        IFeatureFlag featureFlag)
+    public ArtifactProcessingManager(string? testSessionCorrelationId,
+        IFileHelper fileHelper!!,
+        ITestRunAttachmentsProcessingManager testRunAttachmentsProcessingManager!!,
+        IDataSerializer dataSerialized!!,
+        ITestRunAttachmentsProcessingEventsHandler testRunAttachmentsProcessingEventsHandler!!,
+        IFeatureFlag featureFlag!!)
     {
+        _fileHelper = fileHelper;
+        _testRunAttachmentsProcessingManager = testRunAttachmentsProcessingManager;
+        _dataSerialized = dataSerialized;
+        _testRunAttachmentsProcessingEventsHandler = testRunAttachmentsProcessingEventsHandler;
+        _featureFlag = featureFlag;
+
         // We don't validate for null, it's expected, we'll have testSessionCorrelationId only in case of .NET SDK run.
         if (testSessionCorrelationId is not null)
         {
             _testSessionCorrelationId = testSessionCorrelationId;
-            _processArtifactFolder = Path.Combine(Path.GetTempPath(), _testSessionCorrelationId);
+            _processArtifactFolder = Path.Combine(_fileHelper.GetTempPath(), _testSessionCorrelationId);
             _testSessionProcessArtifactFolder = Path.Combine(_processArtifactFolder, $"{Process.GetCurrentProcess().Id}_{Guid.NewGuid()}");
         }
-        _fileHelper = fileHelper;
-        _testRunAttachmentsProcessingManager = testRunAttachmentsProcessingManager;
-        _dataSerialized = dataSerialized;
-        _testRunAttachmentsProcessingEventsHandler = testRunAttachmentsProcessingEventsHandler;
-        _featureFlag = featureFlag;
     }
 
-    public void CollectArtifacts(TestRunCompleteEventArgs testRunCompleteEventArgs, string runSettingsXml)
+    public void CollectArtifacts(TestRunCompleteEventArgs testRunCompleteEventArgs!!, string runSettingsXml!!)
     {
         if (!_featureFlag.IsEnabled(FeatureFlag.ARTIFACTS_POSTPROCESSING))
         {
@@ -159,14 +158,12 @@ private async Task DataCollectorsAttachmentsPostProcessing(TestArtifacts[] testA
     {
         // We take the biggest runsettings in size, it should be the one with more configuration.
         // In future we can think to merge...but it's not easy for custom config, we could break something.
-        string runsettingsFile = testArtifacts
+        string? runsettingsFile = testArtifacts
             .SelectMany(x => x.Artifacts.Where(x => x.Type == ArtifactType.Runsettings))
-            .Select(x => new FileInfo(x.FileName))
-            .OrderByDescending(x => x.Length)
-            .FirstOrDefault()?
-            .FullName;
+            .OrderByDescending(x => _fileHelper.GetFileLength(x.FileName))
+            .FirstOrDefault()?.FileName;
 
-        string runsettingsXml = null;
+        string? runsettingsXml = null;
         if (runsettingsFile is not null)
         {
             using var artifactStream = _fileHelper.GetStream(runsettingsFile, FileMode.Open, FileAccess.Read);
@@ -190,11 +187,11 @@ private async Task DataCollectorsAttachmentsPostProcessing(TestArtifacts[] testA
             string executionCompleteMessage = await streamReader.ReadToEndAsync();
             EqtTrace.Verbose($"ArtifactProcessingManager.MergeDataCollectorAttachments: ExecutionComplete message \n{executionCompleteMessage}");
             TestRunCompleteEventArgs eventArgs = _dataSerialized.DeserializePayload<TestRunCompleteEventArgs>(_dataSerialized.DeserializeMessage(executionCompleteMessage));
-            foreach (var invokedDataCollector in eventArgs?.InvokedDataCollectors)
+            foreach (var invokedDataCollector in eventArgs?.InvokedDataCollectors ?? Enumerable.Empty<InvokedDataCollector>())
             {
                 invokedDataCollectors.Add(invokedDataCollector);
             }
-            foreach (var attachmentSet in eventArgs?.AttachmentSets)
+            foreach (var attachmentSet in eventArgs?.AttachmentSets ?? Enumerable.Empty<AttachmentSet>())
             {
                 attachments.Add(attachmentSet);
             }
@@ -204,7 +201,7 @@ await _testRunAttachmentsProcessingManager.ProcessTestRunAttachmentsAsync(runset
             new RequestData()
             {
                 IsTelemetryOptedIn = IsTelemetryOptedIn(),
-                ProtocolConfig = ObjectModel.Constants.DefaultProtocolConfig
+                ProtocolConfig = Constants.DefaultProtocolConfig
             },
             attachments,
             invokedDataCollectors,
@@ -219,11 +216,11 @@ private TestArtifacts[] LoadTestArtifacts() => _fileHelper.GetFiles(_processArti
         .Select(testSessionArtifact => new TestArtifacts(testSessionArtifact.Key, testSessionArtifact.Select(x => ParseArtifact(x.Artifact)).Where(x => x is not null).ToArray()))
         .ToArray();
 
-    private static Artifact ParseArtifact(string fileName) =>
+    private static Artifact? ParseArtifact(string fileName!!) =>
         Path.GetFileName(fileName) switch
         {
-            RunsettingsFileName => new Artifact(fileName, ArtifactType.ExecutionComplete),
-            ExecutionCompleteFileName => new Artifact(fileName, ArtifactType.Runsettings),
+            RunsettingsFileName => new Artifact(fileName, ArtifactType.Runsettings),
+            ExecutionCompleteFileName => new Artifact(fileName, ArtifactType.ExecutionComplete),
             _ => null
         };
 

src/vstest.console/CommandLine/CommandLineOptions.cs

@@ -81,7 +81,7 @@ internal static CommandLineOptions Instance
     /// <summary>
     /// Default constructor.
     /// </summary>
-    protected CommandLineOptions()
+    internal CommandLineOptions()
     {
         BatchSize = DefaultBatchSize;
         TestStatsEventTimeout = _defaultRetrievalTimeout;

src/vstest.console/Processors/ArtifactProcessingCollectModeProcessor.cs

@@ -32,7 +32,7 @@ public Lazy<IArgumentProcessorCapabilities> Metadata
         {
             if (_metadata == null)
             {
-                _metadata = new Lazy<IArgumentProcessorCapabilities>(() => new ArtifactProcessingCollectModeProcessorCapabilities(CommandLineOptions.Instance));
+                _metadata = new Lazy<IArgumentProcessorCapabilities>(() => new ArtifactProcessingCollectModeProcessorCapabilities());
             }
 
             return _metadata;
@@ -63,13 +63,6 @@ public Lazy<IArgumentExecutor> Executor
 
 internal class ArtifactProcessingCollectModeProcessorCapabilities : BaseArgumentProcessorCapabilities
 {
-    private readonly CommandLineOptions _commandLineOptions;
-
-    public ArtifactProcessingCollectModeProcessorCapabilities(CommandLineOptions options)
-    {
-        _commandLineOptions = options ?? throw new ArgumentNullException(nameof(options));
-    }
-
     public override string CommandName => ArtifactProcessingCollectModeProcessor.CommandName;
 
     public override bool AllowMultiple => false;

src/vstest.console/Processors/ArtifactProcessingPostProcessModeProcessor.cs

@@ -36,7 +36,7 @@ public Lazy<IArgumentProcessorCapabilities> Metadata
         {
             if (_metadata == null)
             {
-                _metadata = new Lazy<IArgumentProcessorCapabilities>(() => new ArtifactProcessingPostProcessModeProcessorCapabilities(CommandLineOptions.Instance));
+                _metadata = new Lazy<IArgumentProcessorCapabilities>(() => new ArtifactProcessingPostProcessModeProcessorCapabilities());
             }
 
             return _metadata;
@@ -73,13 +73,6 @@ public static bool ContainsPostProcessCommand(string[] args, IFeatureFlag featur
 
 internal class ArtifactProcessingPostProcessModeProcessorCapabilities : BaseArgumentProcessorCapabilities
 {
-    private readonly CommandLineOptions _commandLineOptions;
-
-    public ArtifactProcessingPostProcessModeProcessorCapabilities(CommandLineOptions options)
-    {
-        _commandLineOptions = options ?? throw new ArgumentNullException(nameof(options));
-    }
-
     public override string CommandName => ArtifactProcessingPostProcessModeProcessor.CommandName;
 
     public override bool AllowMultiple => false;