From f134a13a9638f9bf357d4dc377cbcee4bda3bb78 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Tue, 10 Dec 2024 15:13:56 +0100 Subject: [PATCH] Address feedback --- ...axFailedTestsCommandLineOptionsProvider.cs | 8 ++--- .../AbortForMaxFailedTestsExtension.cs | 4 +-- .../OutputDevice/TerminalOutputDevice.cs | 24 ++++++------- .../Services/IStopPoliciesService.cs | 4 +-- .../Services/StopPoliciesService.cs | 34 +++++++++++++++---- 5 files changed, 45 insertions(+), 29 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Platform/CommandLine/MaxFailedTestsCommandLineOptionsProvider.cs b/src/Platform/Microsoft.Testing.Platform/CommandLine/MaxFailedTestsCommandLineOptionsProvider.cs index e677ef8d03..e81cc907b7 100644 --- a/src/Platform/Microsoft.Testing.Platform/CommandLine/MaxFailedTestsCommandLineOptionsProvider.cs +++ b/src/Platform/Microsoft.Testing.Platform/CommandLine/MaxFailedTestsCommandLineOptionsProvider.cs @@ -40,10 +40,10 @@ public Task 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)); } diff --git a/src/Platform/Microsoft.Testing.Platform/Extensions/AbortForMaxFailedTestsExtension.cs b/src/Platform/Microsoft.Testing.Platform/Extensions/AbortForMaxFailedTestsExtension.cs index 3b0765e082..78e4999b3c 100644 --- a/src/Platform/Microsoft.Testing.Platform/Extensions/AbortForMaxFailedTestsExtension.cs +++ b/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(); if (TestNodePropertiesCategories.WellKnownTestNodeTestRunOutcomeFailedProperties.Any(t => t == testNodeStateProperty.GetType()) && - ++_failCount > _maxFailedTests.Value && + ++_failCount >= _maxFailedTests.Value && // If already triggered, don't do it again. !_policiesService.IsMaxFailedTestsTriggered) { diff --git a/src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs b/src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs index 8ad4b0ea2f..b50f9aad31 100644 --- a/src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs +++ b/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; } } diff --git a/src/Platform/Microsoft.Testing.Platform/Services/IStopPoliciesService.cs b/src/Platform/Microsoft.Testing.Platform/Services/IStopPoliciesService.cs index d2b49d99d6..d82def8818 100644 --- a/src/Platform/Microsoft.Testing.Platform/Services/IStopPoliciesService.cs +++ b/src/Platform/Microsoft.Testing.Platform/Services/IStopPoliciesService.cs @@ -9,9 +9,9 @@ internal interface IStopPoliciesService bool IsAbortTriggered { get; } - void RegisterOnMaxFailedTestsCallback(Func callback); + Task RegisterOnMaxFailedTestsCallbackAsync(Func callback); - void RegisterOnAbortCallback(Func callback); + Task RegisterOnAbortCallbackAsync(Func callback); Task ExecuteMaxFailedTestsCallbacksAsync(int maxFailedTests, CancellationToken cancellationToken); diff --git a/src/Platform/Microsoft.Testing.Platform/Services/StopPoliciesService.cs b/src/Platform/Microsoft.Testing.Platform/Services/StopPoliciesService.cs index e639e30f9b..fdd0b5f032 100644 --- a/src/Platform/Microsoft.Testing.Platform/Services/StopPoliciesService.cs +++ b/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>? _maxFailedTestsCallbacks; + private BlockingCollection>? _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>? _maxFailedTestsCallbacks; - private BlockingCollection>? _abortCallbacks; + } internal TestProcessRole? ProcessRole { get; set; } @@ -29,6 +36,7 @@ private static void RegisterCallback(ref BlockingCollection? 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 callback) + public async Task RegisterOnMaxFailedTestsCallbackAsync(Func callback) { if (ProcessRole != TestProcessRole.TestHost) { throw ApplicationStateGuard.Unreachable(); } + if (IsMaxFailedTestsTriggered) + { + await callback(_lastMaxFailedTests, _testApplicationCancellationTokenSource.CancellationToken); + } + RegisterCallback(ref _maxFailedTestsCallbacks, callback); } - public void RegisterOnAbortCallback(Func callback) - => RegisterCallback(ref _abortCallbacks, callback); + public async Task RegisterOnAbortCallbackAsync(Func callback) + { + if (IsAbortTriggered) + { + await callback(); + } + + RegisterCallback(ref _abortCallbacks, callback); + } }