diff --git a/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteAlarmLiveCacheServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteAlarmLiveCacheServiceTests.cs index 8735009e..0aead6e8 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteAlarmLiveCacheServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteAlarmLiveCacheServiceTests.cs @@ -41,6 +41,17 @@ public class SiteAlarmLiveCacheServiceTests : TestKit /// Graceful-completion callbacks captured from every subscribe. public List Completions { get { lock (_lock) { return _completions.ToList(); } } } + private int _streamEnds; + + /// + /// 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 IsLive once after a fixed sleep. + /// + public int StreamEndCount => Volatile.Read(ref _streamEnds); + public override Task SubscribeSiteAsync( string correlationId, Action onAlarmEvent, Action 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] diff --git a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/DataConnectionActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/DataConnectionActorTests.cs index 6bd888e5..c1358446 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/DataConnectionActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/DataConnectionActorTests.cs @@ -76,6 +76,23 @@ public class DataConnectionActorTests : TestKit adapter.Disconnected += Raise.Event(); } + /// + /// Register row 40: true once the actor has actually ENTERED the Connected state for + /// . BecomeConnected publishes + /// to the health collector immediately before + /// its Become, so this is the state transition's own observable edge. + /// + /// An adapter-side connect counter is NOT that edge: it only proves + /// ConnectAsync was called, while the actor is still in Connecting until it + /// processes the piped ConnectResult. Tests used to bridge that gap with a + /// fixed Task.Delay. + /// + 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)); diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentWarmThenGateTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentWarmThenGateTests.cs index d8cdaa8d..58f23c41 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentWarmThenGateTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/DeploymentWarmThenGateTests.cs @@ -65,6 +65,21 @@ public class DeploymentWarmThenGateTests : TestKit, IDisposable new SiteRuntimeOptions(), NullLogger.Instance, null, healthCollector))); + /// + /// 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 + /// await Task.Delay(500), 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". + /// + 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); + } + /// /// Captures the deployed-instance count the Deployment Manager reports. The count is /// mutated only on the actor thread — HandleDeploy adds the instance name, @@ -73,7 +88,20 @@ public class DeploymentWarmThenGateTests : TestKit, IDisposable /// private sealed class DeployedCountCollector : ISiteHealthCollector { + private int _reports; + public int LastDeployedCount { get; private set; } + + /// + /// Register row 40: how many times the Deployment Manager has reported instance + /// counts. The manager calls unconditionally the + /// moment its startup config load completes (HandleStartupConfigsLoaded) — + /// 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, LastDeployedCount == 0 is satisfied by the field's initial + /// value and proves nothing about the manager having started at all. + /// + 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 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 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(); diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorChildAttributeRaceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorChildAttributeRaceTests.cs index 437b653b..0f12b6e1 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorChildAttributeRaceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorChildAttributeRaceTests.cs @@ -107,16 +107,54 @@ public class InstanceActorChildAttributeRaceTests : TestKit, IDisposable ] }; - /// Resolves the live actor instance behind a local . - private static object GetActorInstance(IActorRef actorRef) + /// + /// Resolves a named child and the live actor instance behind it, retrying until BOTH + /// exist (register row 40 — absence/settle timing pins). + /// + /// 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: throws + /// until the child's cell is published on the parent, + /// and the cell's internal Actor 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. + /// + private async Task ResolveChildInstanceAsync(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 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(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(testRef, "alarm-HighTemp"); Assert.NotNull(alarmActor.SeedAttributesReference); Assert.False( diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptDeadlineAtEnqueueTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptDeadlineAtEnqueueTests.cs index 9ce81e89..b1ea03cf 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptDeadlineAtEnqueueTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptDeadlineAtEnqueueTests.cs @@ -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(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)); } }