fix(test): order the cached-drain MarkForwarded assertions behind the push they follow

CachedDrain_OrphanRow_PastGrace_IsAbandoned_AndTheValidRowStillFlows gated on
IngestCachedTelemetryAsync being received once and then asserted, bare, that the
valid row had been marked Forwarded. The drain does that strictly AFTER the push
returns: OnCachedDrainAsync abandons the orphan (:351), pushes the batch (:366),
then parses the ack and marks the accepted ids (:380). Observing the push
therefore orders nothing with respect to the second MarkForwardedAsync — under a
loaded parallel run the post-push continuation can be scheduled after the poll
that saw the push, and the assertion fails fast with "Actually received no
matching calls" while the orphan's own earlier call is reported as the single
non-matching one.

Reproduced deterministically by delaying only the post-push step, which fails
exactly this test (11 siblings still pass) at ~1.3s into the assembly run —
matching the observed failure's fast-fail signature and pointing at line 430.
With the fix the same injected delay passes; suppressing the valid row's
MarkForwarded entirely still fails the test with the identical message, so the
claim (orphan abandoned in its own call, valid row pushed and marked, exactly
once each with exactly the same arguments) is unchanged in force.

Same unsynchronized-assertion class as c4caebe9, different actor. Test-only; the
drain's abandon/push/mark ordering is correct as written.
This commit is contained in:
Joseph Doherty
2026-08-14 23:28:31 -04:00
parent 49fb75c8ba
commit cfa6acbf48
@@ -416,15 +416,23 @@ public class SiteAuditTelemetryActorTests : TestKit
// Assert — exactly one push containing ONLY the valid row: the orphan // Assert — exactly one push containing ONLY the valid row: the orphan
// has no operational half to send. // has no operational half to send.
//
// Both MarkForwardedAsync expectations live INSIDE the awaited block
// rather than after it. The valid row is marked Forwarded only after
// the push returns and its ack is parsed (OnCachedDrainAsync: abandon,
// then push, then mark), so observing the push establishes no
// happens-before edge with that call — asserting it bare assumed one.
// On a loaded parallel run the drain's post-push continuation can be
// scheduled after the poll that saw the push, and the bare assertion
// then failed with "Actually received no matching calls" while the
// orphan's own earlier call appeared as the single non-matching one.
// The bounded wait removes the ordering assumption only: each call must
// still occur exactly once, with exactly the same arguments, or the
// test fails.
await AwaitAssertAsync(async () => await AwaitAssertAsync(async () =>
{ {
await _client.Received(1).IngestCachedTelemetryAsync( await _client.Received(1).IngestCachedTelemetryAsync(
Arg.Any<CachedTelemetryBatch>(), Arg.Any<CancellationToken>()); Arg.Any<CachedTelemetryBatch>(), Arg.Any<CancellationToken>());
}, TimeSpan.FromSeconds(5));
Assert.NotNull(capturedBatch);
Assert.Single(capturedBatch!.Packets);
Assert.Equal(valid.EventId.ToString(), capturedBatch.Packets[0].AuditEvent.EventId);
// The valid row is marked Forwarded because central ack'd it... // The valid row is marked Forwarded because central ack'd it...
await _queue.Received(1).MarkForwardedAsync( await _queue.Received(1).MarkForwardedAsync(
@@ -437,6 +445,13 @@ public class SiteAuditTelemetryActorTests : TestKit
await _queue.Received(1).MarkForwardedAsync( await _queue.Received(1).MarkForwardedAsync(
Arg.Is<IReadOnlyList<Guid>>(g => g.Count == 1 && g[0] == orphan.EventId), Arg.Is<IReadOnlyList<Guid>>(g => g.Count == 1 && g[0] == orphan.EventId),
Arg.Any<CancellationToken>()); Arg.Any<CancellationToken>());
}, TimeSpan.FromSeconds(10));
// Safe as bare assertions: capturedBatch is captured synchronously by
// the push stub, so the awaited gate above already ordered them.
Assert.NotNull(capturedBatch);
Assert.Single(capturedBatch!.Packets);
Assert.Equal(valid.EventId.ToString(), capturedBatch.Packets[0].AuditEvent.EventId);
} }
[Fact] [Fact]