Modify ITestEngineResult interface splitting StopRun into two methods

Author: CharliePoole

src/NUnitEngine/nunit.engine.core.tests/Runners/DirectTestRunnerTests.cs

@@ -108,7 +108,7 @@ public void StopRun_Passes_Along_NUnitEngineException()
             _driver.When(x => x.StopRun(Arg.Any<bool>()))
                 .Do(x => { throw new NUnitEngineException("Message"); });
 
-            var ex = Assert.Throws<NUnitEngineException>(() => _directTestRunner.StopRun(true));
+            var ex = Assert.Throws<NUnitEngineException>(() => _directTestRunner.ForcedStop());
             Assert.That(ex.Message, Is.EqualTo("Message"));
         }
 
@@ -118,7 +118,7 @@ public void StopRun_Throws_NUnitEngineException()
             _driver.When(x => x.StopRun(Arg.Any<bool>()))
                 .Do(x => { throw new ArgumentException("Message"); });
 
-            var ex = Assert.Throws<NUnitEngineException>(() => _directTestRunner.StopRun(true));
+            var ex = Assert.Throws<NUnitEngineException>(() => _directTestRunner.ForcedStop());
             Assert.That(ex.InnerException is ArgumentException);
             Assert.That(ex.InnerException.Message, Is.EqualTo("Message"));
         }

src/NUnitEngine/nunit.engine.core/Communication/Messages/MessageCode.cs

@@ -0,0 +1,24 @@
+// Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt
+
+namespace NUnit.Engine.Communication.Messages
+{
+    public class MessageCode
+    {   // All length 4
+        public const string StartAgent = "STRT"; // not used
+        public const string StopAgent = "EXIT";
+        public const string CreateRunner = "RUNR";
+
+        public const string LoadCommand = "LOAD";
+        public const string ReloadCommand = "RELD";
+        public const string UnloadCommand = "UNLD";
+        public const string ExploreCommand = "XPLR";
+        public const string CountCasesCommand = "CNTC";
+        public const string RunCommand = "RSYN";
+        public const string RunAsyncCommand = "RASY";
+        public const string RequestStopCommand = "STOP";
+        public const string ForcedStopCommand = "ABRT";
+
+        public const string ProgressReport = "PROG";
+        public const string CommandResult = "RSLT";
+    }
+}

src/NUnitEngine/nunit.engine.core/Communication/Transports/Remoting/TestAgentRemotingTransport.cs

@@ -172,14 +172,16 @@ public AsyncTestEngineResult RunAsync(ITestEventListener listener, TestFilter fi
         }
 
         /// <summary>
-        /// Cancel the ongoing test run. If no  test is running, the call is ignored.
+        /// Request the current test run to stop. If no tests are 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 (_runner != null)
-                _runner.StopRun(force);
-        }
+        public void RequestStop() => _runner?.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() => _runner?.ForcedStop();
 
         #endregion
 

src/NUnitEngine/nunit.engine.core/ITestEngineRunner.cs

@@ -57,11 +57,16 @@ public interface ITestEngineRunner : IDisposable
         AsyncTestEngineResult RunAsync(ITestEventListener listener, TestFilter filter);
 
         /// <summary>
-        /// Cancel the current test run. If no test is running,
+        /// Request the current test run to stop. If no tests are running,
         /// the call is ignored.
         /// </summary>
-        /// <param name="force">If true, force a stop by cancelling threads if necessary.</param>
-        void StopRun(bool force);
+        void 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>
+        void ForcedStop();
 
         /// <summary>
         /// Explore a loaded TestPackage and return information about

src/NUnitEngine/nunit.engine.core/Runners/AbstractTestRunner.cs

@@ -104,10 +104,16 @@ protected virtual AsyncTestEngineResult RunTestsAsync(ITestEventListener listene
         }
 
         /// <summary>
-        /// Cancel the ongoing test run. If no  test is running, the call is ignored.
+        /// Request the current test run to stop. If no tests are running,
+        /// the call is ignored.
         /// </summary>
-        /// <param name="force">If true, cancel any ongoing test threads, otherwise wait for them to complete.</param>
-        public abstract void StopRun(bool force);
+        public abstract void 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 abstract void ForcedStop();
 
         /// <summary>
         /// Explores the TestPackage and returns information about

src/NUnitEngine/nunit.engine.core/Runners/AggregatingTestRunner.cs

@@ -191,13 +191,23 @@ private void RunTestsInParallel(ITestEventListener listener, TestFilter filter,
         }
 
         /// <summary>
-        /// Cancel the ongoing test run. If no  test is running, the call is ignored.
+        /// Request the current test run to stop. If no tests are running,
+        /// the call is ignored.
         /// </summary>
-        /// <param name="force">If true, cancel any ongoing test threads, otherwise wait for them to complete.</param>
-        public override void StopRun(bool force)
+        public override void RequestStop()
         {
             foreach (var runner in Runners)
-                runner.StopRun(force);
+                runner.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 override void ForcedStop()
+        {
+            foreach (var runner in Runners)
+                runner.ForcedStop();
         }
 
         protected override void Dispose(bool disposing)

src/NUnitEngine/nunit.engine.core/Runners/DirectTestRunner.cs

@@ -209,10 +209,18 @@ protected override TestEngineResult RunTests(ITestEventListener listener, TestFi
         }
 
         /// <summary>
-        /// Cancel the ongoing test run. If no  test is running, the call is ignored.
+        /// Request the current test run to stop. If no tests are running,
+        /// the call is ignored.
         /// </summary>
-        /// <param name="force">If true, cancel any ongoing test threads, otherwise wait for them to complete.</param>
-        public override void StopRun(bool force)
+        public override void RequestStop() => StopRun(false);
+
+        /// <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 override void ForcedStop() => StopRun(true);
+
+        private void StopRun(bool force)
         {
             EnsurePackageIsLoaded();
 

src/NUnitEngine/nunit.engine.core/Runners/NotRunnableTestRunner.cs

@@ -80,9 +80,17 @@ AsyncTestEngineResult ITestEngineRunner.RunAsync(ITestEventListener listener, Te
             throw new NotImplementedException();
         }
 
-        void ITestEngineRunner.StopRun(bool force)
-        {
-        }
+        /// <summary>
+        /// Request the current test run to stop. If no tests are running,
+        /// the call is ignored.
+        /// </summary>
+        void ITestEngineRunner.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>
+        void ITestEngineRunner.ForcedStop() { }
 
         TestEngineResult ITestEngineRunner.Explore(TestFilter filter)
         {

src/NUnitEngine/nunit.engine/Communication/Transports/Tcp/TestAgentTcpProxy.cs

@@ -81,10 +81,9 @@ public AsyncTestEngineResult RunAsync(ITestEventListener listener, TestFilter fi
             //return new AsyncTestEngineResult();
         }
 
-        public void StopRun(bool force)
-        {
-            SendCommandMessage("StopRun", force);
-        }
+        public void RequestStop() => SendCommandMessage(MessageCode.RequestStopCommand);
+
+        public void ForcedStop() => SendCommandMessage(MessageCode.ForcedStopCommand);
 
         public TestEngineResult Explore(TestFilter filter)
         {

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

@@ -169,10 +169,12 @@ public ITestRun RunAsync(ITestEventListener listener, TestFilter filter)
         /// <param name="force">If true, cancel any ongoing test threads, otherwise wait for them to complete.</param>
         public void StopRun(bool force)
         {
-            _engineRunner.StopRun(force);
-
-            if (force)
+            if (!force)
+                _engineRunner.RequestStop();
+            else
             {
+                _engineRunner.ForcedStop();
+
                 // Frameworks should handle StopRun(true) 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