Address feedback

src/Platform/Microsoft.Testing.Platform/CommandLine/MaxFailedTestsCommandLineOptionsProvider.cs

@@ -40,10 +40,10 @@ public Task<ValidationResult> ValidateOptionArgumentsAsync(CommandLineOption com
         if (commandOption.Name == MaxFailedTestsOptionKey)
         {
             string arg = arguments[0];
-            // We consider --maximum-failed-tests 0 as valid.
-            // The idea is that we stop the execution when we *exceed* the max failed tests, not when *reach*.
-            // So zero means, stop execution on the first failure.
-            return int.TryParse(arg, out int maxFailedTestsResult) && maxFailedTestsResult >= 0
+            // We consider --maximum-failed-tests 0 as invalid.
+            // The idea is that we stop the execution when we *reach* the max failed tests, not when *exceed*.
+            // So the value 1 means, stop execution on the first failure.
+            return int.TryParse(arg, out int maxFailedTestsResult) && maxFailedTestsResult > 0
                 ? ValidationResult.ValidTask
                 : ValidationResult.InvalidTask(string.Format(CultureInfo.InvariantCulture, PlatformResources.MaxFailedTestsMustBePositive, arg));
         }

src/Platform/Microsoft.Testing.Platform/Extensions/AbortForMaxFailedTestsExtension.cs

@@ -28,7 +28,7 @@ public AbortForMaxFailedTestsExtension(
     {
         if (commandLineOptions.TryGetOptionArgumentList(MaxFailedTestsCommandLineOptionsProvider.MaxFailedTestsOptionKey, out string[]? args) &&
             int.TryParse(args[0], out int maxFailedTests) &&
-            maxFailedTests >= 0)
+            maxFailedTests > 0)
         {
             _maxFailedTests = maxFailedTests;
         }
@@ -65,7 +65,7 @@ public async Task ConsumeAsync(IDataProducer dataProducer, IData value, Cancella
 
         TestNodeStateProperty testNodeStateProperty = node.TestNode.Properties.Single<TestNodeStateProperty>();
         if (TestNodePropertiesCategories.WellKnownTestNodeTestRunOutcomeFailedProperties.Any(t => t == testNodeStateProperty.GetType()) &&
-            ++_failCount > _maxFailedTests.Value &&
+            ++_failCount >= _maxFailedTests.Value &&
             // If already triggered, don't do it again.
             !_policiesService.IsMaxFailedTestsTriggered)
         {

src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs

@@ -91,13 +91,6 @@ public TerminalOutputDevice(ITestApplicationCancellationTokenSource testApplicat
         _clock = clock;
         _policiesService = policiesService;
 
-        policiesService.RegisterOnAbortCallback(
-            () =>
-            {
-                _terminalTestReporter?.StartCancelling();
-                return Task.CompletedTask;
-            });
-
         if (_runtimeFeature.IsDynamicCodeSupported)
         {
 #if !NETCOREAPP
@@ -120,8 +113,15 @@ public TerminalOutputDevice(ITestApplicationCancellationTokenSource testApplicat
         }
     }
 
-    public Task InitializeAsync()
+    public async Task InitializeAsync()
     {
+        await _policiesService.RegisterOnAbortCallbackAsync(
+            () =>
+            {
+                _terminalTestReporter?.StartCancelling();
+                return Task.CompletedTask;
+            });
+
         if (_fileLoggerInformation is not null)
         {
             _logger = _loggerFactory.CreateLogger(GetType().ToString());
@@ -172,8 +172,6 @@ public Task InitializeAsync()
             UseAnsi = !noAnsi,
             ShowProgress = shouldShowProgress,
         });
-
-        return Task.CompletedTask;
     }
 
     private string GetShortArchitecture(string runtimeIdentifier)
@@ -589,15 +587,13 @@ public async Task ConsumeAsync(IDataProducer dataProducer, IData value, Cancella
     public void Dispose()
         => _terminalTestReporter?.Dispose();
 
-    public Task HandleProcessRoleAsync(TestProcessRole processRole)
+    public async Task HandleProcessRoleAsync(TestProcessRole processRole)
     {
         if (processRole == TestProcessRole.TestHost)
         {
-            _policiesService.RegisterOnMaxFailedTestsCallback(
+            await _policiesService.RegisterOnMaxFailedTestsCallbackAsync(
                 async (maxFailedTests, _) => await DisplayAsync(
                     this, new TextOutputDeviceData(string.Format(CultureInfo.InvariantCulture, PlatformResources.ReachedMaxFailedTestsMessage, maxFailedTests))));
         }
-
-        return Task.CompletedTask;
     }
 }

src/Platform/Microsoft.Testing.Platform/Services/IStopPoliciesService.cs

@@ -9,9 +9,9 @@ internal interface IStopPoliciesService
 
     bool IsAbortTriggered { get; }
 
-    void RegisterOnMaxFailedTestsCallback(Func<int, CancellationToken, Task> callback);
+    Task RegisterOnMaxFailedTestsCallbackAsync(Func<int, CancellationToken, Task> callback);
 
-    void RegisterOnAbortCallback(Func<Task> callback);
+    Task RegisterOnAbortCallbackAsync(Func<Task> callback);
 
     Task ExecuteMaxFailedTestsCallbacksAsync(int maxFailedTests, CancellationToken cancellationToken);
 

src/Platform/Microsoft.Testing.Platform/Services/StopPoliciesService.cs

@@ -9,14 +9,21 @@ namespace Microsoft.Testing.Platform.Services;
 
 internal sealed class StopPoliciesService : IStopPoliciesService
 {
-    public StopPoliciesService(ITestApplicationCancellationTokenSource testApplicationCancellationTokenSource) =>
+    private readonly ITestApplicationCancellationTokenSource _testApplicationCancellationTokenSource;
+
+    private BlockingCollection<Func<int, CancellationToken, Task>>? _maxFailedTestsCallbacks;
+    private BlockingCollection<Func<Task>>? _abortCallbacks;
+    private int _lastMaxFailedTests;
+
+    public StopPoliciesService(ITestApplicationCancellationTokenSource testApplicationCancellationTokenSource)
+    {
+        _testApplicationCancellationTokenSource = testApplicationCancellationTokenSource;
+
 #pragma warning disable VSTHRD101 // Avoid unsupported async delegates
         // Note: If cancellation already requested, Register will still invoke the callback.
         testApplicationCancellationTokenSource.CancellationToken.Register(async () => await ExecuteAbortCallbacksAsync());
 #pragma warning restore VSTHRD101 // Avoid unsupported async delegates
-
-    private BlockingCollection<Func<int, CancellationToken, Task>>? _maxFailedTestsCallbacks;
-    private BlockingCollection<Func<Task>>? _abortCallbacks;
+    }
 
     internal TestProcessRole? ProcessRole { get; set; }
 
@@ -29,6 +36,7 @@ private static void RegisterCallback<T>(ref BlockingCollection<T>? callbacks, T
 
     public async Task ExecuteMaxFailedTestsCallbacksAsync(int maxFailedTests, CancellationToken cancellationToken)
     {
+        _lastMaxFailedTests = maxFailedTests;
         IsMaxFailedTestsTriggered = true;
         if (_maxFailedTestsCallbacks is null)
         {
@@ -60,16 +68,28 @@ public async Task ExecuteAbortCallbacksAsync()
         }
     }
 
-    public void RegisterOnMaxFailedTestsCallback(Func<int, CancellationToken, Task> callback)
+    public async Task RegisterOnMaxFailedTestsCallbackAsync(Func<int, CancellationToken, Task> callback)
     {
         if (ProcessRole != TestProcessRole.TestHost)
         {
             throw ApplicationStateGuard.Unreachable();
         }
 
+        if (IsMaxFailedTestsTriggered)
+        {
+            await callback(_lastMaxFailedTests, _testApplicationCancellationTokenSource.CancellationToken);
+        }
+
         RegisterCallback(ref _maxFailedTestsCallbacks, callback);
     }
 
-    public void RegisterOnAbortCallback(Func<Task> callback)
-        => RegisterCallback(ref _abortCallbacks, callback);
+    public async Task RegisterOnAbortCallbackAsync(Func<Task> callback)
+    {
+        if (IsAbortTriggered)
+        {
+            await callback();
+        }
+
+        RegisterCallback(ref _abortCallbacks, callback);
+    }
 }