From d49e808092a6bd304df406f235bd4a09dc6f6e94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Mon, 25 May 2026 23:08:44 +0200 Subject: [PATCH 1/2] Add two-stage Ctrl+C cancellation UX (#5345) The first Ctrl+C still triggers cooperative cancellation, but now prints a hint to the user that pressing Ctrl+C again will force-exit. The second press calls IEnvironment.Exit with TestSessionAborted (3). State transitions use Interlocked to be robust across concurrent presses. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../SimplifiedConsoleOutputDeviceBase.cs | 1 + .../Terminal/TerminalTestReporter.cs | 1 + .../OutputDevice/TerminalOutputDevice.cs | 6 +- .../Resources/PlatformResources.resx | 3 + .../Resources/xlf/PlatformResources.cs.xlf | 5 + .../Resources/xlf/PlatformResources.de.xlf | 5 + .../Resources/xlf/PlatformResources.es.xlf | 5 + .../Resources/xlf/PlatformResources.fr.xlf | 5 + .../Resources/xlf/PlatformResources.it.xlf | 5 + .../Resources/xlf/PlatformResources.ja.xlf | 5 + .../Resources/xlf/PlatformResources.ko.xlf | 5 + .../Resources/xlf/PlatformResources.pl.xlf | 5 + .../Resources/xlf/PlatformResources.pt-BR.xlf | 5 + .../Resources/xlf/PlatformResources.ru.xlf | 5 + .../Resources/xlf/PlatformResources.tr.xlf | 5 + .../xlf/PlatformResources.zh-Hans.xlf | 5 + .../xlf/PlatformResources.zh-Hant.xlf | 5 + .../CTRLPlusCCancellationTokenSource.cs | 38 +++- .../CTRLPlusCCancellationTokenSourceTests.cs | 186 ++++++++++++++++++ 19 files changed, 298 insertions(+), 2 deletions(-) create mode 100644 test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs diff --git a/src/Platform/Microsoft.Testing.Platform/OutputDevice/SimplifiedConsoleOutputDeviceBase.cs b/src/Platform/Microsoft.Testing.Platform/OutputDevice/SimplifiedConsoleOutputDeviceBase.cs index 6f887e2c39..eb0594f3ea 100644 --- a/src/Platform/Microsoft.Testing.Platform/OutputDevice/SimplifiedConsoleOutputDeviceBase.cs +++ b/src/Platform/Microsoft.Testing.Platform/OutputDevice/SimplifiedConsoleOutputDeviceBase.cs @@ -85,6 +85,7 @@ public async Task InitializeAsync() () => { ConsoleLog(PlatformResources.CancellingTestSession); + ConsoleLog(PlatformResources.PressCtrlCAgainToForceExit); return Task.CompletedTask; }).ConfigureAwait(false); diff --git a/src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.cs b/src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.cs index 6001ce8ba1..51b1333ba9 100644 --- a/src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.cs +++ b/src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.cs @@ -167,6 +167,7 @@ public void StartCancelling() { terminal.AppendLine(); terminal.AppendLine(PlatformResources.CancellingTestSession); + terminal.AppendLine(PlatformResources.PressCtrlCAgainToForceExit); terminal.AppendLine(); }); } diff --git a/src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs b/src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs index 24306b1cc2..7dd32ab6d9 100644 --- a/src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs +++ b/src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs @@ -127,7 +127,11 @@ public async Task InitializeAsync() // stdout, corrupting the JSON document. Route a single-line cancellation notice // to stderr instead so the user still gets feedback on Ctrl+C. await _policiesService.RegisterOnAbortCallbackAsync( - () => WriteToStandardErrorAsync(PlatformResources.CancellingTestSession)).ConfigureAwait(false); + async () => + { + await WriteToStandardErrorAsync(PlatformResources.CancellingTestSession).ConfigureAwait(false); + await WriteToStandardErrorAsync(PlatformResources.PressCtrlCAgainToForceExit).ConfigureAwait(false); + }).ConfigureAwait(false); } else { diff --git a/src/Platform/Microsoft.Testing.Platform/Resources/PlatformResources.resx b/src/Platform/Microsoft.Testing.Platform/Resources/PlatformResources.resx index 942c03e448..326d5c36d4 100644 --- a/src/Platform/Microsoft.Testing.Platform/Resources/PlatformResources.resx +++ b/src/Platform/Microsoft.Testing.Platform/Resources/PlatformResources.resx @@ -522,6 +522,9 @@ Read more about Microsoft Testing Platform telemetry: https://aka.ms/testingplat Canceling the test session... + + Press Ctrl+C again to force exit. + Diagnostic file (level '{0}' with async flush): {1} 0 level such as verbose, diff --git a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.cs.xlf b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.cs.xlf index b025a84e67..7d504abfe7 100644 --- a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.cs.xlf +++ b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.cs.xlf @@ -685,6 +685,11 @@ Takes one argument as string in the format <value>[h|m|s] where 'value' is Může mít jenom jeden argument jako řetězec ve formátu <value>[h|m|s], kde value je hodnota datového typu float. + + Press Ctrl+C again to force exit. + Press Ctrl+C again to force exit. + + Process should have exited before we can determine this value Proces měl být ukončen před tím, než jsme mohli určit tuto hodnotu. diff --git a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.de.xlf b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.de.xlf index 4e4540fd19..68ed5a43c5 100644 --- a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.de.xlf +++ b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.de.xlf @@ -685,6 +685,11 @@ Takes one argument as string in the format <value>[h|m|s] where 'value' is Nimmt ein Argument als Zeichenfolge im Format <value>[h|m|s], wobei "value" auf "float" festgelegt ist. + + Press Ctrl+C again to force exit. + Press Ctrl+C again to force exit. + + Process should have exited before we can determine this value Der Prozess hätte beendet werden müssen, bevor dieser Wert ermittelt werden kann diff --git a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.es.xlf b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.es.xlf index 90f8df9670..d945c1128d 100644 --- a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.es.xlf +++ b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.es.xlf @@ -685,6 +685,11 @@ Takes one argument as string in the format <value>[h|m|s] where 'value' is Toma un argumento como cadena con el formato <value>[h|m|s] donde 'value' es float. + + Press Ctrl+C again to force exit. + Press Ctrl+C again to force exit. + + Process should have exited before we can determine this value El proceso debería haberse terminado para poder determinar este valor diff --git a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.fr.xlf b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.fr.xlf index e85b3dd053..a9419d7f93 100644 --- a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.fr.xlf +++ b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.fr.xlf @@ -685,6 +685,11 @@ Takes one argument as string in the format <value>[h|m|s] where 'value' is Prend un argument sous forme de chaîne au format <value>[h|m|s] où « value » est float. + + Press Ctrl+C again to force exit. + Press Ctrl+C again to force exit. + + Process should have exited before we can determine this value Le processus aurait dû s’arrêter avant que nous puissions déterminer cette valeur diff --git a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.it.xlf b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.it.xlf index dab5720fb9..f404a3bc8c 100644 --- a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.it.xlf +++ b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.it.xlf @@ -685,6 +685,11 @@ Takes one argument as string in the format <value>[h|m|s] where 'value' is Acquisisce un argomento come stringa nel formato <value>[h|m|s] dove 'value' è float. + + Press Ctrl+C again to force exit. + Press Ctrl+C again to force exit. + + Process should have exited before we can determine this value Il processo dovrebbe essere terminato prima di poter determinare questo valore diff --git a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ja.xlf b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ja.xlf index 89044004aa..ffcd01a045 100644 --- a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ja.xlf +++ b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ja.xlf @@ -686,6 +686,11 @@ Takes one argument as string in the format <value>[h|m|s] where 'value' is 1 つの引数を文字列として <value>[h|m|s] の形式で使用します。この場合、'value' は float です。 + + Press Ctrl+C again to force exit. + Press Ctrl+C again to force exit. + + Process should have exited before we can determine this value この値を決定する前にプロセスを終了する必要があります diff --git a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ko.xlf b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ko.xlf index e16040049c..0101a25ca9 100644 --- a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ko.xlf +++ b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ko.xlf @@ -685,6 +685,11 @@ Takes one argument as string in the format <value>[h|m|s] where 'value' is 'value'가 float인 <value>[h|m|s] 형식의 문자열로 인수 하나를 사용합니다. + + Press Ctrl+C again to force exit. + Press Ctrl+C again to force exit. + + Process should have exited before we can determine this value 이 값을 결정하려면 프로세스가 종료되어야 합니다. diff --git a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.pl.xlf b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.pl.xlf index 1cdea07349..681e76a9bb 100644 --- a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.pl.xlf +++ b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.pl.xlf @@ -685,6 +685,11 @@ Takes one argument as string in the format <value>[h|m|s] where 'value' is Pobiera jeden argument jako ciąg w formacie <value>[h|m|s], gdzie element „value” ma wartość zmiennoprzecinkową. + + Press Ctrl+C again to force exit. + Press Ctrl+C again to force exit. + + Process should have exited before we can determine this value Proces powinien zakończyć się przed ustaleniem tej wartości diff --git a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.pt-BR.xlf b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.pt-BR.xlf index f4128481d8..f71102b746 100644 --- a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.pt-BR.xlf +++ b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.pt-BR.xlf @@ -685,6 +685,11 @@ Takes one argument as string in the format <value>[h|m|s] where 'value' is Recebe um argumento como cadeia de caracteres no formato <valor>[h|m|s] em que 'valor' é float. + + Press Ctrl+C again to force exit. + Press Ctrl+C again to force exit. + + Process should have exited before we can determine this value O processo deve ter sido encerrado antes que possamos determinar esse valor diff --git a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ru.xlf b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ru.xlf index 781042a36c..6c301cf2ed 100644 --- a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ru.xlf +++ b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ru.xlf @@ -685,6 +685,11 @@ Takes one argument as string in the format <value>[h|m|s] where 'value' is Принимает один аргумент в виде строки в формате <value>[h|m|s], где "value" — число с плавающей точкой. + + Press Ctrl+C again to force exit. + Press Ctrl+C again to force exit. + + Process should have exited before we can determine this value Процесс должен быть завершен, прежде чем мы сможем определить это значение diff --git a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.tr.xlf b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.tr.xlf index f48a8db2a4..8f61d83f7b 100644 --- a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.tr.xlf +++ b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.tr.xlf @@ -685,6 +685,11 @@ Takes one argument as string in the format <value>[h|m|s] where 'value' is Bir bağımsız değişkeni, 'value' değerinin kayan olduğu <value>[h|m|s] biçiminde dize olarak alır. + + Press Ctrl+C again to force exit. + Press Ctrl+C again to force exit. + + Process should have exited before we can determine this value Bu değeri belirleyebilmemiz için süreçten çıkılmış olması gerekir diff --git a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.zh-Hans.xlf b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.zh-Hans.xlf index 0924db6031..5776f942b7 100644 --- a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.zh-Hans.xlf +++ b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.zh-Hans.xlf @@ -685,6 +685,11 @@ Takes one argument as string in the format <value>[h|m|s] where 'value' is 获取一个字符串形式的参数，格式为 <value>[h|m|s]，其中 "value" 为浮点型。 + + Press Ctrl+C again to force exit. + Press Ctrl+C again to force exit. + + Process should have exited before we can determine this value 在我们确定此值之前，流程应该已退出 diff --git a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.zh-Hant.xlf b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.zh-Hant.xlf index 5a35249a2d..ff9e295b07 100644 --- a/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.zh-Hant.xlf +++ b/src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.zh-Hant.xlf @@ -685,6 +685,11 @@ Takes one argument as string in the format <value>[h|m|s] where 'value' is 將一個引數作為字串，格式為 <value>[h|m|s]，其中 'value' 為 float。 + + Press Ctrl+C again to force exit. + Press Ctrl+C again to force exit. + + Process should have exited before we can determine this value 在我們確定此值之前，流程應已結束 diff --git a/src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs b/src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs index 9226f4c862..9ba2c9d672 100644 --- a/src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs +++ b/src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs @@ -8,16 +8,23 @@ namespace Microsoft.Testing.Platform.Services; internal sealed class CTRLPlusCCancellationTokenSource : ITestApplicationCancellationTokenSource, IDisposable { + private const int StateIdle = 0; + private const int StateCancelling = 1; + private const int StateForcing = 2; + private readonly CancellationTokenSource _cancellationTokenSource = new(); + private readonly IEnvironment _environment; private readonly ILogger? _logger; + private int _state = StateIdle; - public CTRLPlusCCancellationTokenSource(IConsole? console = null, ILogger? logger = null) + public CTRLPlusCCancellationTokenSource(IConsole? console = null, ILogger? logger = null, IEnvironment? environment = null) { if (console is not null && !IsCancelKeyPressNotSupported()) { console.CancelKeyPress += OnConsoleCancelKeyPressed; } + _environment = environment ?? new SystemEnvironment(); _logger = logger; } @@ -40,7 +47,36 @@ public CancellationToken CancellationToken private void OnConsoleCancelKeyPressed(object? sender, ConsoleCancelEventArgs e) { + // Suppress the runtime's default Ctrl+C handling so we control the exit code on + // both the first (cooperative) and the second (force-exit) press. e.Cancel = true; + + // If cancellation is already in progress (from a previous Ctrl+C or from another source + // like a timeout), treat this Ctrl+C as a request to force-exit the process. This honors + // the contract advertised next to the "Cancelling..." message: "Press Ctrl+C again to + // force exit.". + if (_cancellationTokenSource.IsCancellationRequested) + { + if (Interlocked.Exchange(ref _state, StateForcing) == StateForcing) + { + // Another Ctrl+C already triggered the force-exit; suppress the duplicate. + return; + } + + // We intentionally do not print an extra "Forcing exit..." message here because the + // IConsole abstraction has no stderr channel and writing to stdout would corrupt the + // JSON document produced by --list-tests json. The user already saw the + // "Press Ctrl+C again to force exit." hint, so the exit itself is the confirmation. + _environment.Exit((int)ExitCode.TestSessionAborted); + return; + } + + if (Interlocked.CompareExchange(ref _state, StateCancelling, StateIdle) != StateIdle) + { + // Another thread already transitioned the state; nothing to do. + return; + } + try { _cancellationTokenSource.Cancel(); diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs new file mode 100644 index 0000000000..fb46106870 --- /dev/null +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs @@ -0,0 +1,186 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.Testing.Platform.Helpers; +using Microsoft.Testing.Platform.Services; + +namespace Microsoft.Testing.Platform.UnitTests; + +[TestClass] +public sealed class CTRLPlusCCancellationTokenSourceTests +{ + [TestMethod] + public void FirstCtrlC_CancelsToken_AndDoesNotExitProcess() + { + var console = new CancelableConsole(); + var environment = new RecordingEnvironment(); + using var source = new CTRLPlusCCancellationTokenSource(console, logger: null, environment); + + console.FireCancelKeyPress(); + + Assert.IsTrue(source.CancellationToken.IsCancellationRequested); + Assert.IsNull(environment.ExitCode, "Environment.Exit must not be called on the first Ctrl+C press."); + } + + [TestMethod] + public void SecondCtrlC_TriggersForceExit_WithTestSessionAbortedExitCode() + { + var console = new CancelableConsole(); + var environment = new RecordingEnvironment(); + using var source = new CTRLPlusCCancellationTokenSource(console, logger: null, environment); + + console.FireCancelKeyPress(); + console.FireCancelKeyPress(); + + Assert.IsTrue(source.CancellationToken.IsCancellationRequested); + Assert.AreEqual((int)ExitCode.TestSessionAborted, environment.ExitCode); + } + + [TestMethod] + public void CtrlC_WhileCancellationAlreadyTriggeredExternally_TriggersForceExit() + { + var console = new CancelableConsole(); + var environment = new RecordingEnvironment(); + using var source = new CTRLPlusCCancellationTokenSource(console, logger: null, environment); + + // Simulate cancellation from another source (timeout, max-failed-tests, etc.). + source.Cancel(); + + console.FireCancelKeyPress(); + + Assert.AreEqual((int)ExitCode.TestSessionAborted, environment.ExitCode); + } + + [TestMethod] + public void RepeatedCtrlC_AfterForceExit_DoesNotCallExitAgain() + { + var console = new CancelableConsole(); + var environment = new RecordingEnvironment(); + using var source = new CTRLPlusCCancellationTokenSource(console, logger: null, environment); + + console.FireCancelKeyPress(); + console.FireCancelKeyPress(); + console.FireCancelKeyPress(); + console.FireCancelKeyPress(); + + Assert.AreEqual(1, environment.ExitCallCount); + Assert.AreEqual((int)ExitCode.TestSessionAborted, environment.ExitCode); + } + + private sealed class CancelableConsole : IConsole + { + public event ConsoleCancelEventHandler? CancelKeyPress; + + public int BufferHeight => int.MaxValue; + + public int BufferWidth => int.MaxValue; + + public int WindowHeight => int.MaxValue; + + public int WindowWidth => int.MaxValue; + + public bool IsOutputRedirected => false; + + public void FireCancelKeyPress() + { + ConsoleCancelEventHandler? handler = CancelKeyPress; + handler?.Invoke(this, CreateConsoleCancelEventArgs()); + } + + public void Clear() => throw new NotImplementedException(); + + public ConsoleColor GetForegroundColor() => ConsoleColor.White; + + public void SetForegroundColor(ConsoleColor color) + { + // do nothing + } + + public void Write(string? value) + { + // do nothing + } + + public void Write(char value) + { + // do nothing + } + + public void WriteLine() + { + // do nothing + } + + public void WriteLine(string? value) + { + // do nothing + } + + // ConsoleCancelEventArgs has no public constructor; use reflection to instantiate it + // for the purposes of the test. + private static ConsoleCancelEventArgs CreateConsoleCancelEventArgs() + { + ConstructorInfo? constructor = typeof(ConsoleCancelEventArgs).GetConstructor( + BindingFlags.Instance | BindingFlags.NonPublic, + binder: null, + types: [typeof(ConsoleSpecialKey)], + modifiers: null); + + Assert.IsNotNull(constructor, "Failed to locate internal ConsoleCancelEventArgs constructor."); + return (ConsoleCancelEventArgs)constructor.Invoke([ConsoleSpecialKey.ControlC]); + } + } + + private sealed class RecordingEnvironment : IEnvironment + { + public int? ExitCode { get; private set; } + + public int ExitCallCount { get; private set; } + + public string CommandLine => string.Empty; + + public string MachineName => string.Empty; + + public string NewLine => Environment.NewLine; + + public int ProcessId => 0; + + public string OsVersion => string.Empty; + +#if NETCOREAPP + public string? ProcessPath => null; +#endif + + public string[] GetCommandLineArgs() => []; + + public string? GetEnvironmentVariable(string name) => null; + + public IDictionary GetEnvironmentVariables() => new Dictionary(); + + public string GetFolderPath(Environment.SpecialFolder folder, Environment.SpecialFolderOption option) => string.Empty; + + public void FailFast(string? message, Exception? exception) + { + // do nothing + } + + public void FailFast(string? message) + { + // do nothing + } + + public void SetEnvironmentVariable(string variable, string? value) + { + // do nothing + } + + public void Exit(int exitCode) + { + ExitCode = exitCode; + ExitCallCount++; + + // The real implementation never returns; ours does, so subsequent presses still + // observe ExitCallCount accurately for the test. + } + } +} From 359b39e9d967065e40537d4aefbdf9e344941a3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Tue, 26 May 2026 09:00:42 +0200 Subject: [PATCH 2/2] Address review: count Ctrl+C presses via state machine, unsubscribe in Dispose, add coverage - Decouple Ctrl+C counting from IsCancellationRequested so external Cancel() (timeout, max-failed-tests, etc.) no longer turns the first user Ctrl+C into a force-exit. The user must always press Ctrl+C twice to force-exit, matching the advertised 'Press Ctrl+C again to force exit.' contract. - Store the subscribed IConsole and unsubscribe CancelKeyPress in Dispose() to prevent the handler from running on a disposed CancellationTokenSource. - Add tests for: AggregateException-from-Cancel logging, concurrent first Ctrl+C contention, null-console boundary, and Dispose unsubscription. - Update the 'external cancel first' test to assert the new (correct) behavior: first user Ctrl+C is cooperative, second forces exit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../CTRLPlusCCancellationTokenSource.cs | 61 ++++++---- .../CTRLPlusCCancellationTokenSourceTests.cs | 108 +++++++++++++++++- 2 files changed, 145 insertions(+), 24 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs b/src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs index 9ba2c9d672..a2c2844895 100644 --- a/src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs +++ b/src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs @@ -15,13 +15,16 @@ internal sealed class CTRLPlusCCancellationTokenSource : ITestApplicationCancell private readonly CancellationTokenSource _cancellationTokenSource = new(); private readonly IEnvironment _environment; private readonly ILogger? _logger; + private readonly IConsole? _subscribedConsole; private int _state = StateIdle; + private int _disposed; public CTRLPlusCCancellationTokenSource(IConsole? console = null, ILogger? logger = null, IEnvironment? environment = null) { if (console is not null && !IsCancelKeyPressNotSupported()) { console.CancelKeyPress += OnConsoleCancelKeyPressed; + _subscribedConsole = console; } _environment = environment ?? new SystemEnvironment(); @@ -51,44 +54,56 @@ private void OnConsoleCancelKeyPressed(object? sender, ConsoleCancelEventArgs e) // both the first (cooperative) and the second (force-exit) press. e.Cancel = true; - // If cancellation is already in progress (from a previous Ctrl+C or from another source - // like a timeout), treat this Ctrl+C as a request to force-exit the process. This honors - // the contract advertised next to the "Cancelling..." message: "Press Ctrl+C again to - // force exit.". - if (_cancellationTokenSource.IsCancellationRequested) + // The state machine counts user Ctrl+C presses *independently* of external cancellation + // sources (timeout, max-failed-tests, explicit Cancel()). This honors the contract + // advertised next to the "Cancelling..." message ("Press Ctrl+C again to force exit.") + // regardless of who initiated the cancellation: the user must always press Ctrl+C twice + // to force-exit. + if (Interlocked.CompareExchange(ref _state, StateCancelling, StateIdle) == StateIdle) { - if (Interlocked.Exchange(ref _state, StateForcing) == StateForcing) + // First user Ctrl+C: cooperative cancellation. If the token was already cancelled + // by an external source this is effectively a no-op, but we still transitioned the + // state so the next press goes to force-exit. + try { - // Another Ctrl+C already triggered the force-exit; suppress the duplicate. - return; + _cancellationTokenSource.Cancel(); + } + catch (AggregateException ex) + { + _logger?.LogWarning($"Exception during CTRLPlusCCancellationTokenSource cancel:\n{ex}"); } - // We intentionally do not print an extra "Forcing exit..." message here because the - // IConsole abstraction has no stderr channel and writing to stdout would corrupt the - // JSON document produced by --list-tests json. The user already saw the - // "Press Ctrl+C again to force exit." hint, so the exit itself is the confirmation. - _environment.Exit((int)ExitCode.TestSessionAborted); return; } - if (Interlocked.CompareExchange(ref _state, StateCancelling, StateIdle) != StateIdle) + // Second user Ctrl+C: force-exit. We intentionally do not print an extra + // "Forcing exit..." message here because the IConsole abstraction has no stderr channel + // and writing to stdout would corrupt the JSON document produced by --list-tests json. + // The user already saw the "Press Ctrl+C again to force exit." hint, so the exit itself + // is the confirmation. Any subsequent presses are suppressed by the StateForcing guard. + if (Interlocked.CompareExchange(ref _state, StateForcing, StateCancelling) == StateCancelling) { - // Another thread already transitioned the state; nothing to do. - return; + _environment.Exit((int)ExitCode.TestSessionAborted); } + } - try + public void Dispose() + { + if (Interlocked.Exchange(ref _disposed, 1) != 0) { - _cancellationTokenSource.Cancel(); + return; } - catch (AggregateException ex) + + // We stored the console reference only when subscription was actually performed + // (i.e. when CancelKeyPress is supported on this platform), so we can safely call -= + // on the same supported-platform paths. + if (_subscribedConsole is not null && !IsCancelKeyPressNotSupported()) { - _logger?.LogWarning($"Exception during CTRLPlusCCancellationTokenSource cancel:\n{ex}"); + _subscribedConsole.CancelKeyPress -= OnConsoleCancelKeyPressed; } - } - public void Dispose() - => _cancellationTokenSource.Dispose(); + _cancellationTokenSource.Dispose(); + } public void Cancel() => _cancellationTokenSource.Cancel(); diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs index fb46106870..3eb9b00b4c 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using Microsoft.Testing.Platform.Helpers; +using Microsoft.Testing.Platform.Logging; using Microsoft.Testing.Platform.Services; namespace Microsoft.Testing.Platform.UnitTests; @@ -37,7 +38,7 @@ public void SecondCtrlC_TriggersForceExit_WithTestSessionAbortedExitCode() } [TestMethod] - public void CtrlC_WhileCancellationAlreadyTriggeredExternally_TriggersForceExit() + public void CtrlC_AfterExternalCancel_DoesNotForceExit_ButSecondCtrlCDoes() { var console = new CancelableConsole(); var environment = new RecordingEnvironment(); @@ -46,8 +47,13 @@ public void CtrlC_WhileCancellationAlreadyTriggeredExternally_TriggersForceExit( // Simulate cancellation from another source (timeout, max-failed-tests, etc.). source.Cancel(); + // The user has not yet seen the "Press Ctrl+C again to force exit." hint, so the first + // user Ctrl+C must not force-exit — it should be treated as the first cooperative press. console.FireCancelKeyPress(); + Assert.IsNull(environment.ExitCode, "First user Ctrl+C must not force-exit even when cancellation was already requested externally."); + // The second user Ctrl+C should then force-exit. + console.FireCancelKeyPress(); Assert.AreEqual((int)ExitCode.TestSessionAborted, environment.ExitCode); } @@ -67,6 +73,78 @@ public void RepeatedCtrlC_AfterForceExit_DoesNotCallExitAgain() Assert.AreEqual((int)ExitCode.TestSessionAborted, environment.ExitCode); } + [TestMethod] + public void FirstCtrlC_WhenCancelCallbackThrows_LogsWarningAndSuppressesException() + { + var console = new CancelableConsole(); + var environment = new RecordingEnvironment(); + var logger = new RecordingLogger(); + using var source = new CTRLPlusCCancellationTokenSource(console, logger, environment); + + // Registering a throwing callback on the token causes CancellationTokenSource.Cancel() + // to throw AggregateException, exercising the catch/log path in OnConsoleCancelKeyPressed. + source.CancellationToken.Register(() => throw new InvalidOperationException("boom")); + + console.FireCancelKeyPress(); + + Assert.IsTrue(source.CancellationToken.IsCancellationRequested); + Assert.IsNull(environment.ExitCode, "First Ctrl+C must not force-exit even when the cancel callback throws."); + Assert.AreEqual(1, logger.WarningCount, "The AggregateException must be logged as a warning."); + Assert.IsNotNull(logger.LastWarning); + Assert.Contains("CTRLPlusCCancellationTokenSource cancel", logger.LastWarning!); + } + + [TestMethod] + public void ConcurrentFirstCtrlC_OnlyTransitionsStateOnce_AndAllPressesCombinedNeverExitMoreThanOnce() + { + var console = new CancelableConsole(); + var environment = new RecordingEnvironment(); + using var source = new CTRLPlusCCancellationTokenSource(console, logger: null, environment); + + // Registering a callback on the token lets us count how many times the underlying + // CancellationTokenSource.Cancel() was actually invoked. The state machine guarantees + // only the first thread that wins the StateIdle -> StateCancelling transition calls + // Cancel(), so the callback must fire exactly once even under heavy contention. + int cancelCallbackInvocations = 0; + source.CancellationToken.Register(() => Interlocked.Increment(ref cancelCallbackInvocations)); + + Parallel.For(0, 16, _ => console.FireCancelKeyPress()); + + Assert.IsTrue(source.CancellationToken.IsCancellationRequested); + Assert.AreEqual(1, cancelCallbackInvocations, "Only the thread that wins the state transition must call Cancel()."); + Assert.AreEqual(1, environment.ExitCallCount, "Across many concurrent presses Exit() must be called at most once."); + } + + [TestMethod] + public void Constructor_WithNullConsole_DoesNotCrash_AndCancelStillWorks() + { + var environment = new RecordingEnvironment(); + using var source = new CTRLPlusCCancellationTokenSource(console: null, logger: null, environment); + + source.Cancel(); + + Assert.IsTrue(source.CancellationToken.IsCancellationRequested); + Assert.IsNull(environment.ExitCode, "Without a console there is no Ctrl+C handler and Exit must never be called."); + } + + [TestMethod] + public void Dispose_UnsubscribesCancelKeyPressHandler() + { + var console = new CancelableConsole(); + var environment = new RecordingEnvironment(); + var source = new CTRLPlusCCancellationTokenSource(console, logger: null, environment); + + source.Dispose(); + + // After disposal the handler must be detached so a late Ctrl+C cannot touch the disposed + // CancellationTokenSource (which would throw ObjectDisposedException). + Assert.IsFalse(console.HasCancelKeyPressSubscribers); + + // Firing the event after dispose must be a no-op. + console.FireCancelKeyPress(); + Assert.IsNull(environment.ExitCode); + } + private sealed class CancelableConsole : IConsole { public event ConsoleCancelEventHandler? CancelKeyPress; @@ -81,6 +159,8 @@ private sealed class CancelableConsole : IConsole public bool IsOutputRedirected => false; + public bool HasCancelKeyPressSubscribers => CancelKeyPress is not null; + public void FireCancelKeyPress() { ConsoleCancelEventHandler? handler = CancelKeyPress; @@ -183,4 +263,30 @@ public void Exit(int exitCode) // observe ExitCallCount accurately for the test. } } + + private sealed class RecordingLogger : ILogger + { + private int _warningCount; + + public int WarningCount => _warningCount; + + public string? LastWarning { get; private set; } + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, TState state, Exception? exception, Func formatter) + { + if (logLevel == LogLevel.Warning) + { + Interlocked.Increment(ref _warningCount); + LastWarning = formatter(state, exception); + } + } + + public Task LogAsync(LogLevel logLevel, TState state, Exception? exception, Func formatter) + { + Log(logLevel, state, exception, formatter); + return Task.CompletedTask; + } + } }