Hook extension improvements

src/NUnitFramework/framework/Internal/HookExtensions/AsyncEvent.cs

@@ -15,17 +15,6 @@ public sealed class AsyncEvent<TEventArgs>
         private readonly List<Delegate> _handlers = new();
         private readonly List<Delegate> _asyncHandlers = new();
 
-        /// <summary>
-        /// Constructor that initializes the event and provides an event handler to invoke it.
-        /// </summary>
-        /// <param name="invoke">
-        /// The event handler to invoke the event.
-        /// </param>
-        public AsyncEvent(out AsyncEventHandler<TEventArgs> invoke)
-        {
-            invoke = Invoke;
-        }
-
         /// <summary>
         /// Adds a synchronous handler to the event.
         /// </summary>
@@ -46,7 +35,19 @@ public void AddAsyncHandler(AsyncEventHandler<TEventArgs> asyncHandler)
                 _asyncHandlers.Add(asyncHandler);
         }
 
-        private Task Invoke(object? sender, TEventArgs e)
+        internal IReadOnlyList<Delegate> GetHandlers()
+        {
+            lock (_handlers)
+                return _handlers;
+        }
+
+        internal IReadOnlyList<Delegate> GetAsyncHandlers()
+        {
+            lock (_handlers)
+                return _asyncHandlers;
+        }
+
+        internal Task Invoke(object? sender, TEventArgs e)
         {
             if (!_handlers.Any() && !_asyncHandlers.Any())
             {

src/NUnitFramework/framework/Internal/HookExtensions/HookExtension.cs

@@ -17,21 +17,14 @@ public class HookExtension
     /// </summary>
     public HookExtension()
     {
-        BeforeAnySetUps = new AsyncEvent<TestHookIMethodEventArgs>(out _invokeBeforeAnySetUps);
-        AfterAnySetUps = new AsyncEvent<TestHookIMethodEventArgs>(out _invokeAfterAnySetUps);
-        BeforeTest = new AsyncEvent<TestHookTestMethodEventArgs>(out _invokeBeforeTest);
-        AfterTest = new AsyncEvent<TestHookTestMethodEventArgs>(out _invokeAfterTest);
-        BeforeAnyTearDowns = new AsyncEvent<TestHookIMethodEventArgs>(out _invokeBeforeAnyTearDowns);
-        AfterAnyTearDowns = new AsyncEvent<TestHookIMethodEventArgs>(out _invokeAfterAnyTearDowns);
+        BeforeAnySetUps = new AsyncEvent<TestHookIMethodEventArgs>();
+        AfterAnySetUps = new AsyncEvent<TestHookIMethodEventArgs>();
+        BeforeTest = new AsyncEvent<TestHookTestMethodEventArgs>();
+        AfterTest = new AsyncEvent<TestHookTestMethodEventArgs>();
+        BeforeAnyTearDowns = new AsyncEvent<TestHookIMethodEventArgs>();
+        AfterAnyTearDowns = new AsyncEvent<TestHookIMethodEventArgs>();
     }
 
-    private AsyncEventHandler<TestHookIMethodEventArgs> _invokeBeforeAnySetUps;
-    private AsyncEventHandler<TestHookIMethodEventArgs> _invokeAfterAnySetUps;
-    private AsyncEventHandler<TestHookTestMethodEventArgs> _invokeBeforeTest;
-    private AsyncEventHandler<TestHookTestMethodEventArgs> _invokeAfterTest;
-    private AsyncEventHandler<TestHookIMethodEventArgs> _invokeBeforeAnyTearDowns;
-    private AsyncEventHandler<TestHookIMethodEventArgs> _invokeAfterAnyTearDowns;
-
     /// <summary>
     /// Gets or sets the hook event that is triggered before any setup methods are executed.
     /// </summary>
@@ -68,42 +61,49 @@ public HookExtension()
     /// <param name="other">The instance of <see cref="HookExtension"/> to copy hooks from.</param>
     public HookExtension(HookExtension other) : this()
     {
-        other._invokeBeforeAnySetUps?.GetInvocationList()?.ToList().ForEach(d => _invokeBeforeAnySetUps += d as AsyncEventHandler<TestHookIMethodEventArgs>);
-        other._invokeAfterAnySetUps?.GetInvocationList()?.ToList().ForEach(d => _invokeAfterAnySetUps += d as AsyncEventHandler<TestHookIMethodEventArgs>);
-        other._invokeBeforeTest?.GetInvocationList()?.ToList().ForEach(d => _invokeBeforeTest += d as AsyncEventHandler<TestHookTestMethodEventArgs>);
-        other._invokeAfterTest?.GetInvocationList()?.ToList().ForEach(d => _invokeAfterTest += d as AsyncEventHandler<TestHookTestMethodEventArgs>);
-        other._invokeBeforeAnyTearDowns?.GetInvocationList()?.ToList().ForEach(d => _invokeBeforeAnyTearDowns += d as AsyncEventHandler<TestHookIMethodEventArgs>);
-        other._invokeAfterAnyTearDowns?.GetInvocationList()?.ToList().ForEach(d => _invokeAfterAnyTearDowns += d as AsyncEventHandler<TestHookIMethodEventArgs>);
+        other.BeforeAnySetUps.GetHandlers().ToList().ForEach(d => BeforeAnySetUps.AddHandler((EventHandler<TestHookIMethodEventArgs>)d));
+        other.AfterAnySetUps.GetHandlers().ToList().ForEach(d => AfterAnySetUps.AddHandler((EventHandler<TestHookIMethodEventArgs>)d));
+        other.BeforeTest.GetHandlers().ToList().ForEach(d => BeforeTest.AddHandler((EventHandler<TestHookTestMethodEventArgs>)d));
+        other.AfterTest.GetHandlers().ToList().ForEach(d => AfterTest.AddHandler((EventHandler<TestHookTestMethodEventArgs>)d));
+        other.BeforeAnyTearDowns.GetHandlers().ToList().ForEach(d => BeforeAnyTearDowns.AddHandler((EventHandler<TestHookIMethodEventArgs>)d));
+        other.AfterAnyTearDowns.GetHandlers().ToList().ForEach(d => AfterAnyTearDowns.AddHandler((EventHandler<TestHookIMethodEventArgs>)d));
+
+        other.BeforeAnySetUps.GetAsyncHandlers().ToList().ForEach(d => BeforeAnySetUps.AddAsyncHandler((AsyncEventHandler<TestHookIMethodEventArgs>)d));
+        other.AfterAnySetUps.GetAsyncHandlers().ToList().ForEach(d => AfterAnySetUps.AddAsyncHandler((AsyncEventHandler<TestHookIMethodEventArgs>)d));
+        other.BeforeTest.GetAsyncHandlers().ToList().ForEach(d => BeforeTest.AddAsyncHandler((AsyncEventHandler<TestHookTestMethodEventArgs>)d));
+        other.AfterTest.GetAsyncHandlers().ToList().ForEach(d => AfterTest.AddAsyncHandler((AsyncEventHandler<TestHookTestMethodEventArgs>)d));
+        other.BeforeAnyTearDowns.GetAsyncHandlers().ToList().ForEach(d => BeforeAnyTearDowns.AddAsyncHandler((AsyncEventHandler<TestHookIMethodEventArgs>)d));
+        other.AfterAnyTearDowns.GetAsyncHandlers().ToList().ForEach(d => AfterAnyTearDowns.AddAsyncHandler((AsyncEventHandler<TestHookIMethodEventArgs>)d));
     }
 
     internal void OnBeforeAnySetUps(TestExecutionContext context, IMethodInfo method)
     {
-        _invokeBeforeAnySetUps(this, new TestHookIMethodEventArgs(context, method));
+        BeforeAnySetUps.Invoke(this, new TestHookIMethodEventArgs(context, method));
     }
 
     internal void OnAfterAnySetUps(TestExecutionContext context, IMethodInfo method, Exception? exceptionContext = null)
     {
-        _invokeAfterAnySetUps(this, new TestHookIMethodEventArgs(context, method, exceptionContext));
+        AfterAnySetUps.Invoke(this, new TestHookIMethodEventArgs(context, method, exceptionContext));
     }
 
     internal void OnBeforeTest(TestExecutionContext context)
     {
-        _invokeBeforeTest(this, new TestHookTestMethodEventArgs(context));
+        BeforeTest.Invoke(this, new TestHookTestMethodEventArgs(context));
     }
 
     internal void OnAfterTest(TestExecutionContext context, Exception? exceptionContext = null)
     {
-        _invokeAfterTest(this, new TestHookTestMethodEventArgs(context, exceptionContext));
+        AfterTest.Invoke(this, new TestHookTestMethodEventArgs(context, exceptionContext));
     }
 
     internal void OnBeforeAnyTearDowns(TestExecutionContext context, IMethodInfo method)
     {
-        _invokeBeforeAnyTearDowns(this, new TestHookIMethodEventArgs(context, method));
+        BeforeAnyTearDowns.Invoke(this, new TestHookIMethodEventArgs(context, method));
     }
 
     internal void OnAfterAnyTearDowns(TestExecutionContext context, IMethodInfo method, Exception? exceptionContext = null)
     {
-        _invokeAfterAnyTearDowns(this, new TestHookIMethodEventArgs(context, method, exceptionContext));
+        AfterAnyTearDowns.Invoke(this, new TestHookIMethodEventArgs(context, method, exceptionContext));
     }
 }
 

src/NUnitFramework/tests/HookExtension/ConstructorTests.cs

@@ -0,0 +1,65 @@
+// Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt
+
+namespace NUnit.Framework.Tests.HookExtension
+{
+    internal class HookExtensionConstructorTests
+    {
+        [Test]
+        public void CopyCtor_CreateNewHookExtension_InvocationListShouldBeEmpty()
+        {
+            var hookExt = new NUnit.Framework.Internal.HookExtensions.HookExtension();
+
+            Assert.Multiple(() =>
+            {
+                Assert.That(hookExt.BeforeAnySetUps.GetHandlers().Count, Is.EqualTo(0));
+                Assert.That(hookExt.AfterAnySetUps.GetHandlers().Count, Is.EqualTo(0));
+                Assert.That(hookExt.BeforeTest.GetHandlers().Count, Is.EqualTo(0));
+                Assert.That(hookExt.AfterTest.GetHandlers().Count, Is.EqualTo(0));
+                Assert.That(hookExt.BeforeAnyTearDowns.GetHandlers().Count, Is.EqualTo(0));
+                Assert.That(hookExt.AfterAnyTearDowns.GetHandlers().Count, Is.EqualTo(0));
+
+                Assert.That(hookExt.BeforeAnySetUps.GetAsyncHandlers().Count, Is.EqualTo(0));
+                Assert.That(hookExt.AfterAnySetUps.GetAsyncHandlers().Count, Is.EqualTo(0));
+                Assert.That(hookExt.BeforeTest.GetAsyncHandlers().Count, Is.EqualTo(0));
+                Assert.That(hookExt.AfterTest.GetAsyncHandlers().Count, Is.EqualTo(0));
+                Assert.That(hookExt.BeforeAnyTearDowns.GetAsyncHandlers().Count, Is.EqualTo(0));
+                Assert.That(hookExt.AfterAnyTearDowns.GetAsyncHandlers().Count, Is.EqualTo(0));
+            });
+        }
+
+        [Test]
+        public void CopyCtor_CallMultipleTimes_ShallNotIncreaseInvocationList()
+        {
+            var hookExt = new NUnit.Framework.Internal.HookExtensions.HookExtension();
+            hookExt.AfterTest.AddHandler((sender, args) => { });
+            hookExt.AfterTest.AddAsyncHandler(async (sender, args) => { });
+
+            hookExt = new NUnit.Framework.Internal.HookExtensions.HookExtension(hookExt);
+            hookExt = new NUnit.Framework.Internal.HookExtensions.HookExtension(hookExt);
+            hookExt = new NUnit.Framework.Internal.HookExtensions.HookExtension(hookExt);
+
+            // initially assigned handlers shall be copied
+            Assert.Multiple(() =>
+            {
+                Assert.That(hookExt.AfterTest.GetHandlers().Count, Is.EqualTo(1));
+                Assert.That(hookExt.AfterTest.GetAsyncHandlers().Count, Is.EqualTo(1));
+            });
+
+            // all others shall stay empty
+            Assert.Multiple(() =>
+            {
+                Assert.That(hookExt.BeforeAnySetUps.GetHandlers().Count, Is.EqualTo(0));
+                Assert.That(hookExt.AfterAnySetUps.GetHandlers().Count, Is.EqualTo(0));
+                Assert.That(hookExt.BeforeTest.GetHandlers().Count, Is.EqualTo(0));
+                Assert.That(hookExt.BeforeAnyTearDowns.GetHandlers().Count, Is.EqualTo(0));
+                Assert.That(hookExt.AfterAnyTearDowns.GetHandlers().Count, Is.EqualTo(0));
+
+                Assert.That(hookExt.BeforeAnySetUps.GetAsyncHandlers().Count, Is.EqualTo(0));
+                Assert.That(hookExt.AfterAnySetUps.GetAsyncHandlers().Count, Is.EqualTo(0));
+                Assert.That(hookExt.BeforeTest.GetAsyncHandlers().Count, Is.EqualTo(0));
+                Assert.That(hookExt.BeforeAnyTearDowns.GetAsyncHandlers().Count, Is.EqualTo(0));
+                Assert.That(hookExt.AfterAnyTearDowns.GetAsyncHandlers().Count, Is.EqualTo(0));
+            });
+        }
+    }
+}

src/NUnitFramework/tests/Internal/HookExtension/AsyncEventTests.cs

@@ -10,22 +10,21 @@ public class AsyncEventTests
         [Test]
         public void Invoke_AsyncEventThrowingException_AggregatedExceptionIsThrown()
         {
-            var asyncEvent = new AsyncEvent<TestHookTestMethodEventArgs>(out var invoke);
+            var asyncEvent = new AsyncEvent<TestHookTestMethodEventArgs>();
             var exception = new Exception("Test exception");
             var testMethodEventArgs = new TestHookTestMethodEventArgs(null);
             asyncEvent.AddAsyncHandler(async (sender, args) => throw exception);
-            Assert.Throws<AggregateException>(() => invoke(this, testMethodEventArgs).Wait());
+            Assert.Throws<AggregateException>(() => asyncEvent.Invoke(this, testMethodEventArgs).Wait());
         }
 
         [Test]
         public void Invoke_SyncEventThrowingException_AggregatedExceptionIsThrown()
         {
-            var syncEvent = new AsyncEvent<TestHookTestMethodEventArgs>(out var invoke);
+            var syncEvent = new AsyncEvent<TestHookTestMethodEventArgs>();
             var exception = new Exception("Test exception");
             var testMethodEventArgs = new TestHookTestMethodEventArgs(null);
             syncEvent.AddHandler((sender, args) => throw exception);
-            Assert.Throws<AggregateException>(() => invoke(this, testMethodEventArgs).Wait());
+            Assert.Throws<AggregateException>(() => syncEvent.Invoke(this, testMethodEventArgs).Wait());
         }
-
     }
 }