Eliminate use of StopRun(true)

.github/workflows/NUnitConsoleAndEngine.CI.yml

@@ -45,7 +45,7 @@ jobs:
       - name: 🔨 Build, Test, Package and Publish (Cake script decides)
         run: dotnet cake --target=ContinuousIntegration --configuration=Release #--verbosity=diagnostic
 
-      # Upload packages before publishing in case of later failures
+      # Upload packages
       - name: 💾 Upload build artifacts
         if: always()
         uses: actions/upload-artifact@v7

src/Directory.Build.props

@@ -22,7 +22,7 @@
         <DebugSymbols>true</DebugSymbols>
         <!-- Commonly Used Package Versions -->
         <AnnotatedReferenceAssemblyVersion>8.0.0</AnnotatedReferenceAssemblyVersion>
-        <EngineApiVersion>4.0.0-beta.3</EngineApiVersion>
+        <EngineApiVersion>4.0.0-beta.4</EngineApiVersion>
         <!-- Informational Settings -->
         <Company>NUnit Software</Company>
         <Product>NUnit 4 Runner and Engine</Product>

src/NUnitCommon/nunit.agent.core.tests/Runners/TestAgentRunnerExceptionTests.cs

@@ -111,23 +111,22 @@ public void Run_Throws_NUnitEngineException()
         [Test]
         public void StopRun_Passes_Along_NUnitEngineException()
         {
-            _driver.When(x => x.StopRun(Arg.Any<bool>()))
+            _driver.When(x => x.ForcedStop())
                 .Do(x => { throw new NUnitEngineException("Message"); });
 
             var ex = Assert.Throws<NUnitEngineException>(() => _runner.ForcedStop());
             Assert.That(ex.Message, Is.EqualTo("Message"));
         }
 
         [Test]
-        public void StopRun_Throws_NUnitEngineException()
+        public void StopRun_Throws_ArgumentException()
         {
-            _driver.When(x => x.StopRun(Arg.Any<bool>()))
+            _driver.When(x => x.ForcedStop())
                 .Do(x => { throw new ArgumentException("Message"); });
 
-            var ex = Assert.Throws<NUnitEngineException>(() => _runner.ForcedStop());
-            Assert.That(ex.InnerException, Is.Not.Null);
-            Assert.That(ex.InnerException, Is.InstanceOf<ArgumentException>());
-            Assert.That(ex.InnerException.Message, Is.EqualTo("Message"));
+            var ex = Assert.Throws<ArgumentException>(() => _runner.ForcedStop());
+            Assert.That(ex.
+                Message, Is.EqualTo("Message"));
         }
     }
 }

src/NUnitCommon/nunit.agent.core/Drivers/InvalidAssemblyFrameworkDriver.cs

@@ -55,7 +55,11 @@ public string Explore(string filter)
             return GetLoadResult();
         }
 
-        public void StopRun(bool force)
+        public void RequestStop()
+        {
+        }
+
+        public void ForcedStop()
         {
         }
 

src/NUnitCommon/nunit.agent.core/Drivers/NUnit3DriverFactory.cs

@@ -18,7 +18,7 @@ public class NUnit3DriverFactory : IDriverFactory
         /// <param name="reference">An AssemblyName referring to the possible test framework.</param>
         public bool IsSupportedTestFramework(AssemblyName reference)
         {
-            return NUNIT_FRAMEWORK.Equals(reference.Name, StringComparison.OrdinalIgnoreCase) && reference.Version?.Major >= 3;
+            return NUNIT_FRAMEWORK.Equals(reference.Name, StringComparison.OrdinalIgnoreCase) && reference.Version?.Major is 3 or 4;
         }
 
 #if NETFRAMEWORK

src/NUnitCommon/nunit.agent.core/Drivers/NUnitFrameworkApi.cs

@@ -5,6 +5,10 @@
 
 namespace NUnit.Engine.Drivers
 {
+    /// <summary>
+    /// Driver API for the NUnit Framework. Provides a common interface to al
+    /// versions of the framework, in spite of differences in their own API.
+    /// </summary>
     public interface NUnitFrameworkApi
     {
         /// <summary>
@@ -45,7 +49,17 @@ public interface NUnitFrameworkApi
         /// <summary>
         /// Cancel the ongoing test run. If no  test is running, the call is ignored.
         /// </summary>
-        /// <param name="force">If true, cancel any ongoing test threads, otherwise wait for them to complete.</param>
-        void StopRun(bool force);
+        void RequestStop();
+
+        /// <summary>
+        /// Force the current test run to stop, killing threads or processes if necessary.
+        /// </summary>
+        /// <exception cref="NotImplementedException" />
+        void ForcedStop();
+
+        /// <summary>
+        /// Gets a flag indicating whether ForcedStop is supported for the framework version in use.
+        /// </summary>
+        bool ForcedStopSupported { get; }
     }
 }

src/NUnitCommon/nunit.agent.core/Drivers/NUnitFrameworkApi2009.cs

@@ -103,7 +103,11 @@ public string Run(ITestEventListener? listener, string filter)
 
         public void RunAsync(Action<string>? callback, string filter) => throw new NotImplementedException();
 
-        public void StopRun(bool force) => ExecuteAction(STOP_RUN_ACTION, force);
+        public void RequestStop() => ExecuteAction(STOP_RUN_ACTION, false);
+
+        public void ForcedStop() => ExecuteAction(STOP_RUN_ACTION, true);
+
+        public bool ForcedStopSupported => true;
 
         public string Explore(string filter)
         {

src/NUnitCommon/nunit.agent.core/Drivers/NUnitFrameworkApi2018.cs

@@ -178,11 +178,18 @@ public void RunAsync(Action<string>? callback, string filter)
             ExecuteMethod(RUN_ASYNC_METHOD, [typeof(Action<string>), typeof(string)], callback, filter);
         }
 
-        public void StopRun(bool force)
+        public void RequestStop()
         {
-            ExecuteMethod(STOP_RUN_METHOD, force);
+            ExecuteMethod(STOP_RUN_METHOD, false);
         }
 
+        public void ForcedStop()
+        {
+            ExecuteMethod(STOP_RUN_METHOD, true);
+        }
+
+        public bool ForcedStopSupported => _nunitRef.Version.ShouldNotBeNull().Major is 3 or 4;
+
         public string Explore(string filter)
         {
             CheckLoadWasCalled();

src/NUnitCommon/nunit.agent.core/Drivers/NUnitFrameworkDriver.cs

@@ -3,6 +3,7 @@
 using NUnit.Engine.Extensibility;
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Reflection;
 
 #if NETCOREAPP3_1_OR_GREATER
@@ -20,6 +21,8 @@ public class NUnitFrameworkDriver : IFrameworkDriver
         private static readonly Version MINIMUM_NUNIT_VERSION = new(3, 2, 0);
         private static readonly Logger log = InternalTrace.GetLogger(nameof(NUnitFrameworkDriver));
 
+        private readonly Version _nunitVersion;
+
 #if NETFRAMEWORK
         private readonly NUnitFrameworkApi _api;
 
@@ -35,6 +38,7 @@ public NUnitFrameworkDriver(AppDomain testDomain, string id, AssemblyName nunitR
             Guard.ArgumentNotNull(nunitRef);
 
             ID = id;
+            _nunitVersion = nunitRef.Version.ShouldNotBeNull();
 
             if (nunitRef.Version >= MINIMUM_NUNIT_VERSION)
             {
@@ -69,6 +73,8 @@ internal NUnitFrameworkDriver(AppDomain testDomain, string api, string id, Assem
             Guard.ArgumentNotNullOrEmpty(id);
             Guard.ArgumentNotNull(nunitRef);
 
+            _nunitVersion = nunitRef.Version.ShouldNotBeNull();
+
             ID = id;
             API = api;
 
@@ -99,6 +105,7 @@ public NUnitFrameworkDriver(string id, AssemblyName nunitRef)
             ID = id;
             API = "2018";
 
+            _nunitVersion = nunitRef.Version.ShouldNotBeNull();
             _api = new NUnitFrameworkApi2018(ID, nunitRef);
         }
 
@@ -152,8 +159,19 @@ public void RunAsync(ITestEventListener? listener, string filter) =>
         /// <summary>
         /// Cancel the ongoing test run. If no  test is running, the call is ignored.
         /// </summary>
-        /// <param name="force">If true, cancel any ongoing test threads, otherwise wait for them to complete.</param>
-        public void StopRun(bool force) => _api.StopRun(force);
+        public void RequestStop() => _api.RequestStop();
+
+        /// <summary>
+        /// Force the current test run to stop, killing threads or processes if necessary.
+        /// If no tests are running, the call is ignored.
+        /// </summary>
+        public void ForcedStop()
+        {
+            if (_api.ForcedStopSupported)
+                _api.ForcedStop();
+            else
+                Process.GetCurrentProcess().Kill();
+        }
 
         /// <summary>
         /// Returns information about the tests in an assembly.

src/NUnitCommon/nunit.agent.core/Runners/TestAgentRunner.cs

@@ -244,25 +244,13 @@ public AsyncTestEngineResult RunAsync(ITestEventListener? listener, TestFilter f
         /// Request the current test run to stop. If no tests are running,
         /// the call is ignored.
         /// </summary>
-        public void RequestStop() => StopRun(false);
+        public void RequestStop() => GetLoadedDriver().RequestStop();
 
         /// <summary>
         /// Force the current test run to stop, killing threads or processes if necessary.
         /// If no tests are running, the call is ignored.
         /// </summary>
-        public void ForcedStop() => StopRun(true);
-
-        private void StopRun(bool force)
-        {
-            try
-            {
-                GetLoadedDriver().StopRun(force);
-            }
-            catch (Exception ex) when (!(ex is NUnitEngineException))
-            {
-                throw new NUnitEngineException("An exception occurred in the driver while stopping the run.", ex);
-            }
-        }
+        public void ForcedStop() => GetLoadedDriver().ForcedStop();
 
         private IFrameworkDriver GetLoadedDriver()
         {

src/NUnitEngine/nunit.engine.tests/Runners/MasterTestRunnerTests.cs

@@ -94,16 +94,13 @@ public void RunAsync()
             _engineRunner.Received().Run(Arg.Any<TestEventDispatcher>(), filter);
         }
 
-        [TestCase(false)]
-        [TestCase(true)]
-        public void StopRun(bool force)
+        [Test]
+        public void RequestStop()
         {
             _masterTestRunner.GetEngineRunner();
-            _masterTestRunner.StopRun(force);
-            if (force)
-                _engineRunner.Received().ForcedStop();
-            else
-                _engineRunner.Received().RequestStop();
+            _masterTestRunner.RequestStop();
+
+            _engineRunner.Received().RequestStop();
         }
 #endif
     }

src/NUnitEngine/nunit.engine/Runners/MasterTestRunner.cs

@@ -1,5 +1,7 @@
 // Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt
 
+#define USE_WORK_ITEM_TRACKER
+
 using System;
 using System.ComponentModel;
 using System.Diagnostics;
@@ -51,7 +53,9 @@ public class MasterTestRunner : ITestRunner
         private bool _disposed;
 
         private TestEventDispatcher _eventDispatcher = new TestEventDispatcher();
+#if USE_WORK_ITEM_TRACKER
         private WorkItemTracker _workItemTracker = new WorkItemTracker();
+#endif
 
         private int _testRunTimeout;
         private Timer? _testRunTimer;
@@ -186,19 +190,16 @@ public ITestRun RunAsync(ITestEventListener? listener, TestFilter filter)
         /// <summary>
         /// Cancel the ongoing test run. If no  test is running, the call is ignored.
         /// </summary>
-        /// <param name="force">If true, cancel any ongoing test threads, otherwise wait for them to complete.</param>
-        public void StopRun(bool force)
-        {
-            if (_engineRunner is null)
-                return; // No test is was even started.
+        public void RequestStop() => _engineRunner?.RequestStop();
 
-            if (!force)
-                _engineRunner.RequestStop();
-            else
+        public void ForcedStop()
+        {
+            if (_engineRunner is not null)
             {
                 _engineRunner.ForcedStop();
 
-                // Frameworks should handle StopRun(true) by cancelling all tests and notifying
+#if USE_WORK_ITEM_TRACKER
+                // Framework drivers should handle ForcedStop() by cancelling all tests and notifying
                 // us of the completion of any tests that were running. However, this feature
                 // may be absent in some frameworks or may be broken and we may not pass on the
                 // notifications needed by some runners. In fact, such a bug is present in the
@@ -225,6 +226,7 @@ public void StopRun(bool force)
 
                     _engineRunner.Unload();
                 }
+#endif
             }
         }
 
@@ -454,9 +456,11 @@ private int CountTests(TestFilter filter)
         /// <returns>A TestEngineResult giving the result of the test execution</returns>
         private TestEngineResult RunTests(ITestEventListener? listener, TestFilter filter)
         {
-            _workItemTracker.Clear();
             _eventDispatcher.Listeners.Clear();
+#if USE_WORK_ITEM_TRACKER
+            _workItemTracker.Clear();
             _eventDispatcher.Listeners.Add(_workItemTracker);
+#endif
 
             if (listener is not null)
                 _eventDispatcher.Listeners.Add(listener);
@@ -530,15 +534,18 @@ private TestEngineResult RunTests(ITestEventListener? listener, TestFilter filte
         private void OnTestRunTimeout(object? sender, ElapsedEventArgs e)
         {
             // Unlikely as it is, let's see if we can stop this cooperatively
-            StopRun(false);
+            RequestStop();
+            // TODO: Should we wait for run complete if we are not using WorkItemTracker?
 
             // We wait for the cooperative stop and do a forced stop if it fails.
             // WaitForCompletion will actually be called twice, once here and
-            // again in StopRun(true), so the wait is doubled but no harm done.
-            // StopRun(True) also calls _workItemTracker to try to fix up any
+            // again in ForcedStop(), so the wait is doubled but no harm done.
+            // ForcedStop() also calls _workItemTracker to try to fix up any
             // in-flight items and produce as informative a result as possible.
+#if USE_WORK_ITEM_TRACKER
             if (!_workItemTracker.WaitForCompletion(WAIT_FOR_CANCEL_TO_COMPLETE))
-                StopRun(true);
+                ForcedStop();
+#endif
         }
 
         private AsyncTestEngineResult RunTestsAsync(ITestEventListener? listener, TestFilter filter)