From 462850afb749346d13fca35d6e1895ead24a7804 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 18 Aug 2026 07:01:16 -0400 Subject: [PATCH] fix(worker-tests): doc-comment placement + watchdog-window headroom in long-in-flight test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings on 7da52b6: 1. The AssertNotFault doc block landed between the predicate-overload ReadUntilAsync's doc block and the helper, so the compiler attached the merged block to the helper and ReadUntilAsync lost its docs entirely. Helper and its docs moved above ReadUntilAsync, whose docs are back where they belong. 2. The compressed watchdog windows (50 ms grace, 100 ms ceiling) reintroduced the load-sensitivity the fix removed, one layer down. ReportWatchdogFaultIfNeededAsync measures staleness AFTER the heartbeat frame is written and flushed over the real named pipe, so a beat whose pipe I/O outlasts the ceiling faults a healthy session no matter how fresh the captured activity was. At 100 ms that is a plausible stall on a loaded box, and this is the one test asserting the watchdog NEVER fires. Widened to a 200 ms grace and a 1 s ceiling — still two orders of magnitude under the 75 s production default. The inspection loop is now bounded by a 2 s window (twice the ceiling, so a fake whose activity stopped advancing still accumulates past it and faults) with a 30-frame floor, rather than a fixed 30 frames that no longer outran the wider ceiling. The floor keeps a window that saw almost no beats from passing as a clean one. Two seconds stays well inside FakeRuntimeSession's 5 s dispatch-block ceiling, so the command is still in flight for the whole window. 3. AssertNotFault renamed AssertNotWorkerFault, matching the WorkerFault body case it tests. Scenario intent unchanged: long in-flight command, pump refreshing, zero fault frames, reply delivered. Test-only. --- .../Ipc/WorkerPipeSessionTests.cs | 61 +++++++++++-------- 1 file changed, 37 insertions(+), 24 deletions(-) diff --git a/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs b/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs index 30e7044..1bef756 100644 --- a/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.IO.Pipes; using System.Runtime.InteropServices; @@ -1451,17 +1452,17 @@ public sealed class WorkerPipeSessionTests /// refresh LastActivityUtc on every wait iteration, so a healthy /// ReadBulk holding the STA far longer than /// HeartbeatStuckCeiling (75 s in production) keeps its activity - /// timestamp fresh. This test compresses the clock — a 100 ms ceiling - /// with a command in flight across a window many multiples longer — and - /// models the pump refresh with + /// timestamp fresh. This test compresses the clock — a 1 s ceiling with + /// a command in flight across a window twice as long — and models the + /// pump refresh with /// , which /// stamps activity at every heartbeat capture exactly as the pump's /// per-iteration MarkActivity() does. The refresh has to be in - /// effect from construction, not from the moment the command blocks: - /// with a 50 ms grace, the idle window covering handshake and startup - /// carries no correlation id for the watchdog to suppress on, so a fake - /// whose activity timestamp is frozen at construction is reported - /// StaHung before the scenario under test even starts. Contrast + /// effect from construction, not from the moment the command blocks: the + /// idle window covering handshake and startup carries no correlation id + /// for the watchdog to suppress on, so a fake whose activity timestamp is + /// frozen at construction is reported StaHung before the scenario + /// under test even starts. Contrast /// , /// where a frozen timestamp beyond the ceiling correctly faults; here /// the refreshed timestamp must keep the fault suppressed and let the @@ -1489,9 +1490,15 @@ public sealed class WorkerPipeSessionTests runtime, new WorkerPipeSessionOptions { + // Compressed relative to production (75 s ceiling), but no further than the real + // pipe underneath can carry. ReportWatchdogFaultIfNeededAsync measures staleness + // AFTER the heartbeat frame has been written and flushed, so any beat whose pipe + // I/O outlasts the ceiling faults a healthy session. At a 100 ms ceiling that is a + // plausible stall on a loaded box; at 1 s it is not — and this is the one test + // asserting the watchdog NEVER fires, so it has to hold under load. HeartbeatInterval = TimeSpan.FromMilliseconds(20), - HeartbeatGrace = TimeSpan.FromMilliseconds(50), - HeartbeatStuckCeiling = TimeSpan.FromMilliseconds(100), + HeartbeatGrace = TimeSpan.FromMilliseconds(200), + HeartbeatStuckCeiling = TimeSpan.FromSeconds(1), }); Task runTask = session.RunAsync(cancellation.Token); await CompleteGatewayHandshakeAsync(pipePair, cancellation.Token); @@ -1514,16 +1521,22 @@ public sealed class WorkerPipeSessionTests lastEventSequence: 0, currentCommandCorrelationId: "long-bulk-read")); - // Inspect a bounded number of frames over a window many multiples of the - // 100 ms ceiling (at least 30 heartbeats at 20 ms ~ 600 ms). None may be - // a WorkerFault while activity is continuously refreshed. - const int framesToInspect = 30; + // Inspect frames across a window twice the stuck ceiling — long enough that a fake whose + // activity timestamp stopped advancing would accumulate staleness past the ceiling and + // fault — and require the beats to have actually flowed while it ran, so an inspection + // that saw a couple of frames and timed out cannot pass for a clean window. None may be a + // WorkerFault while activity is continuously refreshed. The window stays well inside + // FakeRuntimeSession's 5 s dispatch-block ceiling, so the command is still in flight + // throughout. + TimeSpan inspectionWindow = TimeSpan.FromSeconds(2); + const int minimumFramesInspected = 30; + Stopwatch inspection = Stopwatch.StartNew(); int frameIndex = 0; - for (; frameIndex < framesToInspect; frameIndex++) + while (inspection.Elapsed < inspectionWindow || frameIndex < minimumFramesInspected) { WorkerEnvelope envelope = await pipePair.GatewayReader .ReadAsync(cancellation.Token); - AssertNotFault(envelope, frameIndex); + AssertNotWorkerFault(envelope, frameIndex++); } // Release the command with the pump still running — as it is in @@ -1537,7 +1550,7 @@ public sealed class WorkerPipeSessionTests { WorkerEnvelope envelope = await pipePair.GatewayReader .ReadAsync(cancellation.Token); - AssertNotFault(envelope, frameIndex++); + AssertNotWorkerFault(envelope, frameIndex++); if (envelope.BodyCase == WorkerEnvelope.BodyOneofCase.WorkerCommandReply && envelope.CorrelationId == "long-bulk-read") { @@ -2224,12 +2237,6 @@ public sealed class WorkerPipeSessionTests cancellationToken); } - /// Reads frames until one matches the expected body type and predicate. - /// Frame reader. - /// Expected body case. - /// Predicate to match against envelope. - /// Token to cancel the asynchronous operation. - /// The matching envelope. /// /// Fails when the frame is a WorkerFault, naming the category and diagnostic message. /// A bare body-case comparison reports only "expected not WorkerFault", which says nothing @@ -2238,7 +2245,7 @@ public sealed class WorkerPipeSessionTests /// /// Frame read from the gateway end. /// Ordinal of the frame within the inspected run. - private static void AssertNotFault(WorkerEnvelope envelope, int frameIndex) + private static void AssertNotWorkerFault(WorkerEnvelope envelope, int frameIndex) { if (envelope.BodyCase != WorkerEnvelope.BodyOneofCase.WorkerFault) { @@ -2250,6 +2257,12 @@ public sealed class WorkerPipeSessionTests + envelope.WorkerFault.DiagnosticMessage); } + /// Reads frames until one matches the expected body type and predicate. + /// Frame reader. + /// Expected body case. + /// Predicate to match against envelope. + /// Token to cancel the asynchronous operation. + /// The matching envelope. private static async Task ReadUntilAsync( WorkerFrameReader reader, WorkerEnvelope.BodyOneofCase expectedBody,