From badf0a29a0513b2565cfbe94dbd370194745e839 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 18:41:14 +0000 Subject: [PATCH 1/4] fix: exit through the normal shutdown path on Ctrl+C [patch] UIApplication hid the cursor on startup and only restored it in RunAsync's finally block. Ctrl+C never reached that block: the runtime treats it as a process interrupt and terminated the process first, so the user was left with an invisible cursor in their terminal after the process exited. Take Ctrl+C and SIGTERM for the duration of a run through a new ConsoleInterruptSource, which cancels the runtime's default "terminate now" behaviour and calls Shutdown() instead, letting the existing teardown restore the terminal. The registration is released as the run ends, so a second Ctrl+C during teardown still gets the default behaviour. Cancellation also had to reach a blocked read. ProcessInputAsync awaited the provider directly, and a provider parked in Console.ReadKey does not observe the token, so a shutdown request only took effect once the user pressed an unrelated key. The read is now abandoned when the token fires, which fixes the same latency for the CancellationToken passed to RunAsync. The interrupt source is an internal seam rather than a public contract: the real signals cannot be raised from a test without signalling the test host. UIApplicationTests was an empty file and now covers the run lifecycle -- every wait in it is bounded, because the behaviour under test is an application that fails to notice a shutdown request and an unbounded wait would hang CI on a regression instead of reporting one. Fixes #110 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01671tcpA4zkfbgPcJm8cTsB --- TUI.Core/Contracts/IInterruptSource.cs | 20 ++ TUI.Core/Services/ConsoleInterruptSource.cs | 82 ++++++++ TUI.Core/Services/SpectreConsoleProvider.cs | 4 +- TUI.Core/Services/UIApplication.cs | 39 +++- TUI.Test/BlockingConsoleProvider.cs | 77 ++++++++ TUI.Test/FakeInterruptSource.cs | 49 +++++ TUI.Test/UIApplicationTests.cs | 197 ++++++++++++++++++++ 7 files changed, 466 insertions(+), 2 deletions(-) create mode 100644 TUI.Core/Contracts/IInterruptSource.cs create mode 100644 TUI.Core/Services/ConsoleInterruptSource.cs create mode 100644 TUI.Test/BlockingConsoleProvider.cs create mode 100644 TUI.Test/FakeInterruptSource.cs diff --git a/TUI.Core/Contracts/IInterruptSource.cs b/TUI.Core/Contracts/IInterruptSource.cs new file mode 100644 index 0000000..10c5b5d --- /dev/null +++ b/TUI.Core/Contracts/IInterruptSource.cs @@ -0,0 +1,20 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.TUI.Core.Contracts; + +/// +/// Defines the contract for a source of process interrupt signals such as Ctrl+C and SIGTERM +/// +/// +/// The real source is the process itself, which a test cannot signal without terminating the test +/// run. This seam lets the application's interrupt handling be exercised directly. +/// +internal interface IInterruptSource +{ + /// + /// Registers a callback to invoke when an interrupt signal arrives + /// + /// The callback to invoke + /// A registration that unhooks the callback when disposed + public IDisposable Register(Action onInterrupt); +} diff --git a/TUI.Core/Services/ConsoleInterruptSource.cs b/TUI.Core/Services/ConsoleInterruptSource.cs new file mode 100644 index 0000000..bfa519d --- /dev/null +++ b/TUI.Core/Services/ConsoleInterruptSource.cs @@ -0,0 +1,82 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.TUI.Core.Services; + +using System.Runtime.InteropServices; + +using ktsu.TUI.Core.Contracts; + +/// +/// An backed by the process's real interrupt signals +/// +/// +/// Ctrl+C is taken through rather than by setting +/// , because the latter only delivers the key while the +/// application happens to be inside a blocking read, whereas the event is observed wherever the +/// application is. SIGTERM is taken for the same reason: left alone, either signal ends the +/// process before the application can put back the terminal state it changed. +/// +internal sealed class ConsoleInterruptSource : IInterruptSource +{ + /// + public IDisposable Register(Action onInterrupt) + { + Ensure.NotNull(onInterrupt); + + return new Registration(onInterrupt); + } + + /// + /// Holds the signal hooks for one call and unhooks them on disposal + /// + private sealed class Registration : IDisposable + { + private readonly ConsoleCancelEventHandler _cancelKeyPress; + private readonly PosixSignalRegistration? _sigTerm; + private bool _disposed; + + internal Registration(Action onInterrupt) + { + _cancelKeyPress = (_, e) => + { + // Cancel the runtime's default "terminate now" behaviour so the application + // shuts down through its normal path and gets to restore the terminal. + e.Cancel = true; + onInterrupt(); + }; + + Console.CancelKeyPress += _cancelKeyPress; + _sigTerm = TryRegisterSigTerm(onInterrupt); + } + + /// + public void Dispose() + { + if (_disposed) + { + return; + } + + _disposed = true; + Console.CancelKeyPress -= _cancelKeyPress; + _sigTerm?.Dispose(); + } + + private static PosixSignalRegistration? TryRegisterSigTerm(Action onInterrupt) + { + try + { + return PosixSignalRegistration.Create(PosixSignal.SIGTERM, context => + { + context.Cancel = true; + onInterrupt(); + }); + } + catch (PlatformNotSupportedException) + { + // Nothing to do: Ctrl+C is still handled, which is the common case. + return null; + } + } + } +} diff --git a/TUI.Core/Services/SpectreConsoleProvider.cs b/TUI.Core/Services/SpectreConsoleProvider.cs index 1370e9c..330423a 100644 --- a/TUI.Core/Services/SpectreConsoleProvider.cs +++ b/TUI.Core/Services/SpectreConsoleProvider.cs @@ -74,7 +74,9 @@ public async Task ReadInputAsync() { ConsoleKeyInfo keyInfo = Console.ReadKey(true); - // Handle special cases + // Handle special cases. Ctrl+C normally arrives as an interrupt signal rather than as + // a key, and UIApplication handles it there; this branch only fires for a host that + // has set Console.TreatControlCAsInput. if (keyInfo.Key == ConsoleKey.Escape || (keyInfo.Key == ConsoleKey.C && keyInfo.Modifiers.HasFlag(ConsoleModifiers.Control))) { diff --git a/TUI.Core/Services/UIApplication.cs b/TUI.Core/Services/UIApplication.cs index 54d2502..b72ad7f 100644 --- a/TUI.Core/Services/UIApplication.cs +++ b/TUI.Core/Services/UIApplication.cs @@ -67,6 +67,17 @@ public class UIApplication(IConsoleProvider consoleProvider, ILogger LogUIApplicationSetup = LoggerMessage.Define(LogLevel.Information, new EventId(16, nameof(LogUIApplicationSetup)), "UI application setup with root element of type {ElementType}"); + private static readonly Action LogInterruptReceived = + LoggerMessage.Define(LogLevel.Information, new EventId(17, nameof(LogInterruptReceived)), "Interrupt signal received, shutting down"); + + /// + /// Gets the source of process interrupt signals that shuts the application down + /// + /// + /// Defaults to the real console and process signals. Tests substitute a source they can raise. + /// + internal IInterruptSource InterruptSource { get; init; } = new ConsoleInterruptSource(); + /// public IUIElement? RootElement { get; set; } @@ -90,6 +101,7 @@ public async Task RunAsync(CancellationToken cancellationToken = default) IsRunning = true; _cancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + IDisposable? interruptRegistration = null; try { @@ -98,6 +110,10 @@ public async Task RunAsync(CancellationToken cancellationToken = default) LogStartingApplication(_logger, null); } + // Take Ctrl+C and SIGTERM for the duration of the run. Both otherwise end the process + // outright, skipping the finally below that puts the cursor back. + interruptRegistration = InterruptSource.Register(OnInterrupt); + // Initialize console ConsoleProvider.Clear(); ConsoleProvider.SetCursorVisibility(false); @@ -125,6 +141,9 @@ public async Task RunAsync(CancellationToken cancellationToken = default) } finally { + // Stop taking signals before restoring the terminal, so a second Ctrl+C arriving + // during teardown gets the runtime's default behaviour rather than a second shutdown. + interruptRegistration?.Dispose(); IsRunning = false; ConsoleProvider.SetCursorVisibility(true); if (_logger != null) @@ -134,6 +153,19 @@ public async Task RunAsync(CancellationToken cancellationToken = default) } } + /// + /// Handles an interrupt signal by shutting the application down through its normal path + /// + private void OnInterrupt() + { + if (_logger != null) + { + LogInterruptReceived(_logger, null); + } + + Shutdown(); + } + /// public void Shutdown() { @@ -210,7 +242,12 @@ public async Task ProcessInputAsync(CancellationToken cancellationToken = defaul { try { - Models.InputResult input = await ConsoleProvider.ReadInputAsync().ConfigureAwait(false); + // Abandon the read when cancellation is requested. A provider parked in + // Console.ReadKey does not observe the token, so awaiting it directly would keep + // the loop alive until the user pressed an unrelated key after asking to exit. + Models.InputResult input = await ConsoleProvider.ReadInputAsync() + .WaitAsync(cancellationToken) + .ConfigureAwait(false); if (_logger != null) { diff --git a/TUI.Test/BlockingConsoleProvider.cs b/TUI.Test/BlockingConsoleProvider.cs new file mode 100644 index 0000000..805eceb --- /dev/null +++ b/TUI.Test/BlockingConsoleProvider.cs @@ -0,0 +1,77 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.TUI.Test; + +using ktsu.TUI.Core.Contracts; +using ktsu.TUI.Core.Models; + +/// +/// An test double whose does not +/// complete until a test releases it, standing in for a real provider parked in +/// Console.ReadKey. +/// +/// +/// Lifecycle tests need the input loop to be genuinely blocked. RecordingConsoleProvider +/// returns input immediately, which spins the loop and hides whether a shutdown request can end a +/// run that is waiting on the keyboard — the case that left the cursor hidden after Ctrl+C. +/// +internal sealed class BlockingConsoleProvider : IConsoleProvider +{ + private readonly TaskCompletionSource pendingRead = + new(TaskCreationOptions.RunContinuationsAsynchronously); + + private readonly TaskCompletionSource readStarted = + new(TaskCreationOptions.RunContinuationsAsynchronously); + + /// + /// Gets a task that completes once the application has begun waiting for input. + /// + internal Task ReadStarted => readStarted.Task; + + /// + /// Gets the last cursor visibility set through . + /// + internal bool CursorVisible => Volatile.Read(ref cursorVisible); + + private bool cursorVisible = true; + + /// + public Dimensions Dimensions { get; set; } = new(80, 24); + + /// + public void Clear() + { + // Nothing to record: these tests assert on lifecycle, not on drawn output. + } + + /// + public void Render(IUIElement element, Position position) => element?.Render(this); + + /// + public void WriteAt(string text, Position position, TextStyle? style = null) + { + // Nothing to record: these tests assert on lifecycle, not on drawn output. + } + + /// + public Task ReadInputAsync() + { + readStarted.TrySetResult(); + return pendingRead.Task; + } + + /// + public void SetCursorVisibility(bool visible) => Volatile.Write(ref cursorVisible, visible); + + /// + public void SetCursorPosition(Position position) + { + // Nothing to record: these tests assert on lifecycle, not on cursor placement. + } + + /// + /// Completes the pending read, as pressing a key would. + /// + /// The input to deliver. + internal void Release(InputResult input) => pendingRead.TrySetResult(input); +} diff --git a/TUI.Test/FakeInterruptSource.cs b/TUI.Test/FakeInterruptSource.cs new file mode 100644 index 0000000..0b0fc57 --- /dev/null +++ b/TUI.Test/FakeInterruptSource.cs @@ -0,0 +1,49 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.TUI.Test; + +using ktsu.TUI.Core.Contracts; + +/// +/// An test double a test can raise on demand. +/// +/// +/// The real source is the process's own Ctrl+C and SIGTERM handling, which a test cannot trigger +/// without signalling — and very likely terminating — the test host itself. +/// +internal sealed class FakeInterruptSource : IInterruptSource +{ + private readonly TaskCompletionSource registered = + new(TaskCreationOptions.RunContinuationsAsynchronously); + + private Action? handler; + + /// + /// Gets a task that completes once the application has registered its interrupt handler. + /// + internal Task Registered => registered.Task; + + /// + /// Gets the number of times the registration returned by was disposed. + /// + internal int DisposeCount { get; private set; } + + /// + public IDisposable Register(Action onInterrupt) + { + handler = onInterrupt; + registered.TrySetResult(); + return new Registration(this); + } + + /// + /// Invokes the registered handler, as Ctrl+C or SIGTERM would. + /// + internal void RaiseInterrupt() => handler?.Invoke(); + + private sealed class Registration(FakeInterruptSource owner) : IDisposable + { + /// + public void Dispose() => owner.DisposeCount++; + } +} diff --git a/TUI.Test/UIApplicationTests.cs b/TUI.Test/UIApplicationTests.cs index e69de29..9b43d69 100644 --- a/TUI.Test/UIApplicationTests.cs +++ b/TUI.Test/UIApplicationTests.cs @@ -0,0 +1,197 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.TUI.Test; + +using ktsu.TUI.Core.Models; +using ktsu.TUI.Core.Services; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Tests for 's run lifecycle: how a run ends, and whether the terminal +/// state it changes on the way in is put back on the way out. +/// +[TestClass] +public sealed class UIApplicationTests +{ + /// + /// How long any single step of a run is given before it is declared stuck. Generous, because + /// the assertion being made is "this happens at all", not "this happens quickly". + /// + /// + /// Every wait in this class is bounded by it. The behaviour under test is an application that + /// fails to notice a shutdown request, so an unbounded wait would hang the suite on a + /// regression instead of reporting one. + /// + private static readonly TimeSpan StepTimeout = TimeSpan.FromSeconds(10); + + /// + /// Tests that an interrupt signal ends a run that is blocked waiting for a key, and leaves the + /// cursor visible. Ctrl+C used to terminate the process at the runtime level instead, skipping + /// the teardown that restores the cursor and leaving the user's terminal without one. + /// + [TestMethod] + public async Task InterruptSignalEndsTheRunAndRestoresTheCursor() + { + // Arrange + BlockingConsoleProvider provider = new(); + FakeInterruptSource interrupts = new(); + UIApplication app = new(provider) { InterruptSource = interrupts }; + + Task run = app.RunAsync(); + await AssertCompletesAsync(interrupts.Registered, "The application should register an interrupt handler when it starts").ConfigureAwait(false); + await AssertCompletesAsync(provider.ReadStarted, "The application should start waiting for input").ConfigureAwait(false); + Assert.IsFalse(provider.CursorVisible, "The application should hide the cursor while running"); + + // Act + interrupts.RaiseInterrupt(); + + // Assert + await AssertCompletesAsync(run, "An interrupt should end the run even while it is blocked waiting for a key").ConfigureAwait(false); + Assert.IsTrue(provider.CursorVisible, "The cursor must be visible again once the application has exited"); + Assert.IsFalse(app.IsRunning, "The application should not report itself as running after an interrupt"); + } + + /// + /// Tests that the interrupt registration is released when the run ends, so the application + /// stops taking Ctrl+C once it is no longer the one owning the terminal. + /// + [TestMethod] + public async Task TheInterruptRegistrationIsReleasedWhenTheRunEnds() + { + // Arrange + BlockingConsoleProvider provider = new(); + FakeInterruptSource interrupts = new(); + UIApplication app = new(provider) { InterruptSource = interrupts }; + + Task run = app.RunAsync(); + await AssertCompletesAsync(interrupts.Registered, "The application should register an interrupt handler when it starts").ConfigureAwait(false); + Assert.AreEqual(0, interrupts.DisposeCount, "The registration should stay live while the application runs"); + + // Act + interrupts.RaiseInterrupt(); + await AssertCompletesAsync(run, "An interrupt should end the run").ConfigureAwait(false); + + // Assert + Assert.AreEqual(1, interrupts.DisposeCount, "The interrupt registration should be disposed exactly once"); + } + + /// + /// Tests that cancelling the token passed to ends a run + /// that is blocked waiting for a key. The input loop used to await the provider's read + /// directly, so a cancelled run kept waiting until the user pressed an unrelated key. + /// + [TestMethod] + public async Task CancellingTheRunTokenEndsARunBlockedOnInput() + { + // Arrange + BlockingConsoleProvider provider = new(); + FakeInterruptSource interrupts = new(); + using CancellationTokenSource cancellation = new(); + UIApplication app = new(provider) { InterruptSource = interrupts }; + + Task run = app.RunAsync(cancellation.Token); + await AssertCompletesAsync(provider.ReadStarted, "The application should start waiting for input").ConfigureAwait(false); + + // Act + await cancellation.CancelAsync().ConfigureAwait(false); + + // Assert + await AssertCompletesAsync(run, "Cancelling the run token should end a run blocked waiting for a key").ConfigureAwait(false); + Assert.IsTrue(provider.CursorVisible, "The cursor must be visible again once the application has exited"); + } + + /// + /// Tests that ends a run blocked waiting for a key, which + /// is the path an interrupt takes and the path an element requesting exit takes. + /// + [TestMethod] + public async Task ShutdownEndsARunBlockedOnInput() + { + // Arrange + BlockingConsoleProvider provider = new(); + FakeInterruptSource interrupts = new(); + UIApplication app = new(provider) { InterruptSource = interrupts }; + + Task run = app.RunAsync(); + await AssertCompletesAsync(provider.ReadStarted, "The application should start waiting for input").ConfigureAwait(false); + + // Act + app.Shutdown(); + + // Assert + await AssertCompletesAsync(run, "Shutdown should end a run blocked waiting for a key").ConfigureAwait(false); + Assert.IsTrue(provider.CursorVisible, "The cursor must be visible again once the application has exited"); + } + + /// + /// Tests that the ordinary exit path still works: input flagged as an exit request ends the + /// run and restores the cursor. + /// + [TestMethod] + public async Task ExitInputEndsTheRunAndRestoresTheCursor() + { + // Arrange + BlockingConsoleProvider provider = new(); + FakeInterruptSource interrupts = new(); + UIApplication app = new(provider) { InterruptSource = interrupts }; + + Task run = app.RunAsync(); + await AssertCompletesAsync(provider.ReadStarted, "The application should start waiting for input").ConfigureAwait(false); + + // Act + provider.Release(InputResult.Exit()); + + // Assert + await AssertCompletesAsync(run, "Exit input should end the run").ConfigureAwait(false); + Assert.IsTrue(provider.CursorVisible, "The cursor must be visible again once the application has exited"); + Assert.IsFalse(app.IsRunning, "The application should not report itself as running after exiting"); + } + + /// + /// Tests that the real interrupt source hooks and unhooks the process signals without + /// throwing on whichever platform the suite is running on, and that disposing twice is safe. + /// + [TestMethod] + public void TheConsoleInterruptSourceHooksAndUnhooksTheProcessSignals() + { + // Arrange + ConsoleInterruptSource source = new(); + + // Act + IDisposable registration = source.Register(() => { }); + + // Assert + Assert.IsNotNull(registration, "Registering should return a registration to dispose"); + registration.Dispose(); + registration.Dispose(); + } + + /// + /// Tests that the real interrupt source rejects a missing callback rather than hooking a + /// signal it cannot act on. + /// + [TestMethod] + public void TheConsoleInterruptSourceRejectsAMissingCallback() + { + // Arrange + ConsoleInterruptSource source = new(); + + // Act & Assert + Assert.ThrowsExactly(() => source.Register(null!)); + } + + /// + /// Awaits and fails with if it does not + /// finish within . + /// + /// The task to await. + /// The assertion message to report on a timeout. + private static async Task AssertCompletesAsync(Task task, string because) + { + Task finished = await Task.WhenAny(task, Task.Delay(StepTimeout)).ConfigureAwait(false); + Assert.AreSame(task, finished, because); + + // Observed separately so a task that failed reports its own exception, not the timeout. + await task.ConfigureAwait(false); + } +} From fc98499bbeeff0881fc05e3a75b7cb69feab2a72 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 18:44:23 +0000 Subject: [PATCH 2/4] refactor: use a volatile field for the test double's cursor state [patch] Volatile.Read/Write over a plain field was more ceremony than the situation needs, and it left the field looking mutable-for-no-reason to analysers. A volatile bool gives the same cross-thread visibility in one declaration. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01671tcpA4zkfbgPcJm8cTsB --- TUI.Test/BlockingConsoleProvider.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/TUI.Test/BlockingConsoleProvider.cs b/TUI.Test/BlockingConsoleProvider.cs index 805eceb..8c2086a 100644 --- a/TUI.Test/BlockingConsoleProvider.cs +++ b/TUI.Test/BlockingConsoleProvider.cs @@ -29,11 +29,15 @@ internal sealed class BlockingConsoleProvider : IConsoleProvider internal Task ReadStarted => readStarted.Task; /// - /// Gets the last cursor visibility set through . + /// Volatile because the application sets it from the thread running the app while the test + /// reads it, and one assertion reads it mid-run rather than after the run has completed. /// - internal bool CursorVisible => Volatile.Read(ref cursorVisible); + private volatile bool cursorVisible = true; - private bool cursorVisible = true; + /// + /// Gets the last cursor visibility set through . + /// + internal bool CursorVisible => cursorVisible; /// public Dimensions Dimensions { get; set; } = new(80, 24); @@ -61,7 +65,7 @@ public Task ReadInputAsync() } /// - public void SetCursorVisibility(bool visible) => Volatile.Write(ref cursorVisible, visible); + public void SetCursorVisibility(bool visible) => cursorVisible = visible; /// public void SetCursorPosition(Position position) From 380f52795d2f741b65494165207527ac10769972 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 18:59:26 +0000 Subject: [PATCH 3/4] test: cover the signal response and the interrupt log line [patch] SonarCloud's quality gate failed the PR at 69.7% coverage on new code against a required 80%. Measuring locally rather than guessing showed the gap was concentrated in ConsoleInterruptSource: two near-identical signal handler bodies that no test could reach, because neither ConsoleCancelEventArgs nor PosixSignalContext can be constructed by a test. Both handlers now funnel through one OnSignal method, so the response to a signal is stated once and is assertable separately from the delivery of one. The test asserts the ordering rather than just the calls: notifying runs application shutdown code, and until the default response is cancelled the runtime is still entitled to kill the process partway through it, so cancelling has to come first. Also asserts the interrupt is logged, via a recording ILogger. An operator reading logs should be able to tell why an application exited. New-code line coverage is now 86%. The four lines still uncovered are the SIGTERM callback and the PlatformNotSupportedException guard, which need a real signal delivered to the test host or a platform that rejects SIGTERM. Overall coverage rose from 48.0% to 54.8%. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01671tcpA4zkfbgPcJm8cTsB --- TUI.Core/Services/ConsoleInterruptSource.cs | 35 +++++--- TUI.Test/UIApplicationTests.cs | 96 +++++++++++++++++++++ 2 files changed, 119 insertions(+), 12 deletions(-) diff --git a/TUI.Core/Services/ConsoleInterruptSource.cs b/TUI.Core/Services/ConsoleInterruptSource.cs index bfa519d..ef50beb 100644 --- a/TUI.Core/Services/ConsoleInterruptSource.cs +++ b/TUI.Core/Services/ConsoleInterruptSource.cs @@ -26,6 +26,25 @@ public IDisposable Register(Action onInterrupt) return new Registration(onInterrupt); } + /// + /// Responds to an arriving signal, whichever mechanism delivered it + /// + /// Cancels the runtime's default "terminate now" response + /// Notifies the application that it should shut down + /// + /// Both delivery mechanisms funnel through here because neither + /// nor can be constructed + /// by a test, so the response to a signal is only assertable once it is separated from the + /// delivery of one. + /// + internal static void OnSignal(Action cancelDefaultTermination, Action onInterrupt) + { + // Cancel first. Notifying can run arbitrary application code, and until the default + // response is cancelled the runtime is still entitled to kill the process underneath it. + cancelDefaultTermination(); + onInterrupt(); + } + /// /// Holds the signal hooks for one call and unhooks them on disposal /// @@ -37,13 +56,7 @@ private sealed class Registration : IDisposable internal Registration(Action onInterrupt) { - _cancelKeyPress = (_, e) => - { - // Cancel the runtime's default "terminate now" behaviour so the application - // shuts down through its normal path and gets to restore the terminal. - e.Cancel = true; - onInterrupt(); - }; + _cancelKeyPress = (_, e) => OnSignal(() => e.Cancel = true, onInterrupt); Console.CancelKeyPress += _cancelKeyPress; _sigTerm = TryRegisterSigTerm(onInterrupt); @@ -66,11 +79,9 @@ public void Dispose() { try { - return PosixSignalRegistration.Create(PosixSignal.SIGTERM, context => - { - context.Cancel = true; - onInterrupt(); - }); + return PosixSignalRegistration.Create( + PosixSignal.SIGTERM, + context => OnSignal(() => context.Cancel = true, onInterrupt)); } catch (PlatformNotSupportedException) { diff --git a/TUI.Test/UIApplicationTests.cs b/TUI.Test/UIApplicationTests.cs index 9b43d69..bb2f87c 100644 --- a/TUI.Test/UIApplicationTests.cs +++ b/TUI.Test/UIApplicationTests.cs @@ -4,6 +4,7 @@ namespace ktsu.TUI.Test; using ktsu.TUI.Core.Models; using ktsu.TUI.Core.Services; +using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; /// @@ -24,6 +25,11 @@ public sealed class UIApplicationTests /// private static readonly TimeSpan StepTimeout = TimeSpan.FromSeconds(10); + /// + /// The order a signal must be handled in: cancel the runtime's default response, then notify. + /// + private static readonly string[] CancelThenNotify = ["cancel", "notify"]; + /// /// Tests that an interrupt signal ends a run that is blocked waiting for a key, and leaves the /// cursor visible. Ctrl+C used to terminate the process at the runtime level instead, skipping @@ -166,6 +172,57 @@ public void TheConsoleInterruptSourceHooksAndUnhooksTheProcessSignals() registration.Dispose(); } + /// + /// Tests that a signal cancels the runtime's default termination before notifying the + /// application, and not the other way round. + /// + /// + /// The order is the whole point. Notifying runs application shutdown code, and until the + /// default response is cancelled the runtime is still entitled to kill the process partway + /// through it — which is the failure this PR exists to stop. + /// + [TestMethod] + public void ASignalCancelsDefaultTerminationBeforeNotifyingTheApplication() + { + // Arrange + List order = []; + + // Act + ConsoleInterruptSource.OnSignal(() => order.Add("cancel"), () => order.Add("notify")); + + // Assert + CollectionAssert.AreEqual( + CancelThenNotify, + order, + "The default termination must be cancelled before the application is notified"); + } + + /// + /// Tests that taking an interrupt is logged, so an operator can tell why an application + /// exited rather than being left to guess. + /// + [TestMethod] + public async Task AnInterruptIsLogged() + { + // Arrange + BlockingConsoleProvider provider = new(); + FakeInterruptSource interrupts = new(); + RecordingLogger logger = new(); + UIApplication app = new(provider, logger) { InterruptSource = interrupts }; + + Task run = app.RunAsync(); + await AssertCompletesAsync(interrupts.Registered, "The application should register an interrupt handler when it starts").ConfigureAwait(false); + + // Act + interrupts.RaiseInterrupt(); + await AssertCompletesAsync(run, "An interrupt should end the run").ConfigureAwait(false); + + // Assert + Assert.IsTrue( + logger.Messages.Any(m => m.Contains("Interrupt signal received", StringComparison.Ordinal)), + $"The interrupt should have been logged. Logged: {string.Join(" | ", logger.Messages)}"); + } + /// /// Tests that the real interrupt source rejects a missing callback rather than hooking a /// signal it cannot act on. @@ -194,4 +251,43 @@ private static async Task AssertCompletesAsync(Task task, string because) // Observed separately so a task that failed reports its own exception, not the timeout. await task.ConfigureAwait(false); } + + /// + /// An that records the messages written to it, so a test + /// can assert what the application reported rather than only that it did not throw. + /// + private sealed class RecordingLogger : ILogger + { + private readonly List messages = []; + + /// + /// Gets the messages logged so far, in call order. + /// + internal IEnumerable Messages + { + get + { + lock (messages) + { + return [.. messages]; + } + } + } + + /// + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + /// + public bool IsEnabled(LogLevel logLevel) => true; + + /// + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) + { + string message = formatter is null ? string.Empty : formatter(state, exception); + lock (messages) + { + messages.Add(message); + } + } + } } From 790a7ca04d29b7a29846a9230f23bec030641e53 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 19:10:52 +0000 Subject: [PATCH 4/4] test: adopt the assertion APIs the MSTest analyzers ask for [patch] SonarCloud reported seven new issues, all MSTest analyzer suggestions on the lifecycle tests added by this branch: - MSTEST0049 (x5): pass the test's cancellation token to RunAsync, so a run started by a test ends when the test run itself is cancelled - MSTEST0068: Assert.AreSequenceEqual over CollectionAssert.AreEqual - MSTEST0037: Assert.Contains over Assert.IsTrue with a predicate The assertion rewrites were checked by breaking the expectations on purpose and confirming both tests still fail: swapping assertion APIs is exactly where a test can quietly stop asserting anything. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01671tcpA4zkfbgPcJm8cTsB --- TUI.Test/UIApplicationTests.cs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/TUI.Test/UIApplicationTests.cs b/TUI.Test/UIApplicationTests.cs index bb2f87c..fbbf4e5 100644 --- a/TUI.Test/UIApplicationTests.cs +++ b/TUI.Test/UIApplicationTests.cs @@ -30,6 +30,12 @@ public sealed class UIApplicationTests /// private static readonly string[] CancelThenNotify = ["cancel", "notify"]; + /// + /// Gets or sets the test context MSTest injects, used for its cancellation token so a run + /// started by a test ends when the test run itself is cancelled. + /// + public TestContext TestContext { get; set; } = null!; + /// /// Tests that an interrupt signal ends a run that is blocked waiting for a key, and leaves the /// cursor visible. Ctrl+C used to terminate the process at the runtime level instead, skipping @@ -43,7 +49,7 @@ public async Task InterruptSignalEndsTheRunAndRestoresTheCursor() FakeInterruptSource interrupts = new(); UIApplication app = new(provider) { InterruptSource = interrupts }; - Task run = app.RunAsync(); + Task run = app.RunAsync(TestContext.CancellationToken); await AssertCompletesAsync(interrupts.Registered, "The application should register an interrupt handler when it starts").ConfigureAwait(false); await AssertCompletesAsync(provider.ReadStarted, "The application should start waiting for input").ConfigureAwait(false); Assert.IsFalse(provider.CursorVisible, "The application should hide the cursor while running"); @@ -69,7 +75,7 @@ public async Task TheInterruptRegistrationIsReleasedWhenTheRunEnds() FakeInterruptSource interrupts = new(); UIApplication app = new(provider) { InterruptSource = interrupts }; - Task run = app.RunAsync(); + Task run = app.RunAsync(TestContext.CancellationToken); await AssertCompletesAsync(interrupts.Registered, "The application should register an interrupt handler when it starts").ConfigureAwait(false); Assert.AreEqual(0, interrupts.DisposeCount, "The registration should stay live while the application runs"); @@ -118,7 +124,7 @@ public async Task ShutdownEndsARunBlockedOnInput() FakeInterruptSource interrupts = new(); UIApplication app = new(provider) { InterruptSource = interrupts }; - Task run = app.RunAsync(); + Task run = app.RunAsync(TestContext.CancellationToken); await AssertCompletesAsync(provider.ReadStarted, "The application should start waiting for input").ConfigureAwait(false); // Act @@ -141,7 +147,7 @@ public async Task ExitInputEndsTheRunAndRestoresTheCursor() FakeInterruptSource interrupts = new(); UIApplication app = new(provider) { InterruptSource = interrupts }; - Task run = app.RunAsync(); + Task run = app.RunAsync(TestContext.CancellationToken); await AssertCompletesAsync(provider.ReadStarted, "The application should start waiting for input").ConfigureAwait(false); // Act @@ -191,7 +197,7 @@ public void ASignalCancelsDefaultTerminationBeforeNotifyingTheApplication() ConsoleInterruptSource.OnSignal(() => order.Add("cancel"), () => order.Add("notify")); // Assert - CollectionAssert.AreEqual( + Assert.AreSequenceEqual( CancelThenNotify, order, "The default termination must be cancelled before the application is notified"); @@ -210,7 +216,7 @@ public async Task AnInterruptIsLogged() RecordingLogger logger = new(); UIApplication app = new(provider, logger) { InterruptSource = interrupts }; - Task run = app.RunAsync(); + Task run = app.RunAsync(TestContext.CancellationToken); await AssertCompletesAsync(interrupts.Registered, "The application should register an interrupt handler when it starts").ConfigureAwait(false); // Act @@ -218,9 +224,10 @@ public async Task AnInterruptIsLogged() await AssertCompletesAsync(run, "An interrupt should end the run").ConfigureAwait(false); // Assert - Assert.IsTrue( - logger.Messages.Any(m => m.Contains("Interrupt signal received", StringComparison.Ordinal)), - $"The interrupt should have been logged. Logged: {string.Join(" | ", logger.Messages)}"); + Assert.Contains( + m => m.Contains("Interrupt signal received", StringComparison.Ordinal), + logger.Messages, + "The interrupt should have been logged"); } ///