fix: escape name for control chars in generated xml (#30)

* Use the latest Spekt.TestLogger drop with common xml utilities
* Document logger options supported with core logger

Fixes #25

Author: codito

CHANGELOG.md

@@ -2,8 +2,11 @@
 
 ## Unreleased (v3.0.x)
 
-* Refactor to support [core testlogger][]
-* Compatibility: minimum framework is netstandard1.5 and TestPlatform 15.5.0
-* Use test run start and end times for run duration reporting for assembly. See #26
+- Refactor to support [core testlogger][]
+- Compatibility: minimum framework is netstandard1.5 and TestPlatform 15.5.0
+- Use test run start and end times for run duration reporting for assembly. See #26
+- Escape control characters from the generated xml. See #25
+- Token expansion for `{assembly}` and `{framework}` in results file. See
+  https://github.com/spekt/testlogger/wiki/Logger-Configuration
 
 [core testlogger]: https://github.com/spekt/testlogger

README.md

@@ -34,5 +34,9 @@ A path for the report file can be specified as follows:
 
 **Note:** the arguments to `--logger` should be in quotes since `;` is treated as a command delimiter in shell.
 
+All common options to the logger is documented [in the wiki][config-wiki].
+
+[config-wiki]: https://github.com/spekt/testlogger/wiki/Logger-Configuration
+
 ## License
 MIT

scripts/dependencies.props

@@ -4,7 +4,7 @@
     <MSTestVersion>1.3.2</MSTestVersion>
     <NETTestSdkVersion>15.7.2</NETTestSdkVersion>
     <MoqVersion>4.9.0</MoqVersion>
-    <TestLoggerVersion>3.0.26</TestLoggerVersion>
+    <TestLoggerVersion>3.0.28</TestLoggerVersion>
 
     <!-- Test Assets use the minimum supported versions -->
     <NETTestSdkMinimumVersion>15.5.0</NETTestSdkMinimumVersion>

src/Xunit.Xml.TestLogger/XunitXmlSerializer.cs

@@ -9,10 +9,10 @@ namespace Microsoft.VisualStudio.TestPlatform.Extension.Xunit.Xml.TestLogger
     using System.IO;
     using System.Linq;
     using System.Text;
-    using System.Text.RegularExpressions;
     using System.Xml.Linq;
     using Microsoft.VisualStudio.TestPlatform.ObjectModel;
     using Spekt.TestLogger.Core;
+    using Spekt.TestLogger.Utilities;
 
     public class XunitXmlSerializer : ITestResultSerializer
     {
@@ -166,8 +166,8 @@ private static XElement CreateErrorElement(TestResultInfo result)
         private static XElement CreateFailureElement(string exceptionType, string message, string stackTrace)
         {
             XElement failureElement = new XElement("failure", new XAttribute("exception-type", exceptionType));
-            failureElement.Add(new XElement("message", RemoveInvalidXmlChar(message)));
-            failureElement.Add(new XElement("stack-trace", RemoveInvalidXmlChar(stackTrace)));
+            failureElement.Add(new XElement("message", message.ReplaceInvalidXmlChar()));
+            failureElement.Add(new XElement("stack-trace", stackTrace.ReplaceInvalidXmlChar()));
 
             return failureElement;
         }
@@ -248,7 +248,7 @@ private static XElement CreateTestElement(TestResultInfo result)
         {
             var element = new XElement(
                 "test",
-                new XAttribute("name", result.Name),
+                new XAttribute("name", result.Name.ReplaceInvalidXmlChar()),
                 new XAttribute("type", result.FullTypeName),
                 new XAttribute("method", result.Method),
                 new XAttribute("time", result.Duration.TotalSeconds.ToString("F7", CultureInfo.InvariantCulture)),
@@ -264,13 +264,13 @@ private static XElement CreateTestElement(TestResultInfo result)
                 else if (m.Category == "skipReason")
                 {
                     // Using the self-defined category skipReason for now
-                    element.Add(new XElement("reason", new XCData(RemoveInvalidXmlChar(m.Text))));
+                    element.Add(new XElement("reason", new XCData(m.Text.ReplaceInvalidXmlChar())));
                 }
             }
 
             if (!string.IsNullOrWhiteSpace(stdOut.ToString()))
             {
-                element.Add(new XElement("output", RemoveInvalidXmlChar(stdOut.ToString())));
+                element.Add(new XElement("output", stdOut.ToString().ReplaceInvalidXmlChar()));
             }
 
             var fileName = result.TestCase.CodeFilePath;
@@ -284,8 +284,8 @@ private static XElement CreateTestElement(TestResultInfo result)
             {
                 element.Add(new XElement(
                     "failure",
-                    new XElement("message", RemoveInvalidXmlChar(result.ErrorMessage)),
-                    new XElement("stack-trace", RemoveInvalidXmlChar(result.ErrorStackTrace))));
+                    new XElement("message", result.ErrorMessage.ReplaceInvalidXmlChar()),
+                    new XElement("stack-trace", result.ErrorStackTrace.ReplaceInvalidXmlChar())));
             }
 
             if (result.Traits != null)
@@ -315,27 +315,5 @@ private static string OutcomeToString(TestOutcome outcome)
                     return "Unknown";
             }
         }
-
-        private static string RemoveInvalidXmlChar(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]
-
-                // we are handling only #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD]
-                // because C# support unicode character in range \u0000 to \uFFFF
-                MatchEvaluator evaluator = new MatchEvaluator(ReplaceInvalidCharacterWithUniCodeEscapeSequence);
-                string invalidChar = @"[^\x09\x0A\x0D\x20-\uD7FF\uE000-\uFFFD]";
-                return Regex.Replace(str, invalidChar, evaluator);
-            }
-
-            return str;
-        }
-
-        private static string ReplaceInvalidCharacterWithUniCodeEscapeSequence(Match match)
-        {
-            return string.Format(@"\u{0:x4}", (ushort)match.Value[0]);
-        }
     }
 }

test/Xunit.Xml.TestLogger.AcceptanceTests/TestResultsXmlTests.cs

@@ -4,6 +4,7 @@
 namespace Xunit.Xml.TestLogger.AcceptanceTests
 {
     using System;
+    using System.Collections.Generic;
     using System.Globalization;
     using System.IO;
     using System.Linq;
@@ -133,15 +134,15 @@ public void AssemblyElementTotalAttributeShouldValueEqualToNumberOfTotalTests()
         {
             XmlNode assemblyNode = this.testResultsXmlDocument.SelectSingleNode(TestResultsXmlTests.AssemblyElement);
 
-            Assert.Equal("5", assemblyNode.Attributes["total"].Value);
+            Assert.Equal("6", assemblyNode.Attributes["total"].Value);
         }
 
         [Fact]
         public void AssemblyElementPassedAttributeShouldValueEqualToNumberOfPassedTests()
         {
             XmlNode assemblyNode = this.testResultsXmlDocument.SelectSingleNode(TestResultsXmlTests.AssemblyElement);
 
-            Assert.Equal("2", assemblyNode.Attributes["passed"].Value);
+            Assert.Equal("3", assemblyNode.Attributes["passed"].Value);
         }
 
         [Fact]
@@ -192,7 +193,7 @@ public void CollectionElementsCountShouldBeTwo()
         {
             XmlNodeList collectionElementNodeList = this.testResultsXmlDocument.SelectNodes(TestResultsXmlTests.CollectionElement);
 
-            Assert.True(collectionElementNodeList.Count == 2);
+            Assert.Equal(3, collectionElementNodeList.Count);
         }
 
         [Fact]
@@ -237,13 +238,25 @@ public void CollectionElementTimeAttributeShouldHaveValidFormatValue()
         }
 
         [Fact]
-        public void CollectionElementShouldContainThreeTestsElemets()
+        public void CollectionElementShouldContainThreeTestsElements()
         {
             XmlNode unitTest1Collection = this.GetUnitTest1Collection();
 
             Assert.True(unitTest1Collection.SelectNodes("test").Count == 3);
         }
 
+        [Fact]
+        public void TestElementNameAttributeShouldBeEscaped()
+        {
+            var testNodes = this.GetTestXmlNodePartial(
+                "UnitTest3",
+                @"Xunit.Xml.TestLogger.NetCore.Tests.UnitTest3.TestInvalidName");
+
+            Assert.Equal(
+                "Xunit.Xml.TestLogger.NetCore.Tests.UnitTest3.TestInvalidName(input: \"Head\\u0080r\")",
+                testNodes.Item(0).Attributes["name"].Value);
+        }
+
         [Fact]
         public void TestElementTypeAttributeShouldHaveCorrectValue()
         {
@@ -290,7 +303,9 @@ public void FailedTestElementResultAttributeShouldHaveValueFail()
         [Fact]
         public void PassedTestElementResultAttributeShouldHaveValuePass()
         {
-            XmlNode passedTestXmlNode = this.GetATestXmlNode("Xunit.Xml.TestLogger.NetCore.Tests.UnitTest1.PassTest11");
+            XmlNode passedTestXmlNode = this.GetATestXmlNode(
+                "UnitTest1",
+                "Xunit.Xml.TestLogger.NetCore.Tests.UnitTest1.PassTest11");
 
             Assert.Equal("Pass", passedTestXmlNode.Attributes["result"].Value);
         }
@@ -317,7 +332,9 @@ public void FailedTestElementShouldContainsFailureDetails()
         [Fact]
         public void SkippedTestElementShouldContainSkippingReason()
         {
-            XmlNode skippedTestNode = this.GetATestXmlNode("Xunit.Xml.TestLogger.NetCore.Tests.UnitTest1.SkipTest11");
+            XmlNode skippedTestNode = this.GetATestXmlNode(
+                "UnitTest1",
+                "Xunit.Xml.TestLogger.NetCore.Tests.UnitTest1.SkipTest11");
             var reasonNodes = skippedTestNode.SelectNodes("reason");
 
             Assert.Equal(1, reasonNodes.Count);
@@ -331,27 +348,38 @@ public void SkippedTestElementShouldContainSkippingReason()
             Assert.Equal(expectedReason, reasonData.Value);
         }
 
-        private XmlNode GetATestXmlNode(string queryTestName = "Xunit.Xml.TestLogger.NetCore.Tests.UnitTest1.FailTest11")
+        private XmlNode GetATestXmlNode(
+            string collectionName = "UnitTest1",
+            string queryTestName = "Xunit.Xml.TestLogger.NetCore.Tests.UnitTest1.FailTest11")
         {
-            XmlNode unitTest1Collection = this.GetUnitTest1Collection();
+            var unitTest1Collection = this.GetUnitTestCollection(collectionName);
 
             var testNodes = unitTest1Collection.SelectNodes($"test[@name=\"{queryTestName}\"]");
             return testNodes.Item(0);
         }
 
-        private XmlNode GetUnitTest1Collection()
+        private XmlNodeList GetTestXmlNodePartial(
+            string collectionName,
+            string testName)
         {
-            XmlNodeList collectionElementNodeList =
-                this.testResultsXmlDocument.SelectNodes(TestResultsXmlTests.CollectionElement);
+            var unitTest1Collection = this.GetUnitTestCollection(collectionName);
 
-            Assert.True(collectionElementNodeList.Count == 2);
+            var testNodes = unitTest1Collection.SelectNodes($"test[contains(@name, \"{testName}\")]");
+            return testNodes;
+        }
 
-            XmlNode unitTest1Collection = null;
-            var firstCollectionName = collectionElementNodeList[0].Attributes["name"];
-            unitTest1Collection = "Test collection for Xunit.Xml.TestLogger.NetCore.Tests.UnitTest1".Equals(firstCollectionName.Value)
-                ? collectionElementNodeList[0] : collectionElementNodeList[1];
+        private XmlNode GetUnitTestCollection(string name)
+        {
+            var testNodes = this.testResultsXmlDocument.SelectNodes(
+                $"//assemblies/assembly/collection[contains(@name, \"{name}\")]");
 
-            return unitTest1Collection;
+            Assert.Equal(1, testNodes.Count);
+            return testNodes.Item(0);
+        }
+
+        private XmlNode GetUnitTest1Collection()
+        {
+            return this.GetUnitTestCollection("UnitTest1");
         }
     }
 }

test/assets/Xunit.Xml.TestLogger.NetCore.Tests/UnitTest1.cs

@@ -36,4 +36,13 @@ public void FailTest22()
             Assert.False(true);
         }
     }
+
+    public class UnitTest3
+    {
+        [Theory]
+        [InlineData("Head\x80r")]    // See issue #25
+        public void TestInvalidName(string input)
+        {
+        }
+    }
 }