From b7a24c401991ffca5071939d8d2cef7bfa28034b Mon Sep 17 00:00:00 2001 From: Juan Hoyos Date: Fri, 15 Nov 2019 10:50:26 -0800 Subject: [PATCH] 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 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 --- .../TestConfiguration.cs | 86 +++++++++++++++---- .../TestRunner.cs | 2 +- 2 files changed, 69 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.Diagnostics.TestHelpers/TestConfiguration.cs b/src/Microsoft.Diagnostics.TestHelpers/TestConfiguration.cs index 30c287694..35885c0f6 100644 --- 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> configs = ParseConfigFile(path, new Dictionary[] { initialConfig }); - Configurations = configs.Select(c => new TestConfiguration(c)); + Configurations = configs.Select(c => new TestConfiguration(c)).ToList(); } Dictionary[] ParseConfigFile(string path, Dictionary[] 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('') @@ -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 _settings; + private readonly string _configStringView; + private readonly string _truncatedRuntimeFrameworkVersion; - private Dictionary _settings; public TestConfiguration() { - _settings = new Dictionary(); + _settings = new ReadOnlyDictionary(new Dictionary()); + _truncatedRuntimeFrameworkVersion = null; + _configStringView = string.Empty; } public TestConfiguration(Dictionary initialSettings) { - _settings = new Dictionary(initialSettings); + _settings = new ReadOnlyDictionary(new Dictionary(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 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 c12c5328f..bc8e2f7d5 100644 --- 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); } -- 2.34.1