Xsd format fixes misc features

README.md

@@ -19,7 +19,7 @@ If you're looking for `Nunit`, `Xunit` or `appveyor` loggers, visit following re
 
 ## Usage
 
-JUnit logger can generate xml reports in the JUnit format. The JUnit format seems to be variable across different implementations, with the JUnit 5 repository referring to the [Ant Junit Format](https://github.com/windyroad/JUnit-Schema) as a de-facto standard. This project does not implement that standard exactly. Please [refer to a sample file](docs/assets/TestResults.xml) to see an example of which parts of the format have been implemented. If you find that this format is missing some feature required by your CI/CD system, please open an issue.
+The JUnit Test Logger generates xml reports in the [Ant Junit Format](https://github.com/windyroad/JUnit-Schema), which the JUnit 5 repository refers to as the de-facto standard. While the generated xml complies with that schema, it does not contain values in every case. For example, the logger currently does not log any `properties`. Please [refer to a sample file](docs/assets/TestResults.xml) to see an example. If you find that the format is missing data required by your CI/CD system, please open an issue or PR.
 
 ### Default Behavior
 
@@ -93,6 +93,15 @@ When set to default, the body will contain only the exception which is captured
 - FailureBodyFormat=Default
 - FailureBodyFormat=Verbose
 
+#### FileEncoding
+
+When set to default, file encoding will be UTF-8 with BOM. Use this option if you need plain UTF-8 encoding.
+
+##### Allowed Values
+
+- FileEncoding=Utf8
+- FileEncoding=Utf8Bom  (This is the default, and does not need to be specified explicitly.)
+
 ### Saving Multiple Result Files In One Directory
 
 By default, every test project generates an xml report with the same directory and file name. The tokens {framework} and {assembly} may be placed anywhere in the directory or file names to customize the output location. This is **critical**, when multiple test reports will be written to the same directory, as in the following example. Otherwise, the files would use identical names, and the second output file written would overwrite the first.

src/JUnit.Xml.TestLogger/JUnitXmlTestLogger.cs

@@ -36,12 +36,11 @@ public class JUnitXmlTestLogger : ITestLoggerWithParameters
         public const string ResultDirectoryKey = "TestRunDirectory";
         public const string MethodFormatKey = "MethodFormat";
         public const string FailureBodyFormatKey = "FailureBodyFormat";
+        public const string FileEncodingKey = "FileEncoding";
 
         private const string ResultStatusPassed = "Passed";
         private const string ResultStatusFailed = "Failed";
 
-        private const string DateFormat = "yyyy-MM-ddT HH:mm:ssZ";
-
         // Tokens to allow user to manipulate output file or directory names.
         private const string AssemblyToken = "{assembly}";
         private const string FrameworkToken = "{framework}";
@@ -50,7 +49,7 @@ public class JUnitXmlTestLogger : ITestLoggerWithParameters
         private string outputFilePath;
 
         private List<TestResultInfo> results;
-        private DateTime localStartTime;
+        private DateTime utcStartTime;
 
         public enum MethodFormat
         {
@@ -83,26 +82,41 @@ public enum FailureBodyFormat
             Verbose
         }
 
+        public enum FileEncoding
+        {
+            /// <summary>
+            /// UTF8
+            /// </summary>
+            UTF8,
+
+            /// <summary>
+            /// UTF8 Bom
+            /// </summary>
+            UTF8Bom
+        }
+
         public MethodFormat MethodFormatOption { get; private set; } = MethodFormat.Default;
 
         public FailureBodyFormat FailureBodyFormatOption { get; private set; } = FailureBodyFormat.Default;
 
+        public FileEncoding FileEncodingOption { get; private set; } = FileEncoding.UTF8Bom;
+
         public static IEnumerable<TestSuite> GroupTestSuites(IEnumerable<TestSuite> suites)
         {
             var groups = suites;
             var roots = new List<TestSuite>();
             while (groups.Any())
             {
                 groups = groups.GroupBy(r =>
-                                {
-                                    var name = r.FullName.SubstringBeforeDot();
-                                    if (string.IsNullOrEmpty(name))
-                                    {
-                                        roots.Add(r);
-                                    }
-
-                                    return name;
-                                })
+                {
+                    var name = r.FullName.SubstringBeforeDot();
+                    if (string.IsNullOrEmpty(name))
+                    {
+                        roots.Add(r);
+                    }
+
+                    return name;
+                })
                                 .OrderBy(g => g.Key)
                                 .Where(g => !string.IsNullOrEmpty(g.Key))
                                 .Select(g => AggregateTestSuites(g, "TestSuite", g.Key.SubstringAfterDot(), g.Key))
@@ -208,6 +222,22 @@ public void Initialize(TestLoggerEvents events, Dictionary<string, string> param
                     Console.WriteLine($"JunitXML Logger: The provided Failure Body Format '{failureFormat}' is not a recognized option. Using default");
                 }
             }
+
+            if (parameters.TryGetValue(FileEncodingKey, out string fileEncoding))
+            {
+                if (string.Equals(fileEncoding.Trim(), "UTF8Bom", StringComparison.OrdinalIgnoreCase))
+                {
+                    this.FileEncodingOption = FileEncoding.UTF8Bom;
+                }
+                else if (string.Equals(fileEncoding.Trim(), "UTF8", StringComparison.OrdinalIgnoreCase))
+                {
+                    this.FileEncodingOption = FileEncoding.UTF8;
+                }
+                else
+                {
+                    Console.WriteLine($"JunitXML Logger: The provided File Encoding '{failureFormat}' is not a recognized option. Using default");
+                }
+            }
         }
 
         /// <summary>
@@ -282,9 +312,18 @@ internal void TestRunCompleteHandler(object sender, TestRunCompleteEventArgs e)
                     Directory.CreateDirectory(loggerFileDirPath);
                 }
 
+                var settings = new XmlWriterSettings()
+                {
+                    Encoding = new UTF8Encoding(this.FileEncodingOption == FileEncoding.UTF8 ? false : true),
+                    Indent = true,
+                };
+
                 using (var f = File.Create(this.outputFilePath))
                 {
-                    doc.Save(f);
+                    using (var w = XmlWriter.Create(f, settings))
+                    {
+                        doc.Save(w);
+                    }
                 }
 
                 var resultsFileMessage = string.Format(CultureInfo.CurrentCulture, "JunitXML Logger - Results File: {0}", this.outputFilePath);
@@ -370,7 +409,7 @@ private void InitializeImpl(TestLoggerEvents events, string outputPath)
                 this.results = new List<TestResultInfo>();
             }
 
-            this.localStartTime = DateTime.UtcNow;
+            this.utcStartTime = DateTime.UtcNow;
         }
 
         private XElement CreateTestSuitesElement(List<TestResultInfo> results)
@@ -381,20 +420,21 @@ private XElement CreateTestSuitesElement(List<TestResultInfo> results)
 
             var element = new XElement("testsuites", testsuiteElements);
 
-            element.SetAttributeValue("name", Path.GetFileName(results.First().AssemblyPath));
-
-            element.SetAttributeValue("tests", results.Count);
-            element.SetAttributeValue("failures", results.Where(x => x.Outcome == TestOutcome.Failed).Count());
-            element.SetAttributeValue("time", results.Sum(x => x.Duration.TotalSeconds));
-
             return element;
         }
 
         private XElement CreateTestSuiteElement(List<TestResultInfo> results)
         {
             var testCaseElements = results.Select(a => this.CreateTestCaseElement(a));
 
-            var element = new XElement("testsuite", testCaseElements);
+            // Adding required properties, system-out, and system-err elements in the correct
+            // positions as required by the xsd.
+            var element = new XElement(
+                "testsuite",
+                new XElement("properties"),
+                testCaseElements,
+                new XElement("system-out", "Junit Logger does not log standard output"),
+                new XElement("system-err", "Junit Logger does not log error output"));
 
             element.SetAttributeValue("name", Path.GetFileName(results.First().AssemblyPath));
 
@@ -403,8 +443,10 @@ private XElement CreateTestSuiteElement(List<TestResultInfo> results)
             element.SetAttributeValue("failures", results.Where(x => x.Outcome == TestOutcome.Failed).Count());
             element.SetAttributeValue("errors", 0); // looks like this isn't supported by .net?
             element.SetAttributeValue("time", results.Sum(x => x.Duration.TotalSeconds));
-            element.SetAttributeValue("timestamp", this.localStartTime.ToString(DateFormat, CultureInfo.InvariantCulture));
+            element.SetAttributeValue("timestamp", this.utcStartTime.ToString("s"));
             element.SetAttributeValue("hostname", results.First().TestCase.ExecutorUri);
+            element.SetAttributeValue("id", 0); // we never output multiple, so this is always zero.
+            element.SetAttributeValue("package", Path.GetFileName(results.First().AssemblyPath));
 
             return element;
         }
@@ -430,8 +472,12 @@ private XElement CreateTestCaseElement(TestResultInfo result)
                 testcaseElement.SetAttributeValue("name", result.Name);
             }
 
-            testcaseElement.SetAttributeValue("file", result.TestCase.Source);
-            testcaseElement.SetAttributeValue("time", result.Duration.TotalSeconds);
+            // Ensure time value is never zero because gitlab treats 0 like its null.
+            // 0.1 micro seconds should be low enough it won't interfere with anyone
+            // monitoring test duration.
+            testcaseElement.SetAttributeValue(
+                "time",
+                Math.Max(0.0000001f, result.Duration.TotalSeconds).ToString("0.0000000"));
 
             if (result.Outcome == TestOutcome.Failed)
             {
@@ -447,13 +493,19 @@ private XElement CreateTestCaseElement(TestResultInfo result)
 
                 failureBodySB.AppendLine(result.ErrorStackTrace);
 
-                var failureElement = new XElement("failure", failureBodySB.ToString());
+                var failureElement = new XElement("failure", failureBodySB.ToString().Trim());
 
                 failureElement.SetAttributeValue("type", "failure"); // TODO are there failure types?
                 failureElement.SetAttributeValue("message", result.ErrorMessage);
 
                 testcaseElement.Add(failureElement);
             }
+            else if (result.Outcome == TestOutcome.Skipped)
+            {
+                var skippedElement = new XElement("skipped");
+
+                testcaseElement.Add(skippedElement);
+            }
 
             return testcaseElement;
         }

test/JUnit.Xml.TestLogger.AcceptanceTests/JUnitTestLoggerAcceptanceTests.cs

@@ -46,11 +46,6 @@ public void TestResultFileShouldContainTestSuitesInformation()
             var node = this.resultsXml.XPathSelectElement("/testsuites");
 
             Assert.IsNotNull(node);
-            Assert.AreEqual("JUnit.Xml.TestLogger.NetCore.Tests.dll", node.Attribute(XName.Get("name")).Value);
-            Assert.AreEqual("52", node.Attribute(XName.Get("tests")).Value);
-            Assert.AreEqual("14", node.Attribute(XName.Get("failures")).Value);
-
-            Convert.ToDouble(node.Attribute(XName.Get("time")).Value);
         }
 
         [TestMethod]
@@ -84,13 +79,28 @@ public void TestResultFileShouldContainTestCases()
             Assert.IsTrue(testcases.All(x => double.TryParse(x.Attribute("time").Value, out _)));
 
             // Check failures
-            Assert.AreEqual(14, testcases.Where(x => x.Descendants().Any()).Count());
-            Assert.IsTrue(testcases.Where(x => x.Descendants().Any())
-                                   .All(x => x.Descendants().Count() == 1));
-            Assert.IsTrue(testcases.Where(x => x.Descendants().Any())
-                                   .All(x => x.Descendants().First().Name.LocalName == "failure"));
-            Assert.IsTrue(testcases.Where(x => x.Descendants().Any())
-                                   .All(x => x.Descendants().First().Attribute("type").Value == "failure"));
+            var failures = testcases
+                .Where(x => x.Descendants().Any(y => y.Name.LocalName == "failure"))
+                .ToList();
+
+            Assert.AreEqual(14, failures.Count());
+            Assert.IsTrue(failures.All(x => x.Descendants().Count() == 1));
+            Assert.IsTrue(failures.All(x => x.Descendants().First().Attribute("type").Value == "failure"));
+
+            // Check failures
+            var skips = testcases
+                .Where(x => x.Descendants().Any(y => y.Name.LocalName == "skipped"))
+                .ToList();
+
+            Assert.AreEqual(6, skips.Count());
+        }
+
+        [TestMethod]
+        public void LoggedXmlValidatesAgainstXsdSchema()
+        {
+            var validator = new JunitXmlValidator();
+            var result = validator.IsValid(File.ReadAllText(this.resultsFile));
+            Assert.IsTrue(result);
         }
     }
 }

test/JUnit.Xml.TestLogger.AcceptanceTests/JUnitTestLoggerFormatOptionsAcceptanceTests.cs

@@ -61,7 +61,7 @@ public void FailureBodyFormat_Verbose_ShouldStartWithMessage()
                 var message = failure.Attribute("message").Value.Replace("\r", string.Empty).Replace("\n", string.Empty);
                 var body = failure.Value.Replace("\r", string.Empty).Replace("\n", string.Empty);
 
-                Assert.IsTrue(body.StartsWith(message));
+                Assert.IsTrue(body.Trim().StartsWith(message.Trim()));
             }
         }
 

test/JUnit.Xml.TestLogger.AcceptanceTests/JunitXmlValidator.cs

@@ -0,0 +1,58 @@
+// Copyright (c) Spekt Contributors. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+namespace JUnit.Xml.TestLogger.AcceptanceTests
+{
+    using System;
+    using System.Collections.Generic;
+    using System.IO;
+    using System.Linq;
+    using System.Text;
+    using System.Threading.Tasks;
+    using System.Xml;
+    using System.Xml.Linq;
+    using System.Xml.Schema;
+
+    public class JunitXmlValidator
+    {
+        /// <summary>
+        /// Field is provided only to simplify debugging test failures.
+        /// </summary>
+        private readonly List<XmlSchemaException> failures = new List<XmlSchemaException>();
+
+        public bool IsValid(string xml)
+        {
+            var xmlReader = new StringReader(xml);
+            var xsdReader = new StringReader(
+                File.ReadAllText(
+                    Path.Combine("..", "..", "..", "..", "assets", "JUnit.xsd")));
+
+            var schema = XmlSchema.Read(
+                xsdReader,
+                (sender, args) => { throw new XmlSchemaValidationException(args.Message, args.Exception); });
+
+            var xmlReaderSettings = new XmlReaderSettings();
+            xmlReaderSettings.Schemas.Add(schema);
+            xmlReaderSettings.ValidationType = ValidationType.Schema;
+
+            var veh = new ValidationEventHandler(this.XmlValidationEventHandler);
+
+            xmlReaderSettings.ValidationEventHandler += veh;
+            using (XmlReader reader = XmlReader.Create(xmlReader, xmlReaderSettings))
+            {
+                while (reader.Read())
+                {
+                }
+            }
+
+            xmlReaderSettings.ValidationEventHandler -= veh;
+
+            return this.failures.Any() == false;
+        }
+
+        private void XmlValidationEventHandler(object sender, ValidationEventArgs e)
+        {
+            this.failures.Add(e.Exception);
+        }
+    }
+}