From 3a6af884877c792379f5908517607d7afc9aa6a0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Morten=20Aslo-=C3=98stergaard?= <morten@itinstituttet.dk>
Date: Fri, 6 Nov 2026 09:57:07 +0100
Subject: [PATCH] test(run-commands): ThrottledSignalTests wait for the event,
 not a fixed 500ms

A_burst_of_signals_runs_the_action_once failed on main's CI right after #160
merged (Expected: 1, Actual: 0) while passing in #160's own run and locally.
Each test slept a fixed 500ms of wall clock for a 50ms timer; xunit runs test
classes in parallel, and on a loaded runner the Task.Delay continuation can
land later than that. A_signal_after_the_action_ran_schedules_another had a
second form of the same race: if the first run was late, the second Signal
piggybacked on it and the test saw one run instead of two.

Now each test polls until the expected count is reached (10s ceiling, only
for a broken implementation), then watches a further 300ms to catch an extra
run. ThrottledSignal itself is unchanged.

Mutation-checked: removing the coalescing fails the burst test; clearing
_pending after the action instead of before fails the during-action test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NsYm9iLc2aRnRDfV2uZRQX
---
 .../ThrottledSignalTests.cs                   | 38 +++++++++++++++----
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/tests/CodeShellManager.Tests/ThrottledSignalTests.cs b/tests/CodeShellManager.Tests/ThrottledSignalTests.cs
index ce710cc..bf19c51 100644
--- a/tests/CodeShellManager.Tests/ThrottledSignalTests.cs
+++ b/tests/CodeShellManager.Tests/ThrottledSignalTests.cs
@@ -15,6 +15,25 @@ public class ThrottledSignalTests
 {
     private static readonly TimeSpan Delay = TimeSpan.FromMilliseconds(50);
 
+    // Wait for an EVENT, never for a fixed amount of wall clock. These used to sleep 500ms and
+    // then assert, which failed on a loaded CI runner (main, after #160 merged: "Expected: 1,
+    // Actual: 0") — xunit runs test classes in parallel, and a 50ms Task.Delay continuation on
+    // a starved thread pool can land well past 500ms. The deadline below is only a ceiling for
+    // a genuinely broken implementation; a healthy one returns in ~50ms.
+    private static readonly TimeSpan Deadline = TimeSpan.FromSeconds(10);
+
+    // How long to keep watching after the expected count is reached, to catch an EXTRA run.
+    // Several times Delay, so a wrongly scheduled second run would have fired by then.
+    private static readonly TimeSpan Settle = TimeSpan.FromMilliseconds(300);
+
+    private static async Task WaitForRuns(Func<int> runs, int atLeast)
+    {
+        var until = DateTime.UtcNow + Deadline;
+        while (runs() < atLeast && DateTime.UtcNow < until)
+            await Task.Delay(10);
+        Assert.True(runs() >= atLeast, $"expected at least {atLeast} run(s) within {Deadline.TotalSeconds}s, saw {runs()}");
+    }
+
     [Fact]
     public async Task A_burst_of_signals_runs_the_action_once()
     {
@@ -22,9 +41,10 @@ public async Task A_burst_of_signals_runs_the_action_once()
         var signal = new ThrottledSignal(Delay, a => a(), () => Interlocked.Increment(ref runs));
 
         for (int i = 0; i < 1000; i++) signal.Signal();
-        await Task.Delay(500);
+        await WaitForRuns(() => Volatile.Read(ref runs), 1);
+        await Task.Delay(Settle);
 
-        Assert.Equal(1, runs);
+        Assert.Equal(1, Volatile.Read(ref runs));
     }
 
     [Fact]
@@ -34,11 +54,14 @@ public async Task A_signal_after_the_action_ran_schedules_another()
         var signal = new ThrottledSignal(Delay, a => a(), () => Interlocked.Increment(ref runs));
 
         signal.Signal();
-        await Task.Delay(500);
+        // Must wait for the first run itself: signalling again while it is still pending
+        // piggybacks on it, which is the burst case above, not this one.
+        await WaitForRuns(() => Volatile.Read(ref runs), 1);
         signal.Signal();
-        await Task.Delay(500);
+        await WaitForRuns(() => Volatile.Read(ref runs), 2);
+        await Task.Delay(Settle);
 
-        Assert.Equal(2, runs);
+        Assert.Equal(2, Volatile.Read(ref runs));
     }
 
     [Fact]
@@ -54,9 +77,10 @@ public async Task A_signal_during_the_action_is_not_lost()
         });
 
         signal.Signal();
-        await Task.Delay(500);
+        await WaitForRuns(() => Volatile.Read(ref runs), 2);
+        await Task.Delay(Settle);
 
-        Assert.Equal(2, runs);
+        Assert.Equal(2, Volatile.Read(ref runs));
     }
 
     [Fact]