test(communication)+test(sitecallaudit): cap parallelism, delta/collection-serialize gauge asserts, replace sleep-then-assert, stop leaked 100ms tickers (arch-review 2026-08-19 P8-5 + P8-6, register rows 40/82/95/96/102 patterns)

This commit is contained in:
Joseph Doherty
2026-08-19 15:03:03 -04:00
parent 9a3f006721
commit d9d72f1bc5
7 changed files with 422 additions and 227 deletions
@@ -15,6 +15,15 @@ namespace ZB.MOM.WW.ScadaBridge.Communication.Tests.Grpc;
/// dedup by AlarmKey, placeholder seeded-but-not-on-stream coherence, NodeA↔NodeB reconnect
/// re-seed, and periodic reconcile authoritative-replace (drift correction).
/// </summary>
/// <remarks>
/// Serialized with <c>SiteAlarmLiveCacheTelemetryTests</c> (register row 102): every actor
/// this class spins up moves the process-global
/// <c>scadabridge.site.alarm_cache.aggregators.active</c> gauge via
/// <see cref="SiteAlarmAggregatorActor"/>'s PreStart/PostStop, which the telemetry test's
/// fixed-baseline assertions require to hold still (or only increase) between its capture
/// and its checks. Same collection ⇒ never concurrent.
/// </remarks>
[Collection("SiteAlarmAggregatorGauge")]
public class SiteAlarmAggregatorActorTests : TestKit
{
private const string SiteId = "site-alpha";
@@ -188,6 +197,28 @@ public class SiteAlarmAggregatorActorTests : TestKit
private static AlarmStateChanged? Find(IReadOnlyList<AlarmStateChanged>? rows, string alarmName, string sourceRef) =>
rows?.FirstOrDefault(r => r.AlarmName == alarmName && r.SourceReference == sourceRef);
/// <summary>
/// Row-40 house pattern for an ABSENCE assertion: proves <paramref name="checks"/> keep
/// holding throughout <paramref name="window"/> by polling, rather than a single blind
/// Thread.Sleep(window) followed by one end-of-window sample. A bare sleep-then-assert
/// cannot distinguish "the mechanism correctly stayed quiet" from "the mechanism just
/// hadn't run yet when the one sample was taken", and it also can't catch a value that
/// flips wrong and then flips back before that single sample. This fails immediately on
/// the first tick any check goes false, and only returns once the full window has
/// elapsed with every check still true.
/// </summary>
private static void AssertHoldsFor(TimeSpan window, params (Func<bool> Holds, string Because)[] checks)
{
var deadline = DateTime.UtcNow + window;
while (true)
{
foreach (var (holds, because) in checks)
Assert.True(holds(), because);
if (DateTime.UtcNow >= deadline) return;
Thread.Sleep(20);
}
}
// ── Tests ───────────────────────────────────────────────────────────────────
[Fact]
@@ -226,8 +257,11 @@ public class SiteAlarmAggregatorActorTests : TestKit
seed.CompleteNext(Alarm("PumpFault", "", 500, tSeed));
AwaitCondition(() => sink.Count >= 1, TimeSpan.FromSeconds(3));
// Give a beat to ensure no late (dropped) delta sneaks in.
Thread.Sleep(150);
// Prove no late (dropped) delta sneaks in over the window, not just at one sample.
AssertHoldsFor(TimeSpan.FromMilliseconds(150),
(() => Find(sink.Latest, "PumpFault", "") is not null, "seed row must still be present"),
(() => Find(sink.Latest, "PumpFault", "")?.Priority == 500,
"seed value kept; older buffered delta must stay dropped"));
var latest = sink.Latest!;
var pumpFault = Find(latest, "PumpFault", "");
Assert.NotNull(pumpFault);
@@ -382,8 +416,8 @@ public class SiteAlarmAggregatorActorTests : TestKit
seed.FaultNext();
// A failed INITIAL seed must not publish (page keeps its poll fallback — not live yet).
Thread.Sleep(200);
Assert.Equal(0, sink.Count);
AssertHoldsFor(TimeSpan.FromMilliseconds(200),
(() => sink.Count == 0, "a failed initial seed must not publish"));
// The periodic reconcile is the backstop: it re-runs the fan-out (flag was cleared
// on failure), completes, and the actor publishes → now live.
@@ -464,8 +498,8 @@ public class SiteAlarmAggregatorActorTests : TestKit
firstSub.OnError(new Exception("late zombie fault")); // stale gen-1 error races in
// Pre-fix this burns retry budget and opens a THIRD stream; post-fix it is ignored.
Thread.Sleep(400);
Assert.Equal(2, TotalSubs());
AssertHoldsFor(TimeSpan.FromMilliseconds(400),
(() => TotalSubs() == 2, "a stale gen-1 error must not open a third stream"));
}
// ── WP1.1: graceful (status OK) stream completion is a reconnect trigger ──
@@ -520,8 +554,8 @@ public class SiteAlarmAggregatorActorTests : TestKit
// … and without refunding it either: the next error is the 4th, so it exceeds
// MaxRetries and the stream is given up rather than reconnected.
factory.ClientFor(GrpcNodeB).Subs[2].OnError(new Exception("4"));
Thread.Sleep(400);
Assert.Equal(before, TotalSubs());
AssertHoldsFor(TimeSpan.FromMilliseconds(400),
(() => TotalSubs() == before, "budget exhausted — the 4th error must not reconnect"));
}
[Fact]
@@ -539,9 +573,11 @@ public class SiteAlarmAggregatorActorTests : TestKit
// A completion racing out of the dead gen-1 stream must not tear down the live
// gen-2 stream (which would go deltaless until the next reconcile tick).
firstSub.OnCompleted();
Thread.Sleep(300);
Assert.DoesNotContain("corr-1", factory.ClientFor(GrpcNodeB).Unsubscribed);
Assert.Single(factory.ClientFor(GrpcNodeB).Subs);
AssertHoldsFor(TimeSpan.FromMilliseconds(300),
(() => !factory.ClientFor(GrpcNodeB).Unsubscribed.Contains("corr-1"),
"a late gen-1 completion must not unsubscribe the live gen-2 stream"),
(() => factory.ClientFor(GrpcNodeB).Subs.Count == 1,
"the live gen-2 stream must not be torn down"));
}
// ── R2 T10: live-delta publish coalescing (N6) ──
@@ -621,15 +657,15 @@ public class SiteAlarmAggregatorActorTests : TestKit
factory.ClientFor(GrpcNodeA).Subs[1].OnError(new Exception("3"));
AwaitCondition(() => factory.ClientFor(GrpcNodeB).Subs.Count == 2, TimeSpan.FromSeconds(5));
Thread.Sleep(300);
Assert.Equal(1, seed.CallCount); // pre-fix: 4
AssertHoldsFor(TimeSpan.FromMilliseconds(300),
(() => seed.CallCount == 1, "failed reconnect attempts must not re-seed (pre-fix: 4)"));
// The stream finally comes up - exactly ONE re-seed, on the connect.
factory.ClientFor(GrpcNodeB).Subs[1].OnConnected!();
AwaitCondition(() => seed.CallCount == 2, TimeSpan.FromSeconds(5));
seed.CompleteNext();
Thread.Sleep(300);
Assert.Equal(2, seed.CallCount); // and no second one
AssertHoldsFor(TimeSpan.FromMilliseconds(300),
(() => seed.CallCount == 2, "the successful connect must re-seed exactly once"));
}
[Fact]
@@ -644,8 +680,8 @@ public class SiteAlarmAggregatorActorTests : TestKit
Thread.Sleep(200);
actor.Tell(new RunReconcile()); // skipped - the initial seed covered this window
Thread.Sleep(300);
Assert.Equal(1, seed.CallCount);
AssertHoldsFor(TimeSpan.FromMilliseconds(300),
(() => seed.CallCount == 1, "a tick inside the just-seeded window must be skipped"));
actor.Tell(new RunReconcile()); // and the next one runs
AwaitCondition(() => seed.CallCount == 2, TimeSpan.FromSeconds(3));
@@ -668,8 +704,8 @@ public class SiteAlarmAggregatorActorTests : TestKit
AwaitCondition(() => seed.CallCount == 2, TimeSpan.FromSeconds(3));
seed.CompleteNext(Alarm("PumpFault", "", 500, t)); // identical snapshot
Thread.Sleep(300);
Assert.Equal(1, sink.Count); // no second publish
AssertHoldsFor(TimeSpan.FromMilliseconds(300),
(() => sink.Count == 1, "an unchanged reconcile snapshot must not republish"));
// A genuinely changed snapshot still publishes.
actor.Tell(new RunReconcile());
@@ -733,8 +769,8 @@ public class SiteAlarmAggregatorActorTests : TestKit
Thread.Sleep(150);
actor.Tell(new RunReconcile()); // consumed by the initial seed's flag
Thread.Sleep(200);
Assert.Equal(1, seed.CallCount);
AssertHoldsFor(TimeSpan.FromMilliseconds(200),
(() => seed.CallCount == 1, "the tick right after the initial seed must be skipped"));
// From here on, with no reconnect in between, EVERY tick must fan out.
for (var expected = 2; expected <= 5; expected++)
@@ -778,8 +814,8 @@ public class SiteAlarmAggregatorActorTests : TestKit
// That re-seed covers this window: the next tick is skipped…
actor.Tell(new RunReconcile());
Thread.Sleep(250);
Assert.Equal(3, seed.CallCount);
AssertHoldsFor(TimeSpan.FromMilliseconds(250),
(() => seed.CallCount == 3, "the connect-driven re-seed must stand the next tick down"));
// …and the one after it fans out again.
actor.Tell(new RunReconcile());
@@ -87,12 +87,25 @@ public class PendingDeploymentPurgeActorTests : TestKit
// Fast interval so the self-scheduling timer fires repeatedly; the actor must
// swallow each per-tick exception and keep ticking (best-effort hygiene — a
// crash here would drop the singleton until failover).
CreateActor(repo, TimeSpan.FromMilliseconds(100));
var actor = CreateActor(repo, TimeSpan.FromMilliseconds(100));
AwaitAssert(
() => Assert.True(Volatile.Read(ref calls) >= 2,
$"expected >= 2 ticks despite failures, got {Volatile.Read(ref calls)}"),
duration: TimeSpan.FromSeconds(5),
interval: TimeSpan.FromMilliseconds(50));
try
{
AwaitAssert(
() => Assert.True(Volatile.Read(ref calls) >= 2,
$"expected >= 2 ticks despite failures, got {Volatile.Read(ref calls)}"),
duration: TimeSpan.FromSeconds(5),
interval: TimeSpan.FromMilliseconds(50));
}
finally
{
// Row-95/row-82 pattern: a 100ms self-scheduling ticker left running strands
// later tests in the same assembly (it keeps firing — and keeps calling the
// now-disposed repo/service-provider — for the rest of the process). Stop it
// deterministically before the test ends.
Watch(actor);
Sys.Stop(actor);
ExpectTerminated(actor, TimeSpan.FromSeconds(3));
}
}
}
@@ -11,6 +11,18 @@ namespace ZB.MOM.WW.ScadaBridge.Communication.Tests;
/// relative to a baseline so the process-wide static instruments are robust to any
/// parallel test interleaving.
/// </summary>
/// <remarks>
/// Register row 102's shape: a fixed-baseline delta only survives contributors that hold
/// constant or increase — a parallel class's actors moving the SAME process-global gauge
/// and then STOPPING after this test captures its baseline makes <c>baseline + N</c>
/// unreachable forever. <see cref="SiteAlarmAggregatorActor"/> calls
/// <see cref="ScadaBridgeTelemetry.LiveAlarmAggregatorStarted"/>/
/// <see cref="ScadaBridgeTelemetry.LiveAlarmAggregatorStopped"/> from its own
/// PreStart/PostStop, so every actor <c>Grpc.SiteAlarmAggregatorActorTests</c> spins up
/// moves this exact gauge — serialized into the same collection (row-96 pattern) so the
/// two classes never interleave.
/// </remarks>
[Collection("SiteAlarmAggregatorGauge")]
public class SiteAlarmLiveCacheTelemetryTests
{
private static long ReadGauge(string instrumentName)
@@ -22,6 +22,10 @@
<Using Include="Xunit" />
</ItemGroup>
<ItemGroup>
<Content Include="xunit.runner.json" CopyToOutputDirectory="PreserveNewest" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="../../src/ZB.MOM.WW.ScadaBridge.Communication/ZB.MOM.WW.ScadaBridge.Communication.csproj" />
<ProjectReference Include="../../src/ZB.MOM.WW.ScadaBridge.Commons/ZB.MOM.WW.ScadaBridge.Commons.csproj" />
@@ -0,0 +1,4 @@
{
"$schema": "https://xunit.net/schema/current/xunit.runner.schema.json",
"maxParallelThreads": 4
}