test: replace wall-clock sleeps used as synchronization — register row 40

Six sleeps across four test files stood in for an observable edge. Each preceded an ABSENCE assertion (or "settled" before one), so they failed toward false-pass rather than flake: the assertion could not distinguish "the mechanism ran and correctly stayed quiet" from "the mechanism had not run yet". ScriptDeadlineAtEnqueueTests (two sleeps, the worst offender) - :105 Thread.Sleep(2s) — now two explicit edges: the run must be observably QUEUED (scheduler.QueueDepth >= 1, never checked before), then the wait polls until the 1 s budget has elapsed, bailing early if the run leaves the queue, and asserts it is STILL queued afterwards. The test now proves the design claim it was named for — queue wait burns the run's budget — instead of assuming it. - :128 Thread.Sleep(2s) + Assert.Equal(0, DetachedThreadCount) — replaced by a positive control: a second script that genuinely wedges a worker, on the same scheduler, same 1 s timeout + 1 s grace, armed strictly later. Its stuck report is the edge; once it lands, every watchdog armed at or before it has provably finished its grace delay. The absence is then asserted per script name, and the detach counter is pinned at 1 (the wedged run) rather than 0 — which a counter that never moves also satisfied. DeploymentWarmThenGateTests (:117 plus two sibling Task.Delay(500) startup waits) - The manager calls SetInstanceCounts unconditionally when its startup config load completes, so a report counter on the test collector is the startup edge. The empty-site zero is now the manager's own observation instead of an untouched field. SiteAlarmLiveCacheServiceTests (:195, :216) - Both sampled IsLive once after a fixed sleep. Now they poll continuously across three linger windows on a new teardown observable (the mock client counts subscription cancellations), so a stop at any instant inside the window is caught, and each closes with a positive control proving that counter moves. DataConnectionActorTests (:435) - A Task.Delay(200) "settle" after an AwaitCondition on a connect counter. The counter only proves ConnectAsync was CALLED; the actor is still Connecting until it processes the piped result. Now waits for the Connected health publication that BecomeConnected emits, so the subscribe under test provably runs against Connected rather than being silently stashed. (The stash path keeps its own coverage in WP6_ConnectingState_StashesSubscribeRequests.) Negative controls (product broken in the working tree, old vs new test compared): - Stuck-script watchdog disabled entirely: OLD test passed 2/2, NEW test failed. The whole watchdog could be deleted and the old assertions were satisfied. - Startup reporting a phantom instance count and taking longer than 500 ms: OLD passed 2/2, NEW failed (expected 0, actual 1). - Linger cancellation disabled + the remaining-viewer guard dropped, so the stream is torn down under a live viewer while IsLive keeps reporting true: Resubscribe_Within_Linger OLD passed 2/2, NEW failed. (Last_Viewer_Leaving caught this particular break in the old form too, via a later assertion — its strengthening is the teardown observable and the positive control, not a false-pass fix.) - DataConnectionActorTests :435 has no false-pass demo: the sleep guarded no assertion there, and the change is determinism plus proving the claimed state. Also fixes a latent race these timing changes exposed in InstanceActorChildAttributeRaceTests: it resolved child actors single-shot immediately after construction, but ResolveOne throws until the child's cell is published on the parent AND the cell's Actor property stays null until the child's constructor has run on its own mailbox thread. Neither is ordered against the test thread, so it failed both ways under full-suite load (~1 run in 6 once the sleeps above stopped padding the schedule; 14/14 green on the unmodified tree, so it was latent, not new). Resolution now retries to a bounded deadline; the assertion is unchanged. Verification: SiteRuntime.Tests 8/8 green (605 tests), Communication.Tests 3/3 (717), DataConnectionLayer.Tests 3/3 (319). Test-only change — no src/ modifications.
2026-08-15 17:06:51 -04:00
parent 65b19516c8
commit 7a7d345b89
5 changed files with 241 additions and 41 deletions
@@ -41,6 +41,17 @@ public class SiteAlarmLiveCacheServiceTests : TestKit
        /// <summary>Graceful-completion callbacks captured from every subscribe.</summary>
        public List<Action> Completions { get { lock (_lock) { return _completions.ToList(); } } }

+        private int _streamEnds;
+
+        /// <summary>
+        /// Register row 40: how many site-wide subscriptions have been torn down (their
+        /// cancellation token fired). This is the aggregator-stop observable — the linger
+        /// stop cancels the aggregator's token, which ends the hanging subscribe below.
+        /// Tests that must show an aggregator was NOT stopped assert on this rather than
+        /// sampling <c>IsLive</c> once after a fixed sleep.
+        /// </summary>
+        public int StreamEndCount => Volatile.Read(ref _streamEnds);
+
        public override Task SubscribeSiteAsync(
            string correlationId, Action<Commons.Messages.Streaming.AlarmStateChanged> onAlarmEvent,
            Action<Exception> onError, Action onCompleted, CancellationToken ct,
@@ -55,7 +66,11 @@ public class SiteAlarmLiveCacheServiceTests : TestKit
            // subscribe), which is what makes the aggregator report the stream live.
            if (AutoConnect) onConnected?.Invoke();
            var tcs = new TaskCompletionSource();
-            ct.Register(() => tcs.TrySetResult());
+            ct.Register(() =>
+            {
+                Interlocked.Increment(ref _streamEnds);
+                tcs.TrySetResult();
+            });
            return tcs.Task;
        }
        public override void Unsubscribe(string correlationId) { }
@@ -184,27 +199,47 @@ public class SiteAlarmLiveCacheServiceTests : TestKit
    [Fact]
    public void Last_Viewer_Leaving_Stops_Aggregator_After_Linger()
    {
-        var service = CreateService(TimeSpan.FromMilliseconds(200), out _);
+        var linger = TimeSpan.FromMilliseconds(200);
+        var service = CreateService(linger, out var factory);

        var sub1 = service.Subscribe(SiteId, () => { });
        var sub2 = service.Subscribe(SiteId, () => { });
        AwaitCondition(() => service.IsLive(SiteId), TimeSpan.FromSeconds(5));

        sub1.Dispose();
-        // One viewer remains — still live, not stopped.
-        Thread.Sleep(300);
-        Assert.True(service.IsLive(SiteId));
+
+        // Register row 40: one viewer remains, so the aggregator must NOT be stopped. The old
+        // test slept 300 ms and sampled IsLive once — a teardown that happened at any other
+        // instant (or one that stopped and restarted the stream) was invisible to it. Poll
+        // CONTINUOUSLY across three linger windows instead, bailing out the moment either
+        // signal says the aggregator went away.
+        var held = System.Diagnostics.Stopwatch.StartNew();
+        AwaitCondition(
+            () => held.Elapsed > linger * 3
+                  || !service.IsLive(SiteId)
+                  || factory.Client.StreamEndCount > 0,
+            TimeSpan.FromSeconds(5), TimeSpan.FromMilliseconds(10));
+        Assert.True(service.IsLive(SiteId),
+            "the aggregator was reported not-live while a viewer was still subscribed");
+        Assert.Equal(0, factory.Client.StreamEndCount);
+        Assert.Equal(1, factory.GetOrCreateCount);

        sub2.Dispose();
        // Last viewer left → after the linger the aggregator is torn down and the entry cleared.
        AwaitCondition(() => !service.IsLive(SiteId), TimeSpan.FromSeconds(3));
+
+        // Positive control on the SAME observable the negative above rests on: the site-wide
+        // stream really is torn down now, so the zero above was a live signal rather than a
+        // counter that never moves.
+        AwaitCondition(() => factory.Client.StreamEndCount > 0, TimeSpan.FromSeconds(3));
        Assert.Empty(service.GetCurrentAlarms(SiteId));
    }

    [Fact]
    public void Resubscribe_Within_Linger_Cancels_The_Pending_Stop()
    {
-        var service = CreateService(TimeSpan.FromMilliseconds(400), out var factory);
+        var linger = TimeSpan.FromMilliseconds(400);
+        var service = CreateService(linger, out var factory);

        var sub1 = service.Subscribe(SiteId, () => { });
        AwaitCondition(() => service.IsLive(SiteId), TimeSpan.FromSeconds(5));
@@ -212,13 +247,28 @@ public class SiteAlarmLiveCacheServiceTests : TestKit
        sub1.Dispose();                 // schedules linger stop
        var sub2 = service.Subscribe(SiteId, () => { }); // returns within the linger window → cancels it

-        // Wait comfortably past the original linger; the aggregator must still be alive.
-        Thread.Sleep(700);
-        Assert.True(service.IsLive(SiteId));
+        // Register row 40: the pending stop must be CANCELLED, not merely outrun. The old test
+        // slept 700 ms and sampled once — it could not see a stream that was torn down and
+        // silently replaced inside that window, because GetOrCreateCount is checked only at the
+        // end and the mock hands back the same client object either way. Poll continuously past
+        // the original linger on the teardown observable itself.
+        var kept = System.Diagnostics.Stopwatch.StartNew();
+        AwaitCondition(
+            () => kept.Elapsed > linger * 3
+                  || !service.IsLive(SiteId)
+                  || factory.Client.StreamEndCount > 0,
+            TimeSpan.FromSeconds(5), TimeSpan.FromMilliseconds(10));
+        Assert.True(service.IsLive(SiteId),
+            "the re-subscribe did not cancel the pending linger stop");
+        Assert.Equal(0, factory.Client.StreamEndCount);
        // No second aggregator/stream was opened — the same one was kept warm.
        Assert.Equal(1, factory.GetOrCreateCount);

        sub2.Dispose();
+
+        // Positive control: with the last viewer gone the linger stop is allowed to run, so the
+        // teardown counter this test's zero rests on does in fact move.
+        AwaitCondition(() => factory.Client.StreamEndCount > 0, TimeSpan.FromSeconds(5));
    }

    [Fact]
@@ -76,6 +76,23 @@ public class DataConnectionActorTests : TestKit
        adapter.Disconnected += Raise.Event<Action>();
    }

+    /// <summary>
+    /// Register row 40: true once the actor has actually ENTERED the Connected state for
+    /// <paramref name="connectionName"/>. <c>BecomeConnected</c> publishes
+    /// <see cref="ConnectionHealth.Connected"/> to the health collector immediately before
+    /// its <c>Become</c>, so this is the state transition's own observable edge.
+    ///
+    /// <para>An adapter-side connect counter is NOT that edge: it only proves
+    /// <c>ConnectAsync</c> was called, while the actor is still in Connecting until it
+    /// processes the piped <c>ConnectResult</c>. Tests used to bridge that gap with a
+    /// fixed <c>Task.Delay</c>.</para>
+    /// </summary>
+    private bool HasEnteredConnectedState(string connectionName) =>
+        _mockHealthCollector.ReceivedCalls().Any(c =>
+            c.GetMethodInfo().Name == nameof(ISiteHealthCollector.UpdateConnectionHealth) &&
+            c.GetArguments() is [string name, ConnectionHealth.Connected, ..] &&
+            name == connectionName);
+
    [Fact]
    public void WP6_StartsInConnectingState_AttemptsConnect()
    {
@@ -430,9 +447,14 @@ public class DataConnectionActorTests : TestKit

        var actor = CreateFailoverActor(primaryAdapter, "resub-test", primaryConfig, backupConfig, failoverRetryCount: 1);

-        // Wait for initial connect
+        // Wait for the actor to reach Connected — not merely for ConnectAsync to have been
+        // called, and not for a fixed 200 ms to elapse (register row 40). The subscribe below
+        // is only meaningful against the Connected state: issued during Connecting it is
+        // STASHED, so a too-short settle would silently test the stash path instead, and a
+        // too-long one just slows the suite.
        AwaitCondition(() => primaryConnectCount >= 1, TimeSpan.FromSeconds(2));
-        await Task.Delay(200);
+        AwaitCondition(() => HasEnteredConnectedState("resub-test"), TimeSpan.FromSeconds(5),
+            TimeSpan.FromMilliseconds(10));

        // Subscribe to tags while connected on primary
        actor.Tell(new SubscribeTagsRequest("corr1", "inst1", "resub-test", ["sensor/temp"], DateTimeOffset.UtcNow));
@@ -65,6 +65,21 @@ public class DeploymentWarmThenGateTests : TestKit, IDisposable
            new SiteRuntimeOptions(), NullLogger<DeploymentManagerActor>.Instance, null,
            healthCollector)));

+    /// <summary>
+    /// Register row 40: starts a Deployment Manager and blocks until it has PROVABLY finished
+    /// its startup config load, by waiting for its first instance-count report. Replaces a bare
+    /// <c>await Task.Delay(500)</c>, which merely hoped 500 ms was enough and — for the empty-site
+    /// assertion — could not tell "startup ran and found nothing" from "startup had not run yet".
+    /// </summary>
+    private (IActorRef Dm, DeployedCountCollector Health) CreateStartedDeploymentManager()
+    {
+        var health = new DeployedCountCollector();
+        var dm = CreateDeploymentManager(health);
+        AwaitCondition(() => health.ReportCount > 0, TimeSpan.FromSeconds(20),
+            TimeSpan.FromMilliseconds(25));
+        return (dm, health);
+    }
+
    /// <summary>
    /// Captures the deployed-instance count the Deployment Manager reports. The count is
    /// mutated only on the actor thread — <c>HandleDeploy</c> adds the instance name,
@@ -73,7 +88,20 @@ public class DeploymentWarmThenGateTests : TestKit, IDisposable
    /// </summary>
    private sealed class DeployedCountCollector : ISiteHealthCollector
    {
+        private int _reports;
+
        public int LastDeployedCount { get; private set; }
+
+        /// <summary>
+        /// Register row 40: how many times the Deployment Manager has reported instance
+        /// counts. The manager calls <see cref="SetInstanceCounts"/> unconditionally the
+        /// moment its startup config load completes (<c>HandleStartupConfigsLoaded</c>) —
+        /// including for an empty site — so a non-zero value here is the explicit
+        /// observable edge for "startup has finished", replacing a wall-clock sleep.
+        /// Without it, <c>LastDeployedCount == 0</c> is satisfied by the field's initial
+        /// value and proves nothing about the manager having started at all.
+        /// </summary>
+        public int ReportCount => Volatile.Read(ref _reports);
        public void IncrementScriptError() { }
        public void IncrementAlarmError() { }
        public void IncrementDeadLetter() { }
@@ -86,7 +114,12 @@ public class DeploymentWarmThenGateTests : TestKit, IDisposable
        public void UpdateConnectionEndpoint(string connectionName, string endpoint) { }
        public void UpdateTagQuality(string connectionName, int good, int bad, int uncertain) { }
        public void SetStoreAndForwardDepths(IReadOnlyDictionary<string, int> depths) { }
-        public void SetInstanceCounts(int deployed, int enabled, int disabled) => LastDeployedCount = deployed;
+        public void SetInstanceCounts(int deployed, int enabled, int disabled)
+        {
+            LastDeployedCount = deployed;
+            Interlocked.Increment(ref _reports);
+        }
+
        public void SetParkedMessageCount(int count) { }
        public void SetNodeHostname(string hostname) { }
        public void SetClusterNodes(IReadOnlyList<Commons.Messages.Health.NodeStatus> nodes) { }
@@ -112,9 +145,10 @@ public class DeploymentWarmThenGateTests : TestKit, IDisposable
    [Fact]
    public async Task DeleteArrivingDuringTheCompileWarm_IsQueuedAndAppliedAfterTheDeploy()
    {
-        var health = new DeployedCountCollector();
-        var dm = CreateDeploymentManager(health);
-        await Task.Delay(500); // empty startup
+        // The startup load has demonstrably completed (it reported counts at least once), so
+        // the zero below is the manager's own observation of an empty site rather than an
+        // untouched field.
+        var (dm, health) = CreateStartedDeploymentManager();
        Assert.Equal(0, health.LastDeployedCount);

        var deployProbe = CreateTestProbe();
@@ -145,8 +179,7 @@ public class DeploymentWarmThenGateTests : TestKit, IDisposable
    [Fact]
    public async Task SecondDeployDuringTheWarm_SupersedesTheFirst_AndAnswersItsDeployer()
    {
-        var dm = CreateDeploymentManager();
-        await Task.Delay(500);
+        var (dm, _) = CreateStartedDeploymentManager();

        var first = CreateTestProbe();
        var second = CreateTestProbe();
@@ -177,8 +210,7 @@ public class DeploymentWarmThenGateTests : TestKit, IDisposable
    [Fact]
    public async Task DeploysForDifferentInstances_DoNotBlockEachOther()
    {
-        var dm = CreateDeploymentManager();
-        await Task.Delay(500);
+        var (dm, _) = CreateStartedDeploymentManager();

        var probeX = CreateTestProbe();
        var probeY = CreateTestProbe();
@@ -107,16 +107,54 @@ public class InstanceActorChildAttributeRaceTests : TestKit, IDisposable
            ]
        };

-    /// <summary>Resolves the live actor instance behind a local <see cref="IActorRef"/>.</summary>
-    private static object GetActorInstance(IActorRef actorRef)
+    /// <summary>
+    /// Resolves a named child and the live actor instance behind it, retrying until BOTH
+    /// exist (register row 40 — absence/settle timing pins).
+    ///
+    /// <para>Two distinct publication races sit between "the parent reported N children" and
+    /// "the child's actor object is reachable by reflection", and neither is ordered against
+    /// the test thread: <see cref="ActorSelection.ResolveOne"/> throws
+    /// <see cref="ActorNotFoundException"/> until the child's cell is published on the parent,
+    /// and the cell's internal <c>Actor</c> property stays null until the child's constructor
+    /// has run on the child's own mailbox thread. A single-shot resolve therefore fails
+    /// intermittently under load — observed both ways in full-suite runs ("subject was null"
+    /// from ResolveOne, and a null instance from the reflection read). Retrying to a bounded
+    /// deadline removes the race without weakening the assertion: what the test proves is
+    /// still the seed-dictionary isolation contract, checked once both objects exist.</para>
+    /// </summary>
+    private async Task<T> ResolveChildInstanceAsync<T>(IActorRef parent, string childName)
+        where T : class
    {
-        var cell = ((ActorRefWithCell)actorRef).Underlying;
-        // ActorCell exposes the actor instance via its internal Actor property.
-        var actorProp = cell.GetType().GetProperty(
-            "Actor", BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public);
-        var instance = actorProp!.GetValue(cell);
-        Assert.NotNull(instance);
-        return instance!;
+        var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(15);
+        while (true)
+        {
+            try
+            {
+                var child = await Sys.ActorSelection(parent.Path / childName)
+                    .ResolveOne(TimeSpan.FromSeconds(1));
+                var cell = ((ActorRefWithCell)child).Underlying;
+                // ActorCell exposes the actor instance via its internal Actor property.
+                var actorProp = cell.GetType().GetProperty(
+                    "Actor", BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public);
+                if (actorProp!.GetValue(cell) is T typed)
+                {
+                    return typed;
+                }
+            }
+            catch (ActorNotFoundException)
+            {
+                // The child's cell is not published on the parent yet — retry below.
+            }
+
+            if (DateTime.UtcNow > deadline)
+            {
+                throw new TimeoutException(
+                    $"Child actor '{childName}' (or its {typeof(T).Name} instance) never " +
+                    "materialised within the resolve deadline.");
+            }
+
+            await Task.Delay(25);
+        }
    }

    private static Dictionary<string, object?> GetPrivateAttributes(InstanceActor instance)
@@ -157,9 +195,7 @@ public class InstanceActorChildAttributeRaceTests : TestKit, IDisposable
        // mutates on another thread (SiteRuntime-017).
        foreach (var name in new[] { "WorkerA", "WorkerB" })
        {
-            var child = await Sys.ActorSelection(testRef.Path / $"script-{name}")
-                .ResolveOne(TimeSpan.FromSeconds(5));
-            var scriptActor = (ScriptActor)GetActorInstance(child);
+            var scriptActor = await ResolveChildInstanceAsync<ScriptActor>(testRef, $"script-{name}");

            Assert.NotNull(scriptActor.SeedAttributesReference);
            Assert.False(
@@ -177,9 +213,7 @@ public class InstanceActorChildAttributeRaceTests : TestKit, IDisposable
        }

        // The Alarm Actor must likewise be seeded from an isolated copy.
-        var alarmChild = await Sys.ActorSelection(testRef.Path / "alarm-HighTemp")
-            .ResolveOne(TimeSpan.FromSeconds(5));
-        var alarmActor = (AlarmActor)GetActorInstance(alarmChild);
+        var alarmActor = await ResolveChildInstanceAsync<AlarmActor>(testRef, "alarm-HighTemp");

        Assert.NotNull(alarmActor.SeedAttributesReference);
        Assert.False(
@@ -101,8 +101,26 @@ public class ScriptDeadlineAtEnqueueTests : TestKit, IDisposable
        var caller = CreateTestProbe();
        late.Tell(new ScriptCallRequest("Late", null, 0, "corr-late"), caller.Ref);

-        // Let its whole budget elapse while it is still queued, then free the worker.
-        Thread.Sleep(TimeSpan.FromSeconds(2));
+        // Register row 40 — edge 1 (replaces a bare Thread.Sleep(2s)): the late run must
+        // actually be SITTING IN THE SCHEDULER QUEUE. The old test only slept and hoped;
+        // a blocker that failed to occupy the worker would have let the late run execute
+        // normally, and the shed-at-dequeue path under test would never have been reached.
+        AwaitCondition(() => scheduler.QueueDepth >= 1, TimeSpan.FromSeconds(15),
+            TimeSpan.FromMilliseconds(25));
+
+        // Register row 40 — edge 2: hold until the late run's 1 s budget has provably
+        // elapsed, re-reading the queue on every poll and bailing out early the instant the
+        // run leaves the queue. The post-condition below then proves the budget burned
+        // *while queued* — the actual design claim ("queue wait consumes the run's budget"),
+        // which a fixed sleep asserted nothing about.
+        var queuedFor = System.Diagnostics.Stopwatch.StartNew();
+        AwaitCondition(
+            () => queuedFor.Elapsed >= TimeSpan.FromSeconds(1.5) || scheduler.QueueDepth == 0,
+            TimeSpan.FromSeconds(15), TimeSpan.FromMilliseconds(25));
+        Assert.True(scheduler.QueueDepth >= 1,
+            "the late run left the queue before its budget elapsed, so this run never " +
+            "exercised the shed-at-dequeue path");
+
        DeadlineHooks.Gate.Release();

        var result = caller.ExpectMsg<ScriptCallResult>(TimeSpan.FromSeconds(15));
@@ -123,12 +141,56 @@ public class ScriptDeadlineAtEnqueueTests : TestKit, IDisposable
            Assert.DoesNotContain(rows, r => r.Message.Contains("started", StringComparison.OrdinalIgnoreCase));
        }, TimeSpan.FromSeconds(5));

-        // And the watchdog did NOT report it as a stuck thread: it was cancelled while queued,
-        // so it never held a worker.
-        Thread.Sleep(TimeSpan.FromSeconds(2)); // well past the 1 s grace
+        // ── Register row 40 — the watchdog absence, proved by a positive control ──
+        //
+        // The claim is that the shed run was NOT reported as a stuck thread: it was cancelled
+        // while queued, so it never held a worker. The old test slept 2 s and asserted the
+        // absence, which false-passes whenever the watchdog simply had not got around to
+        // evaluating anything yet — the assertion could not distinguish "the watchdog looked
+        // and correctly stayed silent" from "the watchdog never looked".
+        //
+        // So: enqueue a script that genuinely wedges a worker, on the SAME scheduler and with
+        // the SAME 1 s timeout + 1 s grace, and armed strictly LATER than the late run's
+        // watchdog. Its stuck report is the observable edge — once it lands, every watchdog
+        // armed at or before it has provably run its grace delay and made its decision.
+        //
+        // The body blocks its worker SYNCHRONOUSLY (so the watchdog can detach the thread it
+        // holds) but for a BOUNDED 2.5 s that comfortably clears 1 s timeout + 1 s grace and
+        // then self-releases. A gate held until test teardown would work too, but it leaves a
+        // blocked OS thread alive past the test and perturbs timing-sensitive tests elsewhere
+        // in this assembly.
+        var wedged = BuildScriptActor(
+            "Wedged",
+            CompileRaw("System.Threading.Thread.Sleep(2500); return null;"),
+            new SiteRuntimeOptions { ScriptExecutionTimeoutSeconds = 1, StuckScriptGraceMs = 1000 },
+            scheduler, new SingleServiceProvider(siteLog));
+        wedged.Tell(new ScriptCallRequest("Wedged", null, 0, "corr-wedged"), ActorRefs.NoSender);
+
+        // Positive event, bounded wait: the watchdog fires, names the wedged script and
+        // detaches its worker.
+        AwaitAssert(() =>
+        {
+            Assert.Contains(siteLog.OfType("script"),
+                r => r.Message.Contains("'Wedged'", StringComparison.Ordinal) &&
+                     r.Message.Contains("still executing", StringComparison.OrdinalIgnoreCase));
+        }, TimeSpan.FromSeconds(30), TimeSpan.FromMilliseconds(50));
+
+        // Negative control, evaluated only now that a later-armed watchdog has demonstrably
+        // completed its grace delay: the shed run was never reported stuck.
        Assert.DoesNotContain(siteLog.OfType("script"),
-            r => r.Message.Contains("still executing", StringComparison.OrdinalIgnoreCase));
-        Assert.Equal(0, scheduler.DetachedThreadCount);
+            r => r.Message.Contains("'Late'", StringComparison.Ordinal) &&
+                 r.Message.Contains("still executing", StringComparison.OrdinalIgnoreCase));
+
+        // And the detach counter moved for exactly one run — the wedged one. The old
+        // `Assert.Equal(0, DetachedThreadCount)` was satisfied by a counter that never moves
+        // for any reason; this pins that it does move, and that the shed run did not move it.
+        Assert.Equal(1, scheduler.DetachedThreadCount);
+
+        // Wait for the positive control's body to unwind and its detached worker to exit
+        // BEFORE the scheduler is disposed at end of scope, so the test leaves no threads
+        // behind for the rest of the assembly to contend with.
+        AwaitCondition(() => scheduler.BusyThreadCount == 0 && scheduler.DetachedThreadCount == 0,
+            TimeSpan.FromSeconds(20), TimeSpan.FromMilliseconds(25));
    }
 }