Two alarm-ack tests assert an absence with AwaitAssert, so they pass instantly and prove nothing #501

New Issue

2026-07-25T18:07:33-04:00

dohertj2 commented

2026-07-25 18:07:33 -04:00

Found while root-causing #500. Not a flake - these always pass; the problem is that they pass without testing anything.

DriverHostActorNativeAlarmAckRoutingTests.cs:90 and :116:

// Give the (fire-and-forget) handler time to run; the unmapped node must produce no ack.
AwaitAssert(() => recorder.Acks.ShouldBeEmpty(), duration: TimeSpan.FromMilliseconds(800));

AwaitAssert polls and returns on the first success. recorder.Acks is empty at t=0, so the very first poll succeeds and the call returns immediately - the 800 ms is never spent. The comment says "give the handler time to run", but no time is given at all.

Consequence: if the routing regressed and an ack were produced for an unmapped node (or on a Secondary), these tests would still very likely pass, because the assertion is evaluated before the fire-and-forget handler has had any chance to run.

AwaitAssert is the right tool for a presence assertion (wait until X becomes true) and the wrong tool for an absence one (confirm X never becomes true). The absence form needs settle-then-assert:

ExpectNoMsg(TimeSpan.FromMilliseconds(800));
recorder.Acks.ShouldBeEmpty();

DriverHostActorUnreadableArtifactTests already does this correctly and documents why its settle window is calibrated against a positive control - that is the pattern to copy.

Note before fixing: changing these may turn them red. If it does, that is a genuine defect being surfaced, not a test regression, so it wants doing deliberately rather than folded into an unrelated change.

Checked and NOT affected: PeerProbeSupervisorTests's ChildCount.ShouldBe(0) waits are each preceded by a ShouldBe(1), so they assert a real transition rather than the initial state.

Found while root-causing #500. **Not a flake** - these always pass; the problem is that they pass without testing anything. `DriverHostActorNativeAlarmAckRoutingTests.cs:90` and `:116`: ```csharp // Give the (fire-and-forget) handler time to run; the unmapped node must produce no ack. AwaitAssert(() => recorder.Acks.ShouldBeEmpty(), duration: TimeSpan.FromMilliseconds(800)); ``` `AwaitAssert` polls and returns on the **first success**. `recorder.Acks` is empty at t=0, so the very first poll succeeds and the call returns immediately - the 800 ms is never spent. The comment says "give the handler time to run", but no time is given at all. Consequence: if the routing regressed and an ack *were* produced for an unmapped node (or on a Secondary), these tests would still very likely pass, because the assertion is evaluated before the fire-and-forget handler has had any chance to run. `AwaitAssert` is the right tool for a **presence** assertion (wait until X becomes true) and the wrong tool for an **absence** one (confirm X never becomes true). The absence form needs settle-then-assert: ```csharp ExpectNoMsg(TimeSpan.FromMilliseconds(800)); recorder.Acks.ShouldBeEmpty(); ``` `DriverHostActorUnreadableArtifactTests` already does this correctly and documents why its settle window is calibrated against a positive control - that is the pattern to copy. **Note before fixing:** changing these may turn them red. If it does, that is a genuine defect being surfaced, not a test regression, so it wants doing deliberately rather than folded into an unrelated change. Checked and NOT affected: `PeerProbeSupervisorTests`'s `ChildCount.ShouldBe(0)` waits are each preceded by a `ShouldBe(1)`, so they assert a real transition rather than the initial state.

dohertj2 referenced this issue

2026-07-25 18:39:57 -04:00

50 sub-second presence budgets remain in Runtime.Tests (the class behind three of #500's five defects) #502

dohertj2 referenced this issue

2026-07-25 20:20:36 -04:00

Intermittent single-test failure in Runtime.Tests under full-sweep load (unidentified) #500

dohertj2 referenced this issue

2026-07-25 20:20:37 -04:00

50 sub-second presence budgets remain in Runtime.Tests (the class behind three of #500's five defects) #502

dohertj2 referenced this issue from a commit

2026-07-25 20:24:23 -04:00

test(runtime): fix the intermittent Runtime.Tests failure — 4 ordering/logic defects + the presence-budget class (#500)