test: deflake load-sensitive tests (lmxopcua#769)

- NodeManagerLockTimeBenchmark: wall-clock budget enforced only with OTOPCUA_BENCH=1; default run keeps the state-correctness asserts and logs timings. - OpcUaClientDriverWire host-status: signal (semaphore) wait that only accepts a non-Running state observed after the server is disposed. - AbCip PollLatencyComparison: retry once on a fixture connection reset; p95 criterion enforced only with AB_LAT_ENFORCE=1. - ScriptRunGuardTests: orphan scripts held by a ctx.Released signal instead of a 250 ms wall-clock spin that could finish inside the 60 ms budget under load; stuck-orphan wall-clock rule disabled in the tests that pin the strike counting. - DependencyMuxReplayTests: split the inside-grace positive control (1 h grace) from the release test (control asserted before unregister), removing the 200 ms grace race. Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
2026-11-10 05:39:22 -05:00
parent 000d7f5591
commit 11a5c1d4e2
6 changed files with 128 additions and 59 deletions
@@ -212,20 +212,26 @@ public sealed class OpcUaClientDriverWireTests
            await drv.InitializeAsync("{}", ct);
            drv.GetHostStatuses().Single().State.ShouldBe(HostState.Running);

-            var transitioned = new TaskCompletionSource<HostState>(TaskCreationOptions.RunContinuationsAsynchronously);
-            drv.OnHostStatusChanged += (_, e) =>
-            {
-                if (e.NewState != HostState.Running) transitioned.TrySetResult(e.NewState);
-            };
+            // Signal-based (lmxopcua#769): every host-status event releases the semaphore, and the loop re-reads the
+            // driver's state. A transient non-Running tick BEFORE the server is gone (a starved keep-alive) must not
+            // satisfy the wait, so the loop only accepts a non-Running state observed AFTER the rig is disposed; the
+            // 120 s ceiling only matters on a starved box and costs nothing when the signal arrives.
+            using var signal = new SemaphoreSlim(0);
+            drv.OnHostStatusChanged += (_, _) => signal.Release();

            await rig.DisposeAsync();
            rig = null;
+            signal.Release(); // re-evaluate once now that the server is certainly gone
+
+            var deadline = TimeSpan.FromSeconds(120);
+            var sw = System.Diagnostics.Stopwatch.StartNew();
+            while (drv.GetHostStatuses().Single().State == HostState.Running)
+            {
+                var remaining = deadline - sw.Elapsed;
+                (await signal.WaitAsync(remaining > TimeSpan.Zero ? remaining : TimeSpan.Zero, ct))
+                    .ShouldBeTrue("the host probe never left Running after the server stopped");
+            }

-            // Signal-based (lmxopcua#769): the wait completes the instant the keep-alive loop reports the loss, so
-            // the ceiling only matters on a starved box, where server teardown plus the 10 s request timeout plus a
-            // 1 s keep-alive tick exceeded the old 30 s. A generous ceiling costs nothing when the signal arrives.
-            var state = await transitioned.Task.WaitAsync(TimeSpan.FromSeconds(120), ct);
-            state.ShouldNotBe(HostState.Running);
            drv.GetHostStatuses().Single().State.ShouldNotBe(HostState.Running);
        }
        finally
@@ -20,8 +20,9 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests;
 ///     </list>
 ///     The wall-clock budgets are generous for CI hardware; the before/after numbers are in the commit
 ///     message. P3 is pinned by the bookkeeping's own visit counter, which does not depend on the machine.
-///     Tagged <c>Category=Benchmark</c>, but no filter excludes that category (CI drops only E2E + LiveIntegration),
-///     so it runs by default — on purpose:
+///     lmxopcua#769: the wall-clock legs only ASSERT their budget when <c>OTOPCUA_BENCH=1</c> (opt-in, like
+///     <c>OTOPCUA_SCRIPT_PERF</c>); the default run still executes every teardown and asserts the state correctness.
+///     Historical note on the budget: no filter excludes <c>Category=Benchmark</c> (CI drops only E2E + LiveIntegration):
 ///     measured after the fix at 5–6 ms (P1) and 58–64 ms (P2) on a loaded dev machine against the 1 s budget —
 ///     ≥ 15× headroom, so it does not flake on slow CI. The flip side: the pre-fix code (75–150 ms / 470–680 ms) also
 ///     fits 1 s at these sizes, so the wall-clock legs guard against gross (order-of-magnitude) regressions only; the
@@ -35,6 +36,18 @@ public sealed class NodeManagerLockTimeBenchmarkTests(ITestOutputHelper output)
    // >= 80x headroom against a CPU-starved CI box while still catching an order-of-magnitude (O(N^2)) regression.
    private static readonly TimeSpan Budget = TimeSpan.FromSeconds(5);

+    // lmxopcua#769: even 5 s flaked in a full parallel run on a starved box, so the wall-clock legs are opt-in, like
+    // ScriptRunnerPerfTests (OTOPCUA_SCRIPT_PERF=1): OTOPCUA_BENCH=1 enforces the budget. The default run still executes
+    // every teardown and asserts the state correctness (counts, SDK handles, re-population) and logs the measured time.
+    private static bool EnforceBudget => Environment.GetEnvironmentVariable("OTOPCUA_BENCH") == "1";
+
+    private void AssertWithinBudget(Stopwatch sw, string what)
+    {
+        output.WriteLine($"BENCH {what}: {sw.ElapsedMilliseconds} ms (budget {Budget.TotalMilliseconds:F0} ms, enforced={EnforceBudget})");
+        if (EnforceBudget)
+            sw.Elapsed.ShouldBeLessThan(Budget, $"{what} took {sw.ElapsedMilliseconds} ms");
+    }
+
    private readonly string _pkiRoot = Path.Combine(Path.GetTempPath(), $"otopcua-bench-{Guid.NewGuid():N}");

    /// <summary>P1 — a rebuild that discards one device folder holding 10k tags completes within the budget.</summary>
@@ -64,8 +77,7 @@ public sealed class NodeManagerLockTimeBenchmarkTests(ITestOutputHelper output)
            nm.EnsureVariable($"{device}/T0", device, "T0", "Int32", writable: false, realm: AddressSpaceRealm.Raw);
            nm.TryGetVariable($"{device}/T0", AddressSpaceRealm.Raw)!.Parent.ShouldBeSameAs(nm.TryGetFolder(device, AddressSpaceRealm.Raw));

-            output.WriteLine($"BENCH rebuild 10k-tag folder: {sw.ElapsedMilliseconds} ms");
-            sw.Elapsed.ShouldBeLessThan(Budget, $"rebuild of a 10k-tag folder took {sw.ElapsedMilliseconds} ms");
+            AssertWithinBudget(sw, "rebuild 10k-tag folder");
        }
        finally { await host.DisposeAsync(); }
    }
@@ -101,8 +113,7 @@ public sealed class NodeManagerLockTimeBenchmarkTests(ITestOutputHelper output)
            nm.TryGetVariable("area/line/keep/S0").ShouldNotBeNull();
            nm.GetManagerHandle(new NodeId("area/line/eq0/S0", nm.NamespaceIndexForRealm(AddressSpaceRealm.Uns))).ShouldBeNull();

-            output.WriteLine($"BENCH remove 200 equipment / 50k nodes: {sw.ElapsedMilliseconds} ms");
-            sw.Elapsed.ShouldBeLessThan(Budget, $"removing 200 equipment took {sw.ElapsedMilliseconds} ms");
+            AssertWithinBudget(sw, "remove 200 equipment / 50k nodes");
        }
        finally { await host.DisposeAsync(); }
    }
@@ -90,8 +90,27 @@ public sealed class DependencyMuxReplayTests : RuntimeActorTestBase
        mux.UnderlyingActor.RetainedRefCount.ShouldBe(2);
    }

-    /// <summary>I2 — when the last replay-opting subscriber of a ref leaves, its retained value survives a short
-    /// grace (the restart window: old adapter gone, new one not yet registered) and is then dropped.</summary>
+    /// <summary>I2 — when the last replay-opting subscriber of a ref leaves, its retained value survives the grace
+    /// (the restart window: old adapter gone, new one not yet registered). Long grace, so no wall-clock race: the
+    /// returning subscriber is guaranteed to land inside it however loaded the machine is.</summary>
+    [Fact]
+    public void Retention_survives_the_grace_for_a_returning_replay_subscriber()
+    {
+        var mux = ActorOfAsTestActorRef<DependencyMuxActor>(DependencyMuxActor.Props(TimeSpan.FromHours(1)));
+        var gen1 = CreateTestProbe();
+        mux.Tell(new DependencyMuxActor.RegisterInterest(new[] { "a" }, gen1.Ref, ReplayRetained: true));
+        mux.Tell(Burst(("a", 1)));
+        gen1.ExpectMsg<VirtualTagActor.DependencyValueBatch>(PresenceBudget);
+        mux.Tell(new DependencyMuxActor.UnregisterInterest(gen1.Ref));
+
+        var gen2 = CreateTestProbe();
+        mux.Tell(new DependencyMuxActor.RegisterInterest(new[] { "a" }, gen2.Ref, ReplayRetained: true));
+        gen2.ExpectMsg<VirtualTagActor.DependencyValueBatch>(PresenceBudget).Items.Single().Value.ShouldBe(1);
+    }
+
+    /// <summary>I2 — once the grace is over, the retained value is dropped, no entry is left behind, and a
+    /// returning subscriber replays nothing. The positive control (the value WAS retained) is asserted before the
+    /// last subscriber leaves, where it cannot race the grace; the release itself is awaited, never slept.</summary>
    [Fact]
    public void Retention_is_released_after_the_grace_once_no_replay_subscriber_remains()
    {
@@ -100,14 +119,9 @@ public sealed class DependencyMuxReplayTests : RuntimeActorTestBase
        mux.Tell(new DependencyMuxActor.RegisterInterest(new[] { "a" }, gen1.Ref, ReplayRetained: true));
        mux.Tell(Burst(("a", 1)));
        gen1.ExpectMsg<VirtualTagActor.DependencyValueBatch>(PresenceBudget);
+        mux.UnderlyingActor.RetainedRefCount.ShouldBe(1, "positive control: the value is retained while a replay subscriber is registered");
        mux.Tell(new DependencyMuxActor.UnregisterInterest(gen1.Ref));

-        // Positive control: inside the grace a returning subscriber still gets the value.
-        var gen2 = CreateTestProbe();
-        mux.Tell(new DependencyMuxActor.RegisterInterest(new[] { "a" }, gen2.Ref, ReplayRetained: true));
-        gen2.ExpectMsg<VirtualTagActor.DependencyValueBatch>(PresenceBudget).Items.Single().Value.ShouldBe(1);
-        mux.Tell(new DependencyMuxActor.UnregisterInterest(gen2.Ref));
-
        // Past the grace: nothing retained, no entry left behind, and a returning subscriber replays nothing.
        AwaitAssert(() =>
        {