From 41885ae1c6e63e0da42769f10d8acf6b20206ad4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Terje=20Sandstr=C3=B8m?= <terje@hermit.no>
Date: Sat, 29 May 2021 17:23:34 +0200
Subject: [PATCH] Fix for issue #671, exception stacktrace from OneTimeSetup

---
 .runsettings                                  | 16 ++---
 src/NUnitTestAdapter/AdapterSettings.cs       |  4 +-
 .../NUnitTestEventSuiteFinished.cs            |  6 +-
 src/NUnitTestAdapter/NUnitEventListener.cs    | 63 ++++++++++---------
 .../NUnitEngineTests/NUnitTestEventsTests.cs  | 30 +++++++++
 5 files changed, 78 insertions(+), 41 deletions(-)

diff --git a/.runsettings b/.runsettings
index 9bf92652..f028fe7c 100644
--- a/.runsettings
+++ b/.runsettings
@@ -1,10 +1,10 @@
 <?xml version="1.0" encoding="utf-8"?>
 <RunSettings>
-  <RunConfiguration>
-    <!-- 0 = As many processes as possible, limited by number of cores on machine, 1 = Sequential (1 process), 2-> Given number of processes up to limit by number of cores on machine-->
-    <MaxCpuCount>0</MaxCpuCount>
-   
-  </RunConfiguration>
+    <RunConfiguration>
+        <!-- 0 = As many processes as possible, limited by number of cores on machine, 1 = Sequential (1 process), 2-> Given number of processes up to limit by number of cores on machine-->
+        <MaxCpuCount>0</MaxCpuCount>
+
+    </RunConfiguration>
 
     <!--
     <TestRunParameters>
@@ -46,9 +46,9 @@
 
     <NUnit>
         <Verbosity>5</Verbosity>
-      <DumpXmlTestResults>true</DumpXmlTestResults>
-      <DumpXmlTestDiscovery>true</DumpXmlTestDiscovery>
-      <DiscoveryMethod>Current</DiscoveryMethod>
+        <DumpXmlTestResults>true</DumpXmlTestResults>
+        <DumpXmlTestDiscovery>true</DumpXmlTestDiscovery>
+        <DiscoveryMethod>Current</DiscoveryMethod>
     </NUnit>
 
 
diff --git a/src/NUnitTestAdapter/AdapterSettings.cs b/src/NUnitTestAdapter/AdapterSettings.cs
index 1cf84970..05448ec8 100644
--- a/src/NUnitTestAdapter/AdapterSettings.cs
+++ b/src/NUnitTestAdapter/AdapterSettings.cs
@@ -111,6 +111,7 @@ public interface IAdapterSettings
         int AssemblySelectLimit { get; }
 
         bool UseNUnitFilter { get; }
+        bool IncludeStackTraceForSuites { get; }
 
 
         void Load(IDiscoveryContext context);
@@ -236,6 +237,7 @@ public AdapterSettings(ITestLogger logger)
         public bool SkipNonTestAssemblies { get; private set; }
         public int AssemblySelectLimit { get; private set; }
         public bool UseNUnitFilter { get; private set; }
+        public bool IncludeStackTraceForSuites { get; private set; }
 
         public VsTestCategoryType VsTestCategoryType { get; private set; } = VsTestCategoryType.NUnit;
 
@@ -322,7 +324,7 @@ public void Load(string settingsXml)
             ConsoleOut = GetInnerTextAsInt(nunitNode, nameof(ConsoleOut), 1);  // 0 no output to console, 1 : output to console
             StopOnError = GetInnerTextAsBool(nunitNode, nameof(StopOnError), false);
             UseNUnitFilter = GetInnerTextAsBool(nunitNode, nameof(UseNUnitFilter), true);
-
+            IncludeStackTraceForSuites = GetInnerTextAsBool(nunitNode, nameof(IncludeStackTraceForSuites), true);
 
             // Engine settings
             DiscoveryMethod = MapEnum(GetInnerText(nunitNode, nameof(DiscoveryMethod), Verbosity > 0), DiscoveryMethod.Current);
diff --git a/src/NUnitTestAdapter/NUnitEngine/NUnitTestEventSuiteFinished.cs b/src/NUnitTestAdapter/NUnitEngine/NUnitTestEventSuiteFinished.cs
index fff9134f..c51bb8c1 100644
--- a/src/NUnitTestAdapter/NUnitEngine/NUnitTestEventSuiteFinished.cs
+++ b/src/NUnitTestAdapter/NUnitEngine/NUnitTestEventSuiteFinished.cs
@@ -8,6 +8,7 @@ public interface INUnitTestEventSuiteFinished : INUnitTestEvent
         bool HasReason { get; }
         string FailureMessage { get; }
         bool HasFailure { get; }
+        string StackTrace { get; }
     }
 
     /// <summary>
@@ -28,6 +29,7 @@ public NUnitTestEventSuiteFinished(XmlNode node) : base(node)
             if (failureNode != null)
             {
                 FailureMessage = failureNode.SelectSingleNode("message")?.InnerText;
+                StackTrace = failureNode.SelectSingleNode("stack-trace")?.InnerText;
             }
             ReasonMessage = Node.SelectSingleNode("reason/message")?.InnerText;
         }
@@ -35,7 +37,9 @@ public NUnitTestEventSuiteFinished(XmlNode node) : base(node)
         public string ReasonMessage { get; }
 
         public bool HasReason => !string.IsNullOrEmpty(ReasonMessage);
-        public string FailureMessage { get; }
+        public string FailureMessage { get; } = "";
+
+        public string StackTrace { get; } = "";
 
         public bool HasFailure => !string.IsNullOrEmpty(FailureMessage);
     }
diff --git a/src/NUnitTestAdapter/NUnitEventListener.cs b/src/NUnitTestAdapter/NUnitEventListener.cs
index 6c52b3d8..61b2b2ae 100644
--- a/src/NUnitTestAdapter/NUnitEventListener.cs
+++ b/src/NUnitTestAdapter/NUnitEventListener.cs
@@ -47,10 +47,10 @@ public class NUnitEventListener :
         ITestEventListener, IDisposable // Public for testing
     {
         private static readonly ICollection<INUnitTestEventTestOutput> EmptyNodes = new List<INUnitTestEventTestOutput>();
-        private readonly ITestExecutionRecorder _recorder;
-        private readonly ITestConverterCommon _testConverter;
-        private readonly IAdapterSettings _settings;
-        private readonly Dictionary<string, ICollection<INUnitTestEventTestOutput>> _outputNodes = new ();
+        private readonly ITestExecutionRecorder recorder;
+        private readonly ITestConverterCommon testConverter;
+        private readonly IAdapterSettings settings;
+        private readonly Dictionary<string, ICollection<INUnitTestEventTestOutput>> outputNodes = new ();
 
 #if NET35
         public override object InitializeLifetimeService()
@@ -69,9 +69,9 @@ public NUnitEventListener(ITestConverterCommon testConverter, INUnit3TestExecuto
         {
             this.executor = executor;
             dumpXml = executor.Dump;
-            _settings = executor.Settings;
-            _recorder = executor.FrameworkHandle;
-            _testConverter = testConverter;
+            settings = executor.Settings;
+            recorder = executor.FrameworkHandle;
+            this.testConverter = testConverter;
         }
 
         #region ITestEventListener
@@ -103,8 +103,8 @@ public void OnTestEvent(string report)
             }
             catch (Exception ex)
             {
-                _recorder.SendMessage(TestMessageLevel.Warning, $"Error processing {node.Name} event for {node.FullName}");
-                _recorder.SendMessage(TestMessageLevel.Warning, ex.ToString());
+                recorder.SendMessage(TestMessageLevel.Warning, $"Error processing {node.Name} event for {node.FullName}");
+                recorder.SendMessage(TestMessageLevel.Warning, ex.ToString());
             }
         }
 
@@ -141,44 +141,44 @@ protected virtual void Dispose(bool disposing)
 
         public void TestStarted(INUnitTestEventStartTest testNode)
         {
-            var ourCase = _testConverter.GetCachedTestCase(testNode.Id);
+            var ourCase = testConverter.GetCachedTestCase(testNode.Id);
 
             // Simply ignore any TestCase not found in the cache
             if (ourCase != null)
-                _recorder.RecordStart(ourCase);
+                recorder.RecordStart(ourCase);
         }
 
         public void TestFinished(INUnitTestEventTestCase resultNode)
         {
             var testId = resultNode.Id;
-            if (_outputNodes.TryGetValue(testId, out var outputNodes))
+            if (this.outputNodes.TryGetValue(testId, out var outputNodes))
             {
-                _outputNodes.Remove(testId);
+                this.outputNodes.Remove(testId);
             }
 
-            var result = _testConverter.GetVsTestResults(resultNode, outputNodes ?? EmptyNodes);
-            if (_settings.ConsoleOut == 1)
+            var result = testConverter.GetVsTestResults(resultNode, outputNodes ?? EmptyNodes);
+            if (settings.ConsoleOut == 1)
             {
                 if (!string.IsNullOrEmpty(result.ConsoleOutput) && result.ConsoleOutput != NL)
                 {
                     string msg = result.ConsoleOutput;
-                    if (_settings.UseTestNameInConsoleOutput)
+                    if (settings.UseTestNameInConsoleOutput)
                         msg = $"{resultNode.Name}: {msg}";
-                    _recorder.SendMessage(TestMessageLevel.Informational, msg);
+                    recorder.SendMessage(TestMessageLevel.Informational, msg);
                 }
                 if (!string.IsNullOrEmpty(resultNode.ReasonMessage))
                 {
-                    _recorder.SendMessage(TestMessageLevel.Informational,                        $"{resultNode.Name}: {resultNode.ReasonMessage}");
+                    recorder.SendMessage(TestMessageLevel.Informational, $"{resultNode.Name}: {resultNode.ReasonMessage}");
                 }
             }
 
-            _recorder.RecordEnd(result.TestCaseResult.TestCase, result.TestCaseResult.Outcome);
+            recorder.RecordEnd(result.TestCaseResult.TestCase, result.TestCaseResult.Outcome);
             foreach (var vsResult in result.TestResults)
             {
-                _recorder.RecordResult(vsResult);
+                recorder.RecordResult(vsResult);
             }
 
-            if (result.TestCaseResult.Outcome == TestOutcome.Failed && _settings.StopOnError)
+            if (result.TestCaseResult.Outcome == TestOutcome.Failed && settings.StopOnError)
             {
                 executor.StopRun();
             }
@@ -191,15 +191,16 @@ public void SuiteFinished(INUnitTestEventSuiteFinished resultNode)
             var site = resultNode.Site();
             if (site != NUnitTestEvent.SiteType.Setup && site != NUnitTestEvent.SiteType.TearDown)
                 return;
-            _recorder.SendMessage(TestMessageLevel.Warning, $"{site} failed for test fixture {resultNode.FullName}");
+            recorder.SendMessage(TestMessageLevel.Warning, $"{site} failed for test fixture {resultNode.FullName}");
 
             if (resultNode.HasFailure)
-                _recorder.SendMessage(TestMessageLevel.Warning, resultNode.FailureMessage);
-
-            // Should not be any stacktrace on Suite-finished
-            // var stackNode = resultNode.Failure.StackTrace;
-            // if (!string.IsNullOrEmpty(stackNode))
-            //    _recorder.SendMessage(TestMessageLevel.Warning, stackNode);
+            {
+                string msg = resultNode.FailureMessage;
+                var stackNode = resultNode.StackTrace;
+                if (!string.IsNullOrEmpty(stackNode) && settings.IncludeStackTraceForSuites)
+                    msg += $"\nStackTrace: {stackNode}";
+                recorder.SendMessage(TestMessageLevel.Warning, msg);
+            }
         }
 
         private static readonly string NL = Environment.NewLine;
@@ -222,16 +223,16 @@ public void TestOutput(INUnitTestEventTestOutput outputNodeEvent)
             string testId = outputNodeEvent.TestId;
             if (!string.IsNullOrEmpty(testId))
             {
-                if (!_outputNodes.TryGetValue(testId, out var outputNodes))
+                if (!this.outputNodes.TryGetValue(testId, out var outputNodes))
                 {
                     outputNodes = new List<INUnitTestEventTestOutput>();
-                    _outputNodes.Add(testId, outputNodes);
+                    this.outputNodes.Add(testId, outputNodes);
                 }
 
                 outputNodes.Add(outputNodeEvent);
             }
 
-            _recorder.SendMessage(TestMessageLevel.Warning, text);
+            recorder.SendMessage(TestMessageLevel.Warning, text);
         }
     }
 }
diff --git a/src/NUnitTestAdapterTests/NUnitEngineTests/NUnitTestEventsTests.cs b/src/NUnitTestAdapterTests/NUnitEngineTests/NUnitTestEventsTests.cs
index 12ccaf1c..88fd0c74 100644
--- a/src/NUnitTestAdapterTests/NUnitEngineTests/NUnitTestEventsTests.cs
+++ b/src/NUnitTestAdapterTests/NUnitEngineTests/NUnitTestEventsTests.cs
@@ -223,6 +223,36 @@ public void ThatTestEventIsParsedForFinishSuiteWithFailure()
             Assert.That(sut.FailureMessage, Is.EqualTo("One or more child tests had errors"));
         }
 
+        /// <summary>
+        /// Exception in OneTimeSetUp
+        /// </summary>
+        private readonly string testSuiteFinishedWithFailureAndStackTrace = @"<test-suite type='TestFixture' id='0-1000' name='Tests' fullname='Issue671.Tests' classname='Issue671.Tests' runstate='Runnable' testcasecount='2' result='Failed' label='Error' site='SetUp' start-time='2021-05-29T09:34:10.2386869Z' end-time='2021-05-29T09:34:10.2532435Z' duration='0.014557' total='2' passed='0' failed='2' warnings='0' inconclusive='0' skipped='0' asserts='0'>
+            <failure>
+               <message><![CDATA[System.Exception : oops Deep Down]]></message>
+               <stack-trace><![CDATA[   at Issue671.SomeWhereDeepDown.WhatDoWeDoHere() in d:\repos\NUnit\nunit3-vs-adapter.issues\Issue671\UnitTest1.cs:line 44
+   at Issue671.Tests.OneTimeSetup() in d:\repos\NUnit\nunit3-vs-adapter.issues\Issue671\UnitTest1.cs:line 12]]></stack-trace>
+            </failure>
+            <test-case id='0-1001' name='Test1' fullname='Issue671.Tests.Test1' methodname='Test1' classname='Issue671.Tests' runstate='Runnable' seed='1958545482' result='Failed' label='Error' site='Parent' start-time='2021-05-29T09:34:10.2386869Z' end-time='2021-05-29T09:34:10.2510075Z' duration='0.012408' asserts='0'>
+               <failure>
+                  <message><![CDATA[OneTimeSetUp: System.Exception : oops Deep Down]]></message>
+               </failure>
+            </test-case>
+            <test-case id='0-1002' name='Test2' fullname='Issue671.Tests.Test2' methodname='Test2' classname='Issue671.Tests' runstate='Runnable' seed='711019934' result='Failed' label='Error' site='Parent' start-time='2021-05-29T09:34:10.2386869Z' end-time='2021-05-29T09:34:10.2523634Z' duration='0.013677' asserts='0'>
+               <failure>
+                  <message><![CDATA[OneTimeSetUp: System.Exception : oops Deep Down]]></message>
+               </failure>
+            </test-case>
+         </test-suite>";
+
+        [Test]
+        public void ThatTestEventIsParsedForFinishSuiteWithExceptionInOneTimeSetUp()
+        {
+            var sut = new NUnitTestEventSuiteFinished(testSuiteFinishedWithFailureAndStackTrace);
+            Assert.That(sut.HasFailure);
+            Assert.That(sut.FailureMessage.Length, Is.GreaterThan(0));
+            Assert.That(sut.StackTrace.Length, Is.GreaterThan(0));
+        }
+
 
 
         private readonly string testCaseSucceedsWithOutput = @"<test-case id='0-1074' name='TestSucceeds' fullname='NUnitTestDemo.SimpleTests.TestSucceeds' methodname='TestSucceeds' classname='NUnitTestDemo.SimpleTests' runstate='Runnable' seed='1232497275' result='Passed' start-time='2020-01-24 11:18:32Z' end-time='2020-01-24 11:18:32Z' duration='0.016868' asserts='1' parentId='0-1073'>