Handle illegal xml characters in test data

src/TestLogger/Core/IInputSanitizer.cs

@@ -0,0 +1,10 @@
+// Copyright (c) Spekt Contributors. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+namespace Spekt.TestLogger.Core
+{
+    public interface IInputSanitizer
+    {
+        string Sanitize(string input);
+    }
+}

src/TestLogger/Core/ITestResultSerializer.cs

@@ -7,6 +7,8 @@ namespace Spekt.TestLogger.Core
 
     public interface ITestResultSerializer
     {
+        IInputSanitizer InputSanitizer { get; }
+
         string Serialize(
             LoggerConfiguration loggerConfiguration,
             TestRunConfiguration runConfiguration,

src/TestLogger/Core/InputSanitizerXml.cs

@@ -0,0 +1,35 @@
+// Copyright (c) Spekt Contributors. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+namespace Spekt.TestLogger.Core
+{
+    using System.Text.RegularExpressions;
+
+    public class InputSanitizerXml : IInputSanitizer
+    {
+        private static readonly Regex InvalidXmlChar = new (@"([^\x09\x0A\x0D\x20-\uD7FF\uE000-\uFFFD]|[\u007F-\u0084\u0086-\u009F\uFDD0-\uFDEF])", RegexOptions.Compiled);
+
+        public string Sanitize(string input)
+        {
+            if (input == null)
+            {
+                return null;
+            }
+
+            // From xml spec (http://www.w3.org/TR/xml/#charsets) valid chars:
+            // #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF]
+            // Following control charset are discouraged:
+            // [#x7F-#x84], [#x86-#x9F], [#xFDD0-#xFDEF],
+            // We are handling only #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD]
+            // because C# support unicode character in range \u0000 to \uFFFF
+            var evaluator = new MatchEvaluator(ReplaceInvalidCharacterWithUniCodeEscapeSequence);
+            return InvalidXmlChar.Replace(input, evaluator);
+
+            static string ReplaceInvalidCharacterWithUniCodeEscapeSequence(Match match)
+            {
+                char x = match.Value[0];
+                return $@"\u{(ushort)x:x4}";
+            }
+        }
+    }
+}

src/TestLogger/Core/TestMessageInfo.cs

@@ -3,15 +3,22 @@
 
 namespace Spekt.TestLogger.Core
 {
+    using System;
     using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging;
 
     /// <summary>
     /// A message generated during the test run.
     /// </summary>
     public class TestMessageInfo
     {
-        public TestMessageLevel Level { get; set; }
+        public TestMessageInfo(TestMessageLevel level, string message)
+        {
+            this.Level = level;
+            this.Message = message ?? throw new ArgumentNullException(nameof(message));
+        }
 
-        public string Message { get; set; }
+        public TestMessageLevel Level { get; }
+
+        public string Message { get; }
     }
 }

src/TestLogger/Core/TestResultInfo.cs

@@ -4,75 +4,144 @@
 namespace Spekt.TestLogger.Core
 {
     using System;
-    using System.Collections.ObjectModel;
+    using System.Collections.Generic;
     using Microsoft.VisualStudio.TestPlatform.ObjectModel;
-    using Spekt.TestLogger.Extensions;
 
     public sealed class TestResultInfo
     {
-        private readonly TestResult result;
-
         public TestResultInfo(
-            TestResult result,
             string @namespace,
             string type,
-            string method)
+            string method,
+            TestOutcome outcome,
+            string testResultDisplayName,
+            string testCaseDisplayName,
+            string assemblyPath,
+            string codeFilePath,
+            int lineNumber,
+            DateTime startTime,
+            DateTime endTime,
+            TimeSpan duration,
+            string errorMessage,
+            string errorStackTrace,
+            List<TestResultMessage> messages,
+            IReadOnlyCollection<Trait> traits,
+            string executorUri)
         {
-            this.result = result;
             this.Namespace = @namespace;
             this.Type = type;
             this.Method = method;
-            this.Outcome = result.Outcome;
+            this.Outcome = outcome;
+            this.TestResultDisplayName = testResultDisplayName;
+            this.TestCaseDisplayName = testCaseDisplayName;
+            this.AssemblyPath = assemblyPath;
+            this.CodeFilePath = codeFilePath;
+            this.LineNumber = lineNumber;
+            this.StartTime = startTime;
+            this.EndTime = endTime;
+            this.Duration = duration;
+            this.ErrorMessage = errorMessage;
+            this.ErrorStackTrace = errorStackTrace;
+            this.Messages = messages;
+            this.Traits = traits;
+            this.ExecutorUri = executorUri;
         }
 
-        public TestCase TestCase => this.result.TestCase;
-
-        public TestOutcome Outcome { get; set; }
-
-        public string AssemblyPath => this.result.TestCase.Source;
+        public string Namespace { get; }
 
-        public string Namespace { get; private set; }
-
-        public string Type { get; private set; }
-
-        public string FullTypeName => this.Namespace + "." + this.Type;
+        public string Type { get; }
 
         /// <summary>
         /// Gets a string that contain the method name, along with any paramaterized data related to
         /// the method. For example, `SomeMethod` or `SomeParameterizedMethod(true)`.
         /// </summary>
         public string Method { get; internal set; }
 
-        public DateTime StartTime => this.result.StartTime.UtcDateTime;
+        public TestOutcome Outcome { get; set; }
 
-        public DateTime EndTime => this.result.EndTime.UtcDateTime;
+        public string AssemblyPath { get; }
 
-        public TimeSpan Duration => this.result.Duration;
+        public string CodeFilePath { get; }
 
-        public string ErrorMessage => this.result.ErrorMessage;
+        public int LineNumber { get; }
 
-        public string ErrorStackTrace => this.result.ErrorStackTrace;
+        public DateTime StartTime { get; }
 
-        public Collection<TestResultMessage> Messages => this.result.Messages;
+        public DateTime EndTime { get; }
 
-        public TraitCollection Traits => this.result.Traits;
+        public TimeSpan Duration { get; }
 
-        internal TestResult Result => this.result;
+        public string ErrorMessage { get; }
 
-        public override int GetHashCode()
-        {
-            return this.result.GetHashCode();
-        }
+        public string ErrorStackTrace { get; }
+
+        public List<TestResultMessage> Messages { get; }
+
+        public IReadOnlyCollection<Trait> Traits { get; }
+
+        public string ExecutorUri { get; }
+
+        public string FullTypeName => this.Namespace + "." + this.Type;
+
+        /// <summary>
+        /// Gets value that originated at <see cref="TestResult.DisplayName"/>. Intended for use within
+        /// this library by framework specific adapters, to ensure that <see cref="Method"/> has the
+        /// proper value.
+        /// </summary>
+        internal string TestResultDisplayName { get; }
+
+        /// <summary>
+        /// Gets value that originated at <see cref="TestCase.DisplayName"/>. Intended for use within
+        /// this library by framework specific adapters, to ensure that <see cref="Method"/> has the
+        /// proper value.
+        /// </summary>
+        internal string TestCaseDisplayName { get; }
 
         public override bool Equals(object obj)
         {
-            if (obj is not TestResultInfo objectToCompare)
-            {
-                return false;
-            }
+            return obj is TestResultInfo info &&
+                   this.Namespace == info.Namespace &&
+                   this.Type == info.Type &&
+                   this.Method == info.Method &&
+                   this.Outcome == info.Outcome &&
+                   this.TestResultDisplayName == info.TestResultDisplayName &&
+                   this.TestCaseDisplayName == info.TestCaseDisplayName &&
+                   this.AssemblyPath == info.AssemblyPath &&
+                   this.CodeFilePath == info.CodeFilePath &&
+                   this.LineNumber == info.LineNumber &&
+                   this.StartTime == info.StartTime &&
+                   this.EndTime == info.EndTime &&
+                   this.Duration.Equals(info.Duration) &&
+                   this.ErrorMessage == info.ErrorMessage &&
+                   this.ErrorStackTrace == info.ErrorStackTrace &&
+                   EqualityComparer<List<TestResultMessage>>.Default.Equals(this.Messages, info.Messages) &&
+                   EqualityComparer<IReadOnlyCollection<Trait>>.Default.Equals(this.Traits, info.Traits) &&
+                   this.ExecutorUri == info.ExecutorUri &&
+                   this.FullTypeName == info.FullTypeName;
+        }
 
-            return string.Compare(this.ErrorMessage, objectToCompare.ErrorMessage, StringComparison.CurrentCulture) == 0
-                   && string.Compare(this.ErrorStackTrace, objectToCompare.ErrorStackTrace, StringComparison.CurrentCulture) == 0;
+        public override int GetHashCode()
+        {
+            int hashCode = -1082088776;
+            hashCode = (hashCode * -1521134295) + EqualityComparer<string>.Default.GetHashCode(this.Namespace);
+            hashCode = (hashCode * -1521134295) + EqualityComparer<string>.Default.GetHashCode(this.Type);
+            hashCode = (hashCode * -1521134295) + EqualityComparer<string>.Default.GetHashCode(this.Method);
+            hashCode = (hashCode * -1521134295) + this.Outcome.GetHashCode();
+            hashCode = (hashCode * -1521134295) + EqualityComparer<string>.Default.GetHashCode(this.TestResultDisplayName);
+            hashCode = (hashCode * -1521134295) + EqualityComparer<string>.Default.GetHashCode(this.TestCaseDisplayName);
+            hashCode = (hashCode * -1521134295) + EqualityComparer<string>.Default.GetHashCode(this.AssemblyPath);
+            hashCode = (hashCode * -1521134295) + EqualityComparer<string>.Default.GetHashCode(this.CodeFilePath);
+            hashCode = (hashCode * -1521134295) + this.LineNumber.GetHashCode();
+            hashCode = (hashCode * -1521134295) + this.StartTime.GetHashCode();
+            hashCode = (hashCode * -1521134295) + this.EndTime.GetHashCode();
+            hashCode = (hashCode * -1521134295) + this.Duration.GetHashCode();
+            hashCode = (hashCode * -1521134295) + EqualityComparer<string>.Default.GetHashCode(this.ErrorMessage);
+            hashCode = (hashCode * -1521134295) + EqualityComparer<string>.Default.GetHashCode(this.ErrorStackTrace);
+            hashCode = (hashCode * -1521134295) + EqualityComparer<List<TestResultMessage>>.Default.GetHashCode(this.Messages);
+            hashCode = (hashCode * -1521134295) + EqualityComparer<IReadOnlyCollection<Trait>>.Default.GetHashCode(this.Traits);
+            hashCode = (hashCode * -1521134295) + EqualityComparer<string>.Default.GetHashCode(this.ExecutorUri);
+            hashCode = (hashCode * -1521134295) + EqualityComparer<string>.Default.GetHashCode(this.FullTypeName);
+            return hashCode;
         }
     }
 }

src/TestLogger/Core/TestRunCompleteWorkflow.cs

@@ -24,8 +24,7 @@ public static void Complete(this ITestRun testRun, TestRunCompleteEventArgs comp
             var transformedResults = results;
             if (transformedResults.Any())
             {
-                var executorUri = transformedResults[0]
-                    .TestCase.ExecutorUri?.ToString();
+                var executorUri = transformedResults[0].ExecutorUri;
                 var adapter = testRun.AdapterFactory.CreateTestAdapter(executorUri);
                 transformedResults = adapter.TransformResults(results, messages);
             }

src/TestLogger/Core/TestRunMessageWorkflow.cs

@@ -9,7 +9,10 @@ public static class TestRunMessageWorkflow
     {
         public static void Message(this ITestRun testRun, TestRunMessageEventArgs messageEvent)
         {
-            testRun.Store.Add(new TestMessageInfo { Level = messageEvent.Level, Message = messageEvent.Message });
+            testRun.Store.Add(
+                new TestMessageInfo(
+                    messageEvent.Level,
+                    testRun.Serializer.InputSanitizer.Sanitize(messageEvent.Message)));
         }
     }
 }

src/TestLogger/Core/TestRunResultWorkflow.cs

@@ -4,6 +4,8 @@
 namespace Spekt.TestLogger.Core
 {
     using System;
+    using System.Linq;
+    using Microsoft.VisualStudio.TestPlatform.ObjectModel;
     using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging;
 
     public static class TestRunResultWorkflow
@@ -22,11 +24,27 @@ string x when x.Equals("Legacy", StringComparison.OrdinalIgnoreCase) => LegacyPa
                 _ => Parser.Parse(fqn),
             };
 
+            var result = resultEvent.Result;
+            Func<string, string> sanitize = testRun.Serializer.InputSanitizer.Sanitize;
+
             testRun.Store.Add(new TestResultInfo(
-                resultEvent.Result,
-                parsedName.Namespace,
-                parsedName.Type,
-                parsedName.Method));
+                sanitize(parsedName.Namespace),
+                sanitize(parsedName.Type),
+                sanitize(parsedName.Method),
+                result.Outcome,
+                sanitize(result.DisplayName),
+                sanitize(result.TestCase.DisplayName),
+                sanitize(result.TestCase.Source),
+                sanitize(result.TestCase.CodeFilePath),
+                result.TestCase.LineNumber,
+                result.StartTime.UtcDateTime,
+                result.EndTime.UtcDateTime,
+                result.Duration,
+                sanitize(result.ErrorMessage),
+                sanitize(result.ErrorStackTrace),
+                result.Messages.Select(x => new TestResultMessage(sanitize(x.Category), sanitize(x.Text))).ToList(),
+                result.TestCase.Traits.Select(x => new Trait(sanitize(x.Name), sanitize(x.Value))).ToList(),
+                result.TestCase.ExecutorUri?.ToString()));
         }
     }
 }

src/TestLogger/Extensions/MSTestAdapter.cs

@@ -18,7 +18,7 @@ public List<TestResultInfo> TransformResults(List<TestResultInfo> results, List<
             // So we use the DisplayName whenever it is available.
             foreach (var result in results)
             {
-                string displayName = result.Result.DisplayName;
+                string displayName = result.TestResultDisplayName;
                 string method = result.Method;
 
                 if (string.IsNullOrWhiteSpace(displayName))

src/TestLogger/Extensions/NUnitTestAdapter.cs

@@ -21,7 +21,7 @@ public List<TestResultInfo> TransformResults(List<TestResultInfo> results, List<
                 // is passed as a trait in the test platform. NUnit explicit attribute spec:
                 // https://docs.nunit.org/articles/nunit/writing-tests/attributes/explicit.html
                 if (result.Outcome == Microsoft.VisualStudio.TestPlatform.ObjectModel.TestOutcome.None &&
-                    result.TestCase.Traits.Any(trait => trait.Name.Equals(ExplicitLabel, StringComparison.OrdinalIgnoreCase)))
+                    result.Traits.Any(trait => trait.Name.Equals(ExplicitLabel, StringComparison.OrdinalIgnoreCase)))
                 {
                     result.Outcome = Microsoft.VisualStudio.TestPlatform.ObjectModel.TestOutcome.Skipped;
                 }

src/TestLogger/Extensions/XunitTestAdapter.cs

@@ -40,13 +40,13 @@ public List<TestResultInfo> TransformResults(
 
             foreach (var result in results)
             {
-                if (skippedTestNamesWithReason.TryGetValue(result.Result.TestCase.DisplayName, out var skipReason))
+                if (skippedTestNamesWithReason.TryGetValue(result.TestCaseDisplayName, out var skipReason))
                 {
                     // TODO: Defining a new category for now...
                     result.Messages.Add(new TestResultMessage("skipReason", skipReason));
                 }
 
-                string displayName = result.Result.DisplayName;
+                string displayName = result.TestResultDisplayName;
 
                 // Add parameters for theories.
                 if (string.IsNullOrWhiteSpace(displayName) == false &&

src/TestLogger/Utilities/StringExtensions.cs

@@ -7,24 +7,6 @@ namespace Spekt.TestLogger.Utilities
 
      public static class StringExtensions
      {
-         public static string ReplaceInvalidXmlChar(this string str)
-         {
-             if (str != null)
-             {
-                 // From xml spec (http://www.w3.org/TR/xml/#charsets) valid chars:
-                 // #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF]
-                 // Following control charset are discouraged:
-                 // [#x7F-#x84], [#x86-#x9F], [#xFDD0-#xFDEF],
-                 // We are handling only #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD]
-                 // because C# support unicode character in range \u0000 to \uFFFF
-                 const string invalidChar = @"([^\x09\x0A\x0D\x20-\uD7FF\uE000-\uFFFD]|[\u007F-\u0084\u0086-\u009F\uFDD0-\uFDEF])";
-                 MatchEvaluator evaluator = ReplaceInvalidCharacterWithUniCodeEscapeSequence;
-                 return Regex.Replace(str, invalidChar, evaluator);
-             }
-
-             return str;
-         }
-
          public static string SubstringAfterDot(this string name)
          {
              if (string.IsNullOrEmpty(name))
@@ -56,11 +38,5 @@ public static string SubstringBeforeDot(this string name)
 
              return string.Empty;
          }
-
-         private static string ReplaceInvalidCharacterWithUniCodeEscapeSequence(Match match)
-         {
-             char x = match.Value[0];
-             return string.Format(@"\u{0:x4}", (ushort)x);
-         }
      }
 }