fix: make attachment description optional for nunit test logger. (#87)

* fix: make attachment description optional for nunit test logger. * fix: report unexpected exceptions on console.
spekt · Jun 26, 2024 · 73e23a0 · 73e23a0
1 parent a4a1ee3
commit 73e23a0
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 17 deletions.
diff --git a/src/NUnit.Xml.TestLogger/NUnitXmlSerializer.cs b/src/NUnit.Xml.TestLogger/NUnitXmlSerializer.cs
@@ -20,7 +20,7 @@ public class NUnitXmlSerializer : ITestResultSerializer
         private const string ResultStatusPassed = "Passed";
         private const string ResultStatusFailed = "Failed";
 
-        public IInputSanitizer InputSanitizer { get;  } = new InputSanitizerXml();
+        public IInputSanitizer InputSanitizer { get; } = new InputSanitizerXml();
 
         public static IEnumerable<TestSuite> GroupTestSuites(IEnumerable<TestSuite> suites)
         {
@@ -285,10 +285,15 @@ private static XElement CreateTestCaseElement(TestResultInfo result)
                 var attachmentElement = new XElement("attachments");
                 foreach (var attachment in result.Attachments)
                 {
-                    attachmentElement.Add(new XElement(
-                                "attachment",
-                                new XElement("filePath", attachment.FilePath),
-                                new XElement("description", new XCData(attachment.Description))));
+                    var attachmentContent = new List<XElement> { new XElement("filePath", attachment.FilePath) };
+
+                    if (!string.IsNullOrEmpty(attachment.Description))
+                    {
+                        // Description is optional, add if available
+                        attachmentContent.Add(new XElement("description", new XCData(attachment.Description)));
+                    }
+
+                    attachmentElement.Add(new XElement("attachment", attachmentContent));
                 }
 
                 element.Add(attachmentElement);

diff --git a/src/TestLogger/Core/TestRunBuilder.cs b/src/TestLogger/Core/TestRunBuilder.cs
@@ -50,9 +50,9 @@ public ITestRunBuilder Subscribe(TestLoggerEvents loggerEvents)
             {
                 this.testRun.RunConfiguration = this.testRun.Start(eventArgs);
             };
-            loggerEvents.TestRunMessage += (_, eventArgs) => this.testRun.Message(eventArgs);
-            loggerEvents.TestResult += (_, eventArgs) => this.testRun.Result(eventArgs);
-            loggerEvents.TestRunComplete += (_, eventArgs) => this.testRun.Complete(eventArgs);
+            loggerEvents.TestRunMessage += (_, eventArgs) => this.TraceAndThrow(() => this.testRun.Message(eventArgs), "TestRunMessage");
+            loggerEvents.TestResult += (_, eventArgs) => this.TraceAndThrow(() => this.testRun.Result(eventArgs), "TestResult");
+            loggerEvents.TestRunComplete += (_, eventArgs) => this.TraceAndThrow(() => this.testRun.Complete(eventArgs), "TestRunComplete");
 
             return this;
         }
@@ -73,5 +73,18 @@ public ITestRun Build()
         {
             return this.testRun;
         }
+
+        private void TraceAndThrow(Action action, string source)
+        {
+            try
+            {
+                action();
+            }
+            catch (Exception)
+            {
+                this.testRun.ConsoleOutput?.WriteError($"Test Logger: Unexpected error in {source} workflow. Please rerun with `dotnet test --diag:log.txt` to see the stacktrace and report the issue at https://github.com/spekt/testlogger/issues/new.");
+                throw;
+            }
+        }
     }
 }
diff --git a/test/NUnit.Xml.TestLogger.AcceptanceTests/NUnitTestLoggerAcceptanceTests.cs b/test/NUnit.Xml.TestLogger.AcceptanceTests/NUnitTestLoggerAcceptanceTests.cs
@@ -16,8 +16,8 @@ namespace NUnit.Xml.TestLogger.AcceptanceTests
     public class NUnitTestLoggerAcceptanceTests
     {
         private const string AssetName = "NUnit.Xml.TestLogger.NetCore.Tests";
-        private const string ExpectedTestCaseCount = "54";
-        private const string ExpectedTestCasePassedCount = "26";
+        private const string ExpectedTestCaseCount = "55";
+        private const string ExpectedTestCasePassedCount = "27";
 
         private readonly string resultsFile;
         private readonly XDocument resultsXml;
@@ -99,8 +99,8 @@ public void TestResultFileShouldContainNamespaceTestSuiteForNetFull()
             var node = this.resultsXml.XPathSelectElement(query);
 
             Assert.IsNotNull(node);
-            Assert.AreEqual("30", node.Attribute(XName.Get("total")).Value);
-            Assert.AreEqual("16", node.Attribute(XName.Get("passed")).Value);
+            Assert.AreEqual("31", node.Attribute(XName.Get("total")).Value);
+            Assert.AreEqual("17", node.Attribute(XName.Get("passed")).Value);
             Assert.AreEqual("7", node.Attribute(XName.Get("failed")).Value);
             Assert.AreEqual("3", node.Attribute(XName.Get("inconclusive")).Value);
             Assert.AreEqual("4", node.Attribute(XName.Get("skipped")).Value);

diff --git a/test/TestLogger.UnitTests/TestDoubles/FakeConsoleOutput.cs b/test/TestLogger.UnitTests/TestDoubles/FakeConsoleOutput.cs
@@ -8,18 +8,18 @@ namespace Spekt.TestLogger.UnitTests.TestDoubles
 
     public class FakeConsoleOutput : IConsoleOutput
     {
-        private readonly List<(string, string)> messages;
+        public FakeConsoleOutput() => this.Messages = new List<(string, string)>();
 
-        public FakeConsoleOutput() => this.messages = new List<(string, string)>();
+        public List<(string, string)> Messages { get; private set; }
 
         public void WriteMessage(string message)
         {
-            this.messages.Add(("stdout", message));
+            this.Messages.Add(("stdout", message));
         }
 
         public void WriteError(string message)
         {
-            this.messages.Add(("stderr", message));
+            this.Messages.Add(("stderr", message));
         }
     }
 }
diff --git a/test/TestLogger.UnitTests/TestRunBuilderTests.cs b/test/TestLogger.UnitTests/TestRunBuilderTests.cs
@@ -4,9 +4,14 @@
 namespace Spekt.TestLogger.UnitTests
 {
     using System;
+    using System.Collections.Generic;
+    using System.Linq;
     using Microsoft.VisualStudio.TestPlatform.ObjectModel;
+    using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client;
+    using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging;
     using Microsoft.VisualStudio.TestTools.UnitTesting;
     using Spekt.TestLogger.Core;
+    using Spekt.TestLogger.UnitTests.TestDoubles;
 
     [TestClass]
     public class TestRunBuilderTests
@@ -37,7 +42,7 @@ public void TestRunBuilderShouldCreateDefaultTestAdapterFactory()
         [TestMethod]
         public void WithLoggerConfigurationShouldSetTestLoggerConfiguration()
         {
-            var config = new LoggerConfiguration(new ()
+            var config = new LoggerConfiguration(new Dictionary<string, string>()
             {
                 { LoggerConfiguration.LogFilePathKey, "/tmp/results.json" },
                 { DefaultLoggerParameterNames.TestRunDirectory, "/tmp" }
@@ -66,6 +71,24 @@ public void SubscribeShouldThrowForNullLoggerEvents()
             Assert.ThrowsException<ArgumentNullException>(() => this.testRunBuilder.Subscribe(null));
         }
 
+        [TestMethod]
+        public void SubscribeShouldSetupTraceAndThrowExceptionForEvents()
+        {
+            var testEvents = new MockTestLoggerEvents();
+            var consoleOutput = new FakeConsoleOutput();
+
+            this.testRunBuilder.WithConsoleOutput(consoleOutput).Subscribe(testEvents);
+
+            Assert.ThrowsException<NullReferenceException>(() => testEvents.RaiseTestRunMessage(TestMessageLevel.Error, "dummy message"));
+            Assert.ThrowsException<NullReferenceException>(() => testEvents.RaiseTestResult(new Microsoft.VisualStudio.TestPlatform.ObjectModel.TestResult(new TestCase())));
+            Assert.ThrowsException<NullReferenceException>(() => testEvents.RaiseTestRunComplete(null));
+            Assert.AreEqual(3, consoleOutput.Messages.Count);
+            Assert.IsTrue(consoleOutput.Messages.All(x => x.Item1 == "stderr"));
+            StringAssert.Contains(consoleOutput.Messages[0].Item2, "Unexpected error in TestRunMessage workflow");
+            StringAssert.Contains(consoleOutput.Messages[1].Item2, "Unexpected error in TestResult workflow");
+            StringAssert.Contains(consoleOutput.Messages[2].Item2, "Unexpected error in TestRunComplete workflow");
+        }
+
         [TestMethod]
         public void WithFileSystemShouldThrowForNullFileSystem()
         {

diff --git a/test/assets/NUnit.Xml.TestLogger.NetCore.Tests/AttachmentTest.cs b/test/assets/NUnit.Xml.TestLogger.NetCore.Tests/AttachmentTest.cs
@@ -18,5 +18,16 @@ public void TestAddAttachment()
             }
             TestContext.AddTestAttachment(file, "description");
         }
+
+        [Test]
+        public void TestAddAttachmentWithoutDescription()
+        {
+            var file = Path.Combine(Path.GetTempPath(), "x.txt");
+            if (!File.Exists(file))
+            {
+                File.Create(file);
+            }
+            TestContext.AddTestAttachment(file);
+        }
     }
 }