Fix attachment uri in trx if same attachment filename is same (#1625)

* Fix attachment uri in trx

* Add unit test

* Add null check while logging

* Log different message on null attachmentSets

src/Microsoft.TestPlatform.Extensions.TrxLogger/Utility/Converter.cs

@@ -148,12 +148,19 @@ public static TrxObjectModel.TestOutcome ToOutcome(ObjectModel.TestOutcome rockS
 
         public static List<CollectorDataEntry> ToCollectionEntries(IEnumerable<ObjectModel.AttachmentSet> attachmentSets, TestRun testRun, string trxFileDirectory)
         {
+
             List<CollectorDataEntry> collectorEntries = new List<CollectorDataEntry>();
             if (attachmentSets == null)
             {
+                EqtTrace.Info($"Converter.ToCollectionEntries: Received {nameof(attachmentSets)} as null returning empty collection entries.");
                 return collectorEntries;
             }
 
+            if (EqtTrace.IsInfoEnabled)
+            {
+                EqtTrace.Info($"Converter.ToCollectionEntries: Converting attachmentSets {string.Join(",", attachmentSets)} to collection entries.");
+            }
+
             foreach (var attachmentSet in attachmentSets)
             {
                 if (attachmentSet.Uri.AbsoluteUri.StartsWith(Constants.DataCollectorUriPrefix, StringComparison.OrdinalIgnoreCase))
@@ -456,9 +463,9 @@ private static CollectorDataEntry ToCollectorEntry(ObjectModel.AttachmentSet att
                 string targetFileName = FileHelper.GetNextIterationFileName(targetDirectory, Path.GetFileName(sourceFile), false);
                 CopyFile(sourceFile, targetFileName);
 
-                // Add the source file name to the collector files list. 
-                // (Trx viewer automatically adds In\ to the collected file. 
-                string fileName = Path.Combine(Environment.MachineName, Path.GetFileName(sourceFile));
+                // Add the target file name to the collector files list.
+                // (Trx viewer automatically adds In\ to the collected file.
+                string fileName = Path.Combine(Environment.MachineName, Path.GetFileName(targetFileName));
                 Uri sourceFileUri = new Uri(fileName, UriKind.Relative);
                 TrxObjectModel.UriDataAttachment dataAttachment = new TrxObjectModel.UriDataAttachment(uriDataAttachment.Description, sourceFileUri);
 
@@ -507,8 +514,8 @@ private static IList<string> ToResultFiles(ObjectModel.AttachmentSet attachmentS
                 string targetFileName = FileHelper.GetNextIterationFileName(testResultDirectory, Path.GetFileName(sourceFile), false);
                 CopyFile(sourceFile, targetFileName);
 
-                // Add the source file name to the result files list. 
-                // (Trx viewer automatically adds In\<Guid> to the result file. 
+                // Add the target file name to the result files list.
+                // (Trx viewer automatically adds In\<Guid> to the result file.
                 string fileName = Path.Combine(Environment.MachineName, Path.GetFileName(targetFileName));
                 resultFiles.Add(fileName);
             }

src/Microsoft.TestPlatform.ObjectModel/AttachmentSet.cs

@@ -40,6 +40,11 @@ public AttachmentSet(Uri uri, string displayName)
             DisplayName = displayName;
             Attachments = new List<UriDataAttachment>();
         }
+
+        public override string ToString()
+        {
+            return $"{nameof(Uri)}: {Uri.AbsoluteUri}, {nameof(DisplayName)}: {DisplayName}, {nameof(Attachments)}: [{ string.Join(",", Attachments)}]";
+        }
     }
 
 
@@ -67,5 +72,10 @@ public UriDataAttachment(Uri uri, string description)
             Uri = uri;
             Description = description;
         }
+
+        public override string ToString()
+        {
+            return $"{nameof(Uri)}: {Uri.AbsoluteUri}, {nameof(Description)}: {Description}";
+        }
     }
 }

test/Microsoft.TestPlatform.Extensions.TrxLogger.UnitTests/Microsoft.TestPlatform.Extensions.TrxLogger.UnitTests.csproj

@@ -3,6 +3,8 @@
   <PropertyGroup>
     <TestPlatformRoot Condition="$(TestPlatformRoot) == ''">..\..\</TestPlatformRoot>
     <TestProject>true</TestProject>
+    <WarningsAsErrors>true</WarningsAsErrors>
+    <EnableCodeAnalysis>true</EnableCodeAnalysis>
   </PropertyGroup>
   <Import Project="$(TestPlatformRoot)scripts/build/TestPlatform.Settings.targets" />
   <PropertyGroup>

test/Microsoft.TestPlatform.Extensions.TrxLogger.UnitTests/Utility/ConverterTests.cs

@@ -6,8 +6,13 @@ namespace Microsoft.TestPlatform.Extensions.TrxLogger.UnitTests.Utility
     using Microsoft.TestPlatform.Extensions.TrxLogger.Utility;
     using Microsoft.VisualStudio.TestPlatform.ObjectModel;
     using Microsoft.VisualStudio.TestTools.UnitTesting;
-
+    using ObjectModel;
+    using System;
+    using System.Collections.Generic;
+    using System.IO;
+    using TestOutcome = VisualStudio.TestPlatform.ObjectModel.TestOutcome;
     using TrxLoggerOutcome = Microsoft.TestPlatform.Extensions.TrxLogger.ObjectModel.TestOutcome;
+    using UriDataAttachment = VisualStudio.TestPlatform.ObjectModel.UriDataAttachment;
 
     [TestClass]
     public class ConverterTests
@@ -41,5 +46,57 @@ public void ToOutcomeShouldMapNotFoundToNotExecuted()
         {
             Assert.AreEqual(TrxLoggerOutcome.NotExecuted, Converter.ToOutcome(TestOutcome.NotFound));
         }
+
+        [TestMethod]
+        public void ToCollectionEntriesShouldRenameAttachmentUriIfTheAttachmentNameIsSame()
+        {
+            ConverterTests.SetupForToCollectionEntries(out var tempDir, out var attachmentSets, out var testRun, out var testResultsDirectory);
+
+            List<CollectorDataEntry> collectorDataEntries = Converter.ToCollectionEntries(attachmentSets, testRun, testResultsDirectory);
+
+            Assert.AreEqual($@"{Environment.MachineName}\123.coverage", ((ObjectModel.UriDataAttachment) collectorDataEntries[0].Attachments[0]).Uri.OriginalString);
+            Assert.AreEqual($@"{Environment.MachineName}\123[1].coverage", ((ObjectModel.UriDataAttachment)collectorDataEntries[0].Attachments[1]).Uri.OriginalString);
+
+            Directory.Delete(tempDir, true);
+        }
+
+        private static void SetupForToCollectionEntries(out string tempDir, out List<AttachmentSet> attachmentSets, out TestRun testRun,
+            out string testResultsDirectory)
+        {
+            ConverterTests.CreateTempCoverageFiles(out tempDir, out var coverageFilePath1, out var coverageFilePath2);
+
+            UriDataAttachment uriDataAttachment1 =
+                new UriDataAttachment(new Uri($"file:///{coverageFilePath1}"), "Description 1");
+            UriDataAttachment uriDataAttachment2 =
+                new UriDataAttachment(new Uri($"file:///{coverageFilePath2}"), "Description 2");
+            attachmentSets = new List<AttachmentSet>
+            {
+                new AttachmentSet(new Uri("datacollector://microsoft/CodeCoverage/2.0"), "Code Coverage")
+            };
+
+            testRun = new TestRun(Guid.NewGuid());
+            testRun.RunConfiguration = new TestRunConfiguration("Testrun 1");
+            attachmentSets[0].Attachments.Add(uriDataAttachment1);
+            attachmentSets[0].Attachments.Add(uriDataAttachment2);
+            testResultsDirectory = Path.Combine(tempDir, "TestResults");
+        }
+
+        private static void CreateTempCoverageFiles(out string tempDir, out string coverageFilePath1,
+            out string coverageFilePath2)
+        {
+            tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
+
+            var covDir1 = Path.Combine(tempDir, Guid.NewGuid().ToString());
+            var covDir2 = Path.Combine(tempDir, Guid.NewGuid().ToString());
+
+            Directory.CreateDirectory(covDir1);
+            Directory.CreateDirectory(covDir2);
+
+            coverageFilePath1 = Path.Combine(covDir1, "123.coverage");
+            coverageFilePath2 = Path.Combine(covDir2, "123.coverage");
+
+            File.WriteAllText(coverageFilePath1, string.Empty);
+            File.WriteAllText(coverageFilePath2, string.Empty);
+        }
     }
 }