WIP: Additional Test Events for OneTimeSetUp / OneTimeTearDown

AdditionalTestEvents.md

@@ -0,0 +1,49 @@
+# Additional Test Events for OneTimeSetUp and OneTimeTearDown
+
+## Problem Statement
+
+For our higher-level System Tests, we need a mechanism that notified us during the execution of a test run when certain parts of the test suite are executed. Regarding the begin and the end of a test case, NUnit already already provides corresponding test events. So we can implement a custom TestEventListener and attach some handlers to it that react when the corresponding events occure.
+
+However, we need a similar mechanism also for OneTimeSetUp and OneTimeTearDown.
+
+## Solution Statement
+
+In this branch we tried to implement a solution by adding additional test events for 
+* Start of OneTimeSetUp 
+* End of OneTimeSetUp 
+* Start of OneTimeTearDown 
+* End of OneTimeTearDown
+
+We would appreciate your opinion if the provided solution is in principle a good idea or if we should tackle the problem in a completely different way.
+
+### Description of the Implementation
+
+#### Extending the ITestListener
+
+First I extended `ITestListener` by the corresponding APIs,
+e.g. `void OneTimeSetUpStarted(ITest test);`
+> I am fully aware that the parameter type ITest is not suitable here, because it only contains information about the surrounding test fixture. However I took it as a starting point in order to simulate how event parameters will be passed in the `TestProgressReporter`.
+
+Possible drawback:
+Every class implementing `ITestListener` has to be adapted. These are in this case here:
+* The EventQueue
+* The QueuingEventListener
+* The always empty (dummy) TestListener.cs
+* The TestProgressReporter, that creates the xml formated events
+* The TeamCityEventListener (seems specific, I left the implementation empty)
+* The TextRunner for NUnitLite (left implementation empty for now)
+* All test classes that implement `ITestListener` 
+
+#### Finding the Right Place to Raise the Events
+
+I ended up raising the events in the `OneTimeSetUpCommand` and `OneTimeTearDownCommand`, however I experimented with different locations:
+
+* OneTimeSetUpCommand.cs
+Felt most fitting because the event can be raised as part of the `BeforeTest` / `AfterTest` action, thats why I ended up imlementing the solution here. However I noticed that the OneTimeTearDown event in this case is raised too often in the special case that a fixture has an additional `OneTimeSetUp` in a base class. See my additions in the `TestAssemblyRunnerTests` in order to see what I mean. 
+* SetUpTearDownItem.cs
+Benefit from raising the event here would be that we are most close to the execution of the actual method. 
+Drawback: At this level we cannot distinguish anymore between `SetUp` and `OneTimeSetUp` etc...
+Please see also my comments in this class.
+* CompositeWorkItem.cs
+This was my first attempt, just because the method name ("MakeOneTimeSetUpCommand") indicated it. However, also here I received additional unexpected events and the methods for OneTimeSetUp and OneTimeTearDown looked a bit asymetric, so I didnt follow up the implementation at that place. 
+I also stumbled over a code block that looked a little suspicious and could be responsible for the additional unexpected event. Please see also my comments in this class.

src/NUnitFramework/framework/Api/NUnitTestAssemblyRunner.cs

@@ -323,6 +323,10 @@ private TestExecutionContext CreateTestExecutionContext(ITest loadedTest, ITestL
             // Set the listener - overriding runners may replace this
             context.Listener = listener;
 
+            // Set the listener for extended test events like OneTimeSetUp / OneTimeTearDown
+            if (listener is ITestListenerExt testListenerExt)
+                context.ListenerExt = testListenerExt;
+
             int levelOfParallelism = GetLevelOfParallelism(loadedTest);
 
             if (Settings.TryGetValue(FrameworkPackageSettings.RunOnMainThread, out var runOnMainThread) &&

src/NUnitFramework/framework/Interfaces/ITestListenerExt.cs

@@ -0,0 +1,41 @@
+// Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt
+
+namespace NUnit.Framework.Interfaces
+{
+    /// <summary>
+    /// The ITestListenerExt interface is used internally to receive
+    /// notifications of significant events while a test is being
+    /// run. The events are propagated to clients by means of an
+    /// AsyncCallback. NUnit extensions may also monitor these events.
+    ///
+    /// It is an extension to <see cref="ITestListener"/> that
+    /// covers events for <see cref="OneTimeSetUpAttribute"/>
+    /// and <see cref="OneTimeTearDownAttribute"/> methods.
+    /// </summary>
+    public interface ITestListenerExt
+    {
+        /// <summary>
+        /// Called when a OneTimeSetUp has just started.
+        /// </summary>
+        /// <param name="test">The test to which this OneTimeSetUp belongs.</param>
+        void OneTimeSetUpStarted(ITest test);
+
+        /// <summary>
+        /// Called when a OneTimeSetUp has just finished.
+        /// </summary>
+        /// <param name="test">The test to which this OneTimeSetUp belongs.</param>
+        void OneTimeSetUpFinished(ITest test);
+
+        /// <summary>
+        /// Called when a OneTimeTearDown has just started.
+        /// </summary>
+        /// <param name="test">The test to which this OneTimeTearDown belongs.</param>
+        void OneTimeTearDownStarted(ITest test);
+
+        /// <summary>
+        /// Called when a OneTimeTearDown has just finished.
+        /// </summary>
+        /// <param name="test">The test to which this OneTimeTearDown belongs.</param>
+        void OneTimeTearDownFinished(ITest test);
+    }
+}

src/NUnitFramework/framework/Internal/Commands/SetUpTearDownItem.cs

@@ -50,7 +50,29 @@ public void RunSetUp(TestExecutionContext context)
             _setUpWasRun = true;
 
             foreach (IMethodInfo setUpMethod in _setUpMethods)
-                RunSetUpOrTearDownMethod(context, setUpMethod);
+            {
+                RaiseOneTimeSetUpStarted(context);
+                try
+                {
+                    RunSetUpOrTearDownMethod(context, setUpMethod);
+                }
+                finally
+                {
+                    RaiseOneTimeSetUpFinished(context);
+                }
+            }
+        }
+
+        private void RaiseOneTimeSetUpStarted(TestExecutionContext context)
+        {
+            if (context.CurrentTest is not null && context.CurrentTest.IsSuite)
+                context.ListenerExt?.OneTimeSetUpStarted(context.CurrentTest);
+        }
+
+        private void RaiseOneTimeSetUpFinished(TestExecutionContext context)
+        {
+            if (context.CurrentTest is not null && context.CurrentTest.IsSuite)
+                context.ListenerExt?.OneTimeSetUpFinished(context.CurrentTest);
         }
 
         /// <summary>
@@ -72,7 +94,17 @@ public void RunTearDown(TestExecutionContext context)
                     // run the teardowns in reverse order to provide consistency.
                     var index = _tearDownMethods.Count;
                     while (--index >= 0)
-                        RunSetUpOrTearDownMethod(context, _tearDownMethods[index]);
+                    {
+                        RaiseOneTimeTearDownStarted(context);
+                        try
+                        {
+                            RunSetUpOrTearDownMethod(context, _tearDownMethods[index]);
+                        }
+                        finally
+                        {
+                            RaiseOneTimeTearDownFinished(context);
+                        }
+                    }
 
                     // If there are new assertion results here, they are warnings issued
                     // in teardown. Redo test completion so they are listed properly.
@@ -86,6 +118,18 @@ public void RunTearDown(TestExecutionContext context)
             }
         }
 
+        private void RaiseOneTimeTearDownStarted(TestExecutionContext context)
+        {
+            if (context.CurrentTest is not null && context.CurrentTest.IsSuite)
+                context.ListenerExt?.OneTimeTearDownStarted(context.CurrentTest);
+        }
+
+        private void RaiseOneTimeTearDownFinished(TestExecutionContext context)
+        {
+            if (context.CurrentTest is not null && context.CurrentTest.IsSuite)
+                context.ListenerExt?.OneTimeTearDownFinished(context.CurrentTest);
+        }
+
         private void RunSetUpOrTearDownMethod(TestExecutionContext context, IMethodInfo method)
         {
             Guard.ArgumentNotAsyncVoid(method.MethodInfo, nameof(method));

src/NUnitFramework/framework/Internal/Execution/EventPumpExt.cs

@@ -0,0 +1,26 @@
+// Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt
+
+using System;
+using NUnit.Framework.Interfaces;
+
+namespace NUnit.Framework.Internal.Execution
+{
+    /// <summary>
+    /// EventPump pulls extended Event instances out of an EventQueue and sends
+    /// them to a ITestListenerExt. It is used to send these events back to
+    /// the client without using the CallContext of the test
+    /// runner thread.
+    /// </summary>
+    public sealed class EventPumpExt : EventPump<ExtendedEvent, ITestListenerExt>, IDisposable
+    {
+        /// <summary>
+        /// Constructor for extended EventPump
+        /// </summary>
+        /// <param name="eventListener">The EventListener to receive events</param>
+        /// <param name="events">The event queue to pull events from</param>
+        public EventPumpExt(ITestListenerExt eventListener, EventQueue<ExtendedEvent> events)
+            : base(eventListener, events, "ExtendedEventPump")
+        {
+        }
+    }
+}

src/NUnitFramework/framework/Internal/Execution/EventQueue.cs

@@ -37,6 +37,23 @@ public abstract class Event : IEvent<ITestListener>
         public abstract void Send(ITestListener listener);
     }
 
+    /// <summary>
+    /// NUnit.Core.ExtendedEvent is the abstract base for
+    /// extended events (e.g. for OneTimeSetUp started).
+    /// An Event is the stored representation of a call to the
+    /// <see cref="ITestListenerExt"/> interface and is used to record such calls
+    /// or to queue them for forwarding on another thread or at
+    /// a later time.
+    /// </summary>
+    public abstract class ExtendedEvent : IEvent<ITestListenerExt>
+    {
+        /// <summary>
+        /// The Send method is implemented by derived classes to send the event to the specified listener.
+        /// </summary>
+        /// <param name="listener">The listener.</param>
+        public abstract void Send(ITestListenerExt listener);
+    }
+
     /// <summary>
     /// TestStartedEvent holds information needed to call the TestStarted method.
     /// </summary>
@@ -89,6 +106,110 @@ public override void Send(ITestListener listener)
         }
     }
 
+    /// <summary>
+    /// OneTimeSetUpStartedEvent holds information needed to call the OneTimeSetUpStarted method.
+    /// </summary>
+    public class OneTimeSetUpStartedEvent : ExtendedEvent
+    {
+        private readonly ITest _test;
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="OneTimeSetUpStartedEvent"/> class.
+        /// </summary>
+        /// <param name="test">Object that holds information about the event. Placeholder as of now, needs to be changed for productization.</param>
+        public OneTimeSetUpStartedEvent(ITest test)
+        {
+            _test = test;
+        }
+
+        /// <summary>
+        /// Calls OneTimeSetUpStarted on the specified listener.
+        /// </summary>
+        /// <param name="listener">The listener.</param>
+        public override void Send(ITestListenerExt listener)
+        {
+            listener.OneTimeSetUpStarted(_test);
+        }
+    }
+
+    /// <summary>
+    /// OneTimeSetUpFinishedEvent holds information needed to call the OneTimeSetUpFinished method.
+    /// </summary>
+    public class OneTimeSetUpFinishedEvent : ExtendedEvent
+    {
+        private readonly ITest _test;
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="OneTimeSetUpFinishedEvent"/> class.
+        /// </summary>
+        /// <param name="test">Object that holds information about the event. Placeholder as of now, needs to be changed for productization.</param>
+        public OneTimeSetUpFinishedEvent(ITest test)
+        {
+            _test = test;
+        }
+
+        /// <summary>
+        /// Calls OneTimeSetUpFinished on the specified listener.
+        /// </summary>
+        /// <param name="listener">The listener.</param>
+        public override void Send(ITestListenerExt listener)
+        {
+            listener.OneTimeSetUpFinished(_test);
+        }
+    }
+
+    /// <summary>
+    /// OneTimeTearDownStartedEvent holds information needed to call the OneTimeTearDownStarted method.
+    /// </summary>
+    public class OneTimeTearDownStartedEvent : ExtendedEvent
+    {
+        private readonly ITest _test;
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="OneTimeTearDownStartedEvent"/> class.
+        /// </summary>
+        /// <param name="test">Object that holds information about the event. Placeholder as of now, needs to be changed for productization.</param>
+        public OneTimeTearDownStartedEvent(ITest test)
+        {
+            _test = test;
+        }
+
+        /// <summary>
+        /// Calls OneTimeTearDownStarted on the specified listener.
+        /// </summary>
+        /// <param name="listener">The listener.</param>
+        public override void Send(ITestListenerExt listener)
+        {
+            listener.OneTimeTearDownStarted(_test);
+        }
+    }
+
+    /// <summary>
+    /// OneTimeTearDownFinishedEvent holds information needed to call the OneTimeTearDownFinished method.
+    /// </summary>
+    public class OneTimeTearDownFinishedEvent : ExtendedEvent
+    {
+        private readonly ITest _test;
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="OneTimeTearDownFinishedEvent"/> class.
+        /// </summary>
+        /// <param name="test">Object that holds information about the event. Placeholder as of now, needs to be changed for productization.</param>
+        public OneTimeTearDownFinishedEvent(ITest test)
+        {
+            _test = test;
+        }
+
+        /// <summary>
+        /// Calls OneTimeTearDownFinished on the specified listener.
+        /// </summary>
+        /// <param name="listener">The listener.</param>
+        public override void Send(ITestListenerExt listener)
+        {
+            listener.OneTimeTearDownFinished(_test);
+        }
+    }
+
     /// <summary>
     /// TestOutputEvent holds information needed to call the TestOutput method.
     /// </summary>
@@ -154,6 +275,14 @@ public sealed class EventQueue : EventQueue<Event>
     {
     }
 
+    /// <summary>
+    /// Implements a queue of work items for extended Event types each of which
+    /// is queued as a WaitCallback.
+    /// </summary>
+    public sealed class EventQueueExt : EventQueue<ExtendedEvent>
+    {
+    }
+
     /// <summary>
     /// Implements a template for a queue of work items each of which
     /// is queued as a WaitCallback.