Fix config parser and logging path to support (#620)

* Fix flaw in configuration parser error detection for negative clauses
* Logging paths can't have wildcards, but runtime specifiers can and this is used in our log names. In that case I resorted to use <Major.Minor.Patch> as the version for the log while preserving old behavior. That way xUnit still displays the full arg list for test disambiguation, but the logs only contain valid file names

diff --git a/src/Microsoft.Diagnostics.TestHelpers/TestConfiguration.cs b/src/Microsoft.Diagnostics.TestHelpers/TestConfiguration.cs
index 30c287694017cd62222021d36dd8bad7744dff88..35885c0f6ef69bb9d21f140a46fd1cc696b2f980 100644 (file)
--- a/src/Microsoft.Diagnostics.TestHelpers/TestConfiguration.cs
+++ b/src/Microsoft.Diagnostics.TestHelpers/TestConfiguration.cs
@@ -4,11 +4,13 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using System.Runtime.InteropServices;
 using System.Text;
+using System.Text.RegularExpressions;
 using System.Xml.Linq;
 using Xunit;
 using Xunit.Extensions;
@@ -83,7 +85,7 @@ namespace Microsoft.Diagnostics.TestHelpers
                 initialConfig["WinDir"] = Path.GetFullPath(Environment.GetEnvironmentVariable("WINDIR"));
             }
             IEnumerable<Dictionary<string, string>> configs = ParseConfigFile(path, new Dictionary<string, string>[] { initialConfig });
-            Configurations = configs.Select(c => new TestConfiguration(c));
+            Configurations = configs.Select(c => new TestConfiguration(c)).ToList();
         }
 
         Dictionary<string, string>[] ParseConfigFile(string path, Dictionary<string, string>[] templates)
@@ -196,7 +198,7 @@ namespace Microsoft.Diagnostics.TestHelpers
 
             foreach (XAttribute attr in node.Attributes("Condition"))
             {
-                string conditionText = attr.Value;
+                string conditionText = attr.Value.Trim();
                 bool isNegative = conditionText.Length > 0 && conditionText[0] == '!';
 
                 // Check if Exists('<directory or file>')
@@ -264,7 +266,10 @@ namespace Microsoft.Diagnostics.TestHelpers
                 return false;
             }
 
-            if (functionKeyworkIndex != 0 || functionKeyworkIndex >= 1 && expression[0] != '!' || !expression.EndsWith(')'))
+            if ((functionKeyworkIndex != 0 
+                    && functionKeyworkIndex == 1 && expression[0] != '!')
+                || functionKeyworkIndex > 1
+                || !expression.EndsWith(')'))
             {
                 throw new InvalidDataException($"Condition {expression} malformed. Currently only single-function conditions are supported.");
             }
@@ -359,19 +364,74 @@ namespace Microsoft.Diagnostics.TestHelpers
     {
         const string DebugTypeKey = "DebugType";
         const string DebuggeeBuildRootKey = "DebuggeeBuildRoot";
-
+        
         internal static readonly string BaseDir = Path.GetFullPath(".");
+        private static readonly Regex versionRegex = new Regex(@"^(\d+\.\d+\.\d+)(-.*)?", RegexOptions.Compiled);
+        
+        private ReadOnlyDictionary<string, string> _settings;
+        private readonly string _configStringView;
+        private readonly string _truncatedRuntimeFrameworkVersion;
 
-        private Dictionary<string, string> _settings;
 
         public TestConfiguration()
         {
-            _settings = new Dictionary<string, string>();
+            _settings = new ReadOnlyDictionary<string, string>(new Dictionary<string, string>());
+            _truncatedRuntimeFrameworkVersion = null;
+            _configStringView = string.Empty;
         }
 
         public TestConfiguration(Dictionary<string, string> initialSettings)
         {
-            _settings = new Dictionary<string, string>(initialSettings);
+            _settings = new ReadOnlyDictionary<string, string>(new Dictionary<string, string>(initialSettings));
+            _truncatedRuntimeFrameworkVersion = GetTruncatedRuntimeFrameworkVersion();
+            _configStringView = GetStringViewWithVersion(RuntimeFrameworkVersion); 
+        }
+
+        private string GetTruncatedRuntimeFrameworkVersion()
+        {
+            string version = RuntimeFrameworkVersion;
+            if (string.IsNullOrEmpty(version))
+            {
+                return null;
+            }
+
+            Match matchingVer = versionRegex.Match(version);
+            if (!matchingVer.Success)
+            {
+                throw new InvalidDataException($"{version} is not a valid version string according to SemVer.");
+            }
+
+            return matchingVer.Groups[1].Value;
+        }
+
+        private string GetStringViewWithVersion(string version)
+        {
+            var sb = new StringBuilder();
+            sb.Append(TestProduct);
+            sb.Append(".");
+            sb.Append(DebuggeeBuildProcess);
+            if (!string.IsNullOrEmpty(version))
+            {
+                sb.Append(".");
+                sb.Append(version);
+            }
+            return sb.ToString();
+        }
+
+        internal string GetLogSuffix()
+        {
+            string version = RuntimeFrameworkVersion;
+
+            // The log name can't contain wild cards, which are used in some testing scenarios.
+            // TODO: The better solution would be to sanitize the file name properly, in case
+            // there's a key being used that contains a character that is not a valid file
+            // name charater.
+            if (!string.IsNullOrEmpty(version) && version.Contains('*'))
+            {
+                version = _truncatedRuntimeFrameworkVersion;
+            }
+
+            return GetStringViewWithVersion(version);
         }
 
         public IReadOnlyDictionary<string, string> AllSettings
@@ -749,17 +809,7 @@ namespace Microsoft.Diagnostics.TestHelpers
 
         public override string ToString()
         {
-            var sb = new StringBuilder();
-            sb.Append(TestProduct);
-            sb.Append(".");
-            sb.Append(DebuggeeBuildProcess);
-            string version = RuntimeFrameworkVersion;
-            if (!string.IsNullOrEmpty(version))
-            {
-                sb.Append(".");
-                sb.Append(version);
-            }
-            return sb.ToString();
+            return _configStringView;
         }
     }
 

diff --git a/src/Microsoft.Diagnostics.TestHelpers/TestRunner.cs b/src/Microsoft.Diagnostics.TestHelpers/TestRunner.cs
index c12c5328f1b73324b5f90d05c8168fce5292a55f..bc8e2f7d5c883e2d83144c3d96f2f78000c26d43 100644 (file)
--- a/src/Microsoft.Diagnostics.TestHelpers/TestRunner.cs
+++ b/src/Microsoft.Diagnostics.TestHelpers/TestRunner.cs
@@ -139,7 +139,7 @@ namespace Microsoft.Diagnostics.TestHelpers
             ConsoleTestOutputHelper consoleLogger = null;
             if (!string.IsNullOrEmpty(config.LogDirPath))
             {
-                string logFileName = testName + "." + config.ToString() + ".log";
+                string logFileName = $"{ testName }.{ config.GetLogSuffix() }.log";
                 string logPath = Path.Combine(config.LogDirPath, logFileName);
                 fileLogger = new FileTestOutputHelper(logPath, FileMode.Append);
             }