From f4576adf8b4743eec7ee99a844bd8089de3a5b04 Mon Sep 17 00:00:00 2001 From: Jamie Chapman <104535858+bilbospocketses@users.noreply.github.com> Date: Wed, 26 Aug 2026 12:18:13 -0400 Subject: [PATCH 1/2] test(TransientNotice): drive auto-dismiss from an injectable clock Show_TwiceQuickly_LaterMessageSurvivesTheEarlierTimer has been failing CI on both open Dependabot PRs. It is flaky, not a regression: PR #110's same commit passed at 21:39 and failed at 21:44 on 2026-08-17. The test slept 100ms then 180ms against a 200ms dismiss deadline that restarted at the 100ms mark, so it asserted ~280ms into a window closing at ~300ms. Twenty milliseconds of headroom across two Task.Delay calls, on a shared runner, against ~15.6ms timer granularity. TransientNotice now takes an optional TimeProvider, defaulting to TimeProvider.System so all six call sites are untouched, and the auto-dismiss awaits Task.Delay against it. The tests drive a FakeTimeProvider and land their assertions exactly on the deadlines that matter instead of sampling somewhere between them. The dismiss clears state before invoking the re-render callback, so the tests use that callback as their completion signal rather than sleeping. Where a test asserts that a cancelled timer never fires, it first waits a real grace window - safe here because fake time only moves when a test moves it, so a correctly cancelled timer never fires however long we wait. Confirmed by mutation: dropping the Cancel() from Show() fails Show_TwiceQuickly_LaterMessageSurvivesTheEarlierTimer, and dropping it from Clear() fails Clear_RemovesMessageImmediatelyAndStopsTimer. 710 tests green in Release. --- Directory.Packages.props | 1 + .../Components/Shared/TransientNotice.cs | 36 ++++++++-- .../Components/TransientNoticeTests.cs | 69 +++++++++++++++---- .../ControlMenu.Tests.csproj | 1 + 4 files changed, 87 insertions(+), 20 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index 8e341031..66701fa2 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -23,6 +23,7 @@ + diff --git a/src/ControlMenu/Components/Shared/TransientNotice.cs b/src/ControlMenu/Components/Shared/TransientNotice.cs index a6b61584..a7b966fa 100644 --- a/src/ControlMenu/Components/Shared/TransientNotice.cs +++ b/src/ControlMenu/Components/Shared/TransientNotice.cs @@ -11,9 +11,19 @@ namespace ControlMenu.Components.Shared; public sealed class TransientNotice : IDisposable { private readonly Func _onChange; + private readonly TimeProvider _timeProvider; private CancellationTokenSource? _cts; - public TransientNotice(Func onChange) => _onChange = onChange; + /// Invoked after an auto-dismiss clears the message, so the owning + /// component can re-render. + /// Clock backing the auto-dismiss delay. Defaults to + /// ; tests pass a fake clock so they can land assertions + /// exactly on a dismiss deadline instead of racing real elapsed time. + public TransientNotice(Func onChange, TimeProvider? timeProvider = null) + { + _onChange = onChange; + _timeProvider = timeProvider ?? TimeProvider.System; + } public string? Message { get; private set; } public string CssClass { get; private set; } = ""; @@ -31,13 +41,25 @@ public void Show(string message, string cssClass = "", string icon = "", int dis _cts = new CancellationTokenSource(); var token = _cts.Token; - _ = Task.Delay(dismissMs, token).ContinueWith(async _ => + _ = DismissAfterAsync(dismissMs, token); + } + + private async Task DismissAfterAsync(int dismissMs, CancellationToken token) + { + try { - Message = null; - CssClass = ""; - Icon = ""; - await _onChange(); - }, token, TaskContinuationOptions.OnlyOnRanToCompletion, TaskScheduler.Default); + await Task.Delay(TimeSpan.FromMilliseconds(dismissMs), _timeProvider, token) + .ConfigureAwait(false); + } + catch (OperationCanceledException) + { + return; // a newer Show() or a Clear() superseded this timer + } + + Message = null; + CssClass = ""; + Icon = ""; + await _onChange().ConfigureAwait(false); } /// Clears the message immediately and cancels/disposes any pending auto-dismiss. diff --git a/tests/ControlMenu.Tests/Components/TransientNoticeTests.cs b/tests/ControlMenu.Tests/Components/TransientNoticeTests.cs index a003c6ed..c410652b 100644 --- a/tests/ControlMenu.Tests/Components/TransientNoticeTests.cs +++ b/tests/ControlMenu.Tests/Components/TransientNoticeTests.cs @@ -1,13 +1,43 @@ using ControlMenu.Components.Shared; +using Microsoft.Extensions.Time.Testing; namespace ControlMenu.Tests.Components; public class TransientNoticeTests { + /// + /// Builds a notice on a fake clock plus a signal that completes the first time the notice asks + /// its component to re-render. The auto-dismiss clears the message *before* invoking that + /// callback, so awaiting the signal is a deterministic "the dismiss has finished" handle — which + /// is what lets every test below drive time explicitly instead of racing real elapsed time. + /// + private static (TransientNotice Notice, Task Rerendered) NoticeOn(FakeTimeProvider time) + { + var rerendered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var notice = new TransientNotice( + () => { rerendered.TrySetResult(); return Task.CompletedTask; }, + time); + return (notice, rerendered.Task); + } + + /// + /// Asserts no auto-dismiss re-render arrives within a real grace window. A dismiss runs its + /// state change on a continuation, so a wrongly-surviving timer fires slightly *after* the + /// that released it — asserting instantly would race + /// past the very bug these tests exist to catch. Waiting real time is safe here precisely + /// because the clock is fake: a correctly-cancelled timer never fires no matter how long we + /// wait, since fake time only moves when a test moves it. + /// + private static async Task AssertNoRerender(Task rerendered, string because) + { + var grace = Task.Delay(TimeSpan.FromMilliseconds(250)); + Assert.False(await Task.WhenAny(rerendered, grace) == rerendered, because); + } + [Fact] public void Show_SetsMessageClassIconAndVisible() { - var notice = new TransientNotice(() => Task.CompletedTask); + var notice = new TransientNotice(() => Task.CompletedTask, new FakeTimeProvider()); notice.Show("hello", "status-success", "bi-check", dismissMs: 60_000); Assert.Equal("hello", notice.Message); @@ -19,16 +49,17 @@ public void Show_SetsMessageClassIconAndVisible() [Fact] public async Task AutoDismiss_ClearsMessageAndNotifies() { - var changes = 0; - var notice = new TransientNotice(() => { Interlocked.Increment(ref changes); return Task.CompletedTask; }); - notice.Show("bye", dismissMs: 80); + var time = new FakeTimeProvider(); + var (notice, rerendered) = NoticeOn(time); + notice.Show("bye", dismissMs: 80); Assert.True(notice.IsVisible); - await Task.Delay(250); + + time.Advance(TimeSpan.FromMilliseconds(80)); + await rerendered.WaitAsync(TimeSpan.FromSeconds(5)); Assert.Null(notice.Message); Assert.False(notice.IsVisible); - Assert.True(changes >= 1, "auto-dismiss should notify the component to re-render"); } [Fact] @@ -36,30 +67,42 @@ public async Task Show_TwiceQuickly_LaterMessageSurvivesTheEarlierTimer() { // The bug this helper fixes: the settings pages started a Task.Delay timer with no CTS, so // an earlier message's timer would fire and wipe a newer message. Show() must cancel the - // prior timer. - var notice = new TransientNotice(() => Task.CompletedTask); + // prior timer. The fake clock lands the assertions exactly on the two deadlines that matter, + // rather than sampling somewhere between them and hoping the scheduler cooperates. + var time = new FakeTimeProvider(); + var (notice, rerendered) = NoticeOn(time); notice.Show("first", dismissMs: 200); - await Task.Delay(100); + time.Advance(TimeSpan.FromMilliseconds(100)); notice.Show("second", dismissMs: 200); // restarts; the first 200ms timer must be cancelled - await Task.Delay(180); // ~280ms: past the first message's 200ms deadline, before the second's (~300ms) + // t=200ms — exactly when the *first* message's timer was due. It was cancelled, so the newer + // message must still be standing. + time.Advance(TimeSpan.FromMilliseconds(100)); + await AssertNoRerender(rerendered, "the first message's timer was cancelled and must not fire"); Assert.Equal("second", notice.Message); - await Task.Delay(180); // ~460ms: past the second's deadline + // t=300ms — the second message's own deadline; now it clears. + time.Advance(TimeSpan.FromMilliseconds(100)); + await rerendered.WaitAsync(TimeSpan.FromSeconds(5)); Assert.Null(notice.Message); } [Fact] public async Task Clear_RemovesMessageImmediatelyAndStopsTimer() { - var notice = new TransientNotice(() => Task.CompletedTask); + var time = new FakeTimeProvider(); + var (notice, rerendered) = NoticeOn(time); + notice.Show("x", dismissMs: 60_000); notice.Clear(); Assert.Null(notice.Message); Assert.False(notice.IsVisible); - await Task.Delay(50); // the long timer must not fire and clobber state later + + // Advance far past the cancelled timer's deadline: it must never fire and clobber state. + time.Advance(TimeSpan.FromMinutes(5)); + await AssertNoRerender(rerendered, "Clear() cancelled the timer, so it must never fire"); Assert.Null(notice.Message); } } diff --git a/tests/ControlMenu.Tests/ControlMenu.Tests.csproj b/tests/ControlMenu.Tests/ControlMenu.Tests.csproj index 574a0f8b..0a576947 100644 --- a/tests/ControlMenu.Tests/ControlMenu.Tests.csproj +++ b/tests/ControlMenu.Tests/ControlMenu.Tests.csproj @@ -11,6 +11,7 @@ + From 8795148ed6df2b5b5410124efa6ebfbfa28a4f00 Mon Sep 17 00:00:00 2001 From: Jamie Chapman <104535858+bilbospocketses@users.noreply.github.com> Date: Wed, 26 Aug 2026 12:25:27 -0400 Subject: [PATCH 2/2] chore: nudge CI to run on this PR