From 5c50ef51954459bc3e0a4e2debc0f6d675fcd19a Mon Sep 17 00:00:00 2001 From: Phil Scott Date: Tue, 7 Jul 2026 13:56:31 -0400 Subject: [PATCH 1/2] test: deflake timing-sensitive FileWatcher and AuditRunner tests Both tests asserted on outcomes the OS/thread-pool scheduler controls, so a loaded CI runner could fail them though the production code was correct. FileWatcher_CoalescesRapidBurst drove a real OS FileSystemWatcher on the real clock and asserted a 6-write burst debounced to exactly one notification. Under load the watcher event pump stalls >100ms mid-burst, elapsing the debounce window and splitting the burst into two notifications (the failing main CI run). Drive the debounce off a FakeTimeProvider instead: the raw events still arrive in real time, but the timer only fires on Advance, so load can no longer split the window. Reproduced 3/8 failures under CPU saturation on the old test; 12/12 pass on the new one under the same load. AuditRunner's headless-one-shot test polled the cache for up to 500ms after ApplicationStarted, which could expire under thread-pool starvation; the dev-mode test slept a flat 100ms before asserting the rendered auditor never ran. Both now await the purpose-built WaitForInitialPassAsync() seam, removing the timing entirely. --- .../Generation/AuditRunnerTests.cs | 14 +++++------ .../Infrastructure/FileWatcherTests.cs | 23 +++++++++++++++---- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/tests/Pennington.Tests/Generation/AuditRunnerTests.cs b/tests/Pennington.Tests/Generation/AuditRunnerTests.cs index 714a0eec..6c530572 100644 --- a/tests/Pennington.Tests/Generation/AuditRunnerTests.cs +++ b/tests/Pennington.Tests/Generation/AuditRunnerTests.cs @@ -42,11 +42,10 @@ public async Task StartAsync_InHeadlessOneShot_RunsRenderedAuditor_AndCachesItsD cache.Diagnostics.ShouldBeEmpty(); lifetime.FireStarted(); - // RunAsync started on ApplicationStarted; give the cache a moment to be populated. - for (var i = 0; i < 50 && cache.Diagnostics.IsEmpty; i++) - { - await Task.Delay(10, TestContext.Current.CancellationToken); - } + // FireStarted synchronously kicks off the initial pass; await it to completion so the + // assertion reads a settled cache rather than polling with a bounded timeout (which could + // expire under thread-pool starvation on a loaded runner). + await runner.WaitForInitialPassAsync(); cache.Diagnostics.Count.ShouldBe(1); cache.Diagnostics[0].Message.ShouldBe("rendered audit ran"); @@ -80,8 +79,9 @@ public async Task StartAsync_InDevMode_SkipsRenderedAuditors() await runner.StartAsync(TestContext.Current.CancellationToken); lifetime.FireStarted(); - // Wait long enough for any background run to settle. - await Task.Delay(100, TestContext.Current.CancellationToken); + // Await the pass to completion, then assert the rendered auditor was never invoked — + // deterministic where a fixed delay only hoped the (skipped) run had settled. + await runner.WaitForInitialPassAsync(); renderedAuditor.AuditCalls.ShouldBe(0); cache.Diagnostics.ShouldBeEmpty(); diff --git a/tests/Pennington.Tests/Infrastructure/FileWatcherTests.cs b/tests/Pennington.Tests/Infrastructure/FileWatcherTests.cs index 025288d9..8ddf7bb8 100644 --- a/tests/Pennington.Tests/Infrastructure/FileWatcherTests.cs +++ b/tests/Pennington.Tests/Infrastructure/FileWatcherTests.cs @@ -121,10 +121,18 @@ public async Task FileWatcher_CoalescesRapidBurst_IntoSingleNotification() { // One logical save fans out into a truncate/write/close burst (plus Windows duplicate // events). The trailing-edge debounce must collapse the whole burst into a single - // notification delivered after the file goes quiet — not one per raw event. + // notification — not one per raw event. + // + // Drive the debounce off a fake clock so the assertion is deterministic. The raw OS events + // still arrive in real time, but the debounce timer fires only when we advance the clock, so + // while it stays frozen every event re-arms the same pending timer no matter how spread out + // their delivery is. That removes the real-clock flake this replaces: a loaded runner that + // dribbled the six events out over >100ms used to let the timer elapse mid-burst, splitting + // it into two notifications. var ct = TestContext.Current.CancellationToken; var fs = new RealFileSystem(); - using var watcher = new FileWatcher(fs); + var clock = new FakeTimeProvider(); + using var watcher = new FileWatcher(fs, clock); var count = 0; var filePath = Path.Combine(_tempDir, "burst.txt"); @@ -133,14 +141,19 @@ public async Task FileWatcher_CoalescesRapidBurst_IntoSingleNotification() watcher.AddPathWatch(_tempDir, "*.txt", (_, _) => { }); watcher.SubscribeToChanges(() => System.Threading.Interlocked.Increment(ref count)); - // Tight synchronous burst — completes far inside the debounce window. + // Tight synchronous burst. for (var i = 1; i <= 6; i++) { File.WriteAllText(filePath, $"v{i}"); } - // Wait comfortably past the debounce window for the single coalesced notification. - await Task.Delay(600, ct); + // Let the OS watcher drain every raw event into the pending buffer. The frozen clock keeps + // any of them from firing, so they collapse onto one re-armed timer; this wait only has to + // outlast event *delivery*, not sit above the debounce window. + await Task.Delay(500, ct); + + // Advance once past the 100ms debounce window: the single settled timer fires exactly once. + clock.Advance(TimeSpan.FromMilliseconds(150)); count.ShouldBe(1); } From a07ace8fe607e43cd0c447f5716349456a24f671 Mon Sep 17 00:00:00 2001 From: Phil Scott Date: Tue, 7 Jul 2026 13:56:48 -0400 Subject: [PATCH 2/2] test(docsite): fix CS8604 nullable-argument warning in area-home test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IElement.GetAttribute("href") returns a nullable string, so passing it straight to ShouldContain tripped CS8604. Assert the href is present first, then match on it — clears the warning via flow analysis and makes a missing href fail loudly instead of NRE-ing inside the matcher. --- .../DocsSite/AreaHomeNavigationTests.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/Pennington.IntegrationTests/DocsSite/AreaHomeNavigationTests.cs b/tests/Pennington.IntegrationTests/DocsSite/AreaHomeNavigationTests.cs index efb011a5..cb192b57 100644 --- a/tests/Pennington.IntegrationTests/DocsSite/AreaHomeNavigationTests.cs +++ b/tests/Pennington.IntegrationTests/DocsSite/AreaHomeNavigationTests.cs @@ -30,7 +30,9 @@ public async Task AreaWithHomePage_PillNavigatesDirectly_AndHomeIsDroppedFromToc // The pill links to the area home and is flagged so the click handler lets the // href navigate instead of just previewing the TOC. pill.GetAttribute("data-area-direct").ShouldBe("true"); - pill.GetAttribute("href").ShouldContain("showcase"); + var href = pill.GetAttribute("href"); + href.ShouldNotBeNull(); + href.ShouldContain("showcase"); // The home entry is the only showcase page, so dropping it leaves an empty TOC — // no link back to the home the pill already points at.