--diag should take files with no extension (#3048)

Files with no extension are totally valid files, even though they are unusual on Windows Fix #3045
microsoft · Sep 10, 2021 · 30345de · 30345de
1 parent 487db55
commit 30345de
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 27 deletions.
diff --git a/src/vstest.console/Processors/EnableDiagArgumentProcessor.cs b/src/vstest.console/Processors/EnableDiagArgumentProcessor.cs
@@ -225,13 +225,6 @@ private string GetDiagFilePath(string diagFilePathArgument)
             // Remove double quotes if present.
             diagFilePathArgument = diagFilePathArgument.Replace("\"", "");
 
-            // Throw error in case diag file path is not a valid file path
-            var fileExtension = Path.GetExtension(diagFilePathArgument);
-            if (string.IsNullOrWhiteSpace(fileExtension))
-            {
-                throw new CommandLineException(string.Format(CultureInfo.CurrentCulture, CommandLineResources.InvalidDiagFilePath, diagFilePathArgument));
-            }
-
             // Create base directory for diag file path (if doesn't exist)
             CreateDirectoryIfNotExists(diagFilePathArgument);
 

diff --git a/test/vstest.console.UnitTests/Processors/EnableDiagArgumentProcessorTests.cs b/test/vstest.console.UnitTests/Processors/EnableDiagArgumentProcessorTests.cs
@@ -90,20 +90,11 @@ public void EnableDiagArgumentProcessorExecutorDoesNotThrowsIfFileDotOpenThrow()
             this.diagProcessor.Executor.Value.Initialize(this.dummyFilePath);
         }
 
-        [TestMethod]
-        [DataRow("abs;dfsdc.txt;verbosity=normal", "abs")] // ; in file path is not supported
-        [DataRow("\"abst;dfsdc.txt\";verbosity=normal", "abst")] // Even though in escaped double quotes, semi colon is not supported in file path
-        [DataRow("foo", "foo")]
-        public void EnableDiagArgumentProcessorExecutorShouldThrowIfDirectoryPathIsProvided(string argument, string filePath)
-        {
-            var exceptionMessage = string.Format(CultureInfo.CurrentCulture, CommandLineResources.InvalidDiagFilePath, filePath);
-
-            EnableDiagArgumentProcessorExecutorShouldThrowIfInvalidArgument(argument, exceptionMessage);
-        }
-
         [TestMethod]
         [DataRow("abc.txt;verbosity=normal=verbose")] // Multiple '=' in parameter
         [DataRow("abc.txt;verbosity;key1=value1")] // No '=' in parameter
+        [DataRow("\"abst;dfsdc.txt\";verbosity=normal")] // Too many parameters
+        [DataRow("abs;dfsdc.txt;verbosity=normal")] // Too many parameters
         public void EnableDiagArgumentProcessorExecutorShouldThrowIfInvalidArgument(string argument)
         {
             string exceptionMessage = string.Format(CultureInfo.CurrentCulture, CommandLineResources.InvalidDiagArgument, argument);
@@ -116,6 +107,15 @@ public void EnableDiagArgumentProcessorExecutorShouldThrowIfInvalidArgument(stri
         [DataRow("abc.txt;tracelevel=info;newkey=newvalue")]
         [DataRow("\"abc.txt\";verbosity=normal;newkey=newvalue")] //escaped double quotes are allowed for file path.
         [DataRow(";;abc.txt;;;;verbosity=normal;;;;")]
+        // Files with no extension are totally valid files.
+        // When we delegate to host or datacollector we change the name in GetTimestampedLogFile
+        // Path.ChangeExtension replaces the curent extension with our new one that is timestamp and 
+        // the name of the target (e.g. host). When there is no extension it just adds it, so we do either: 
+        // log.txt -> log.host.21-09-10_12-25-41_68765_5.txt
+        // log.my.txt -> log.my.host.21-09-10_12-25-50_55183_5.txt
+        // log -> log.host.21-09-10_12-25-27_94286_5
+        [DataRow("log")]
+        [DataRow("log.log")]
         public void EnableDiagArgumentProcessorExecutorShouldNotThrowIfValidArgument(string argument)
         {
             try
@@ -192,15 +192,8 @@ public TestableEnableDiagArgumentProcessor(IFileHelper fileHelper)
 
         private void EnableDiagArgumentProcessorExecutorShouldThrowIfInvalidArgument(string argument, string exceptionMessage)
         {
-            try
-            {
-                this.diagProcessor.Executor.Value.Initialize(argument);
-            }
-            catch (Exception e)
-            {
-                Assert.IsTrue(e.GetType().Equals(typeof(CommandLineException)));
-                Assert.IsTrue(e.Message.Contains(exceptionMessage));
-            }
+            var e = Assert.ThrowsException<CommandLineException>(() => this.diagProcessor.Executor.Value.Initialize(argument));
+            StringAssert.Contains(e.Message, exceptionMessage);
         }
     }
 }