fix(test): remove the unsynchronized audit-attempt assertion in the two dispatcher audit-safety tests

Both NotifyDispatcher_AuditWriter_Throws_DeliveryStillSucceeds and NotificationDispatch_BrokenAuditWriter_StillTransitionsToDelivered read the throwing writer's attempt counter with a bare Assert immediately after an AwaitAssert on the Notifications row reaching Delivered. That assumes the audit writes happen no later than the operational status write, which the dispatcher deliberately does NOT guarantee: DeliverOneAsync persists the delivery state first (NotificationOutboxActor.cs:657) and only then emits the Attempted (:663) and terminal (:676) audit rows — audit is best-effort and must never gate the user-facing action. Observing Delivered therefore establishes no happens-before edge with the writer, and under a loaded full-solution parallel run the continuation after the DB write can be scheduled after the poll that saw Delivered, so the counter reads 0 and the test fails with "saw 0". Reproduced deterministically by delaying only the post-update audit emission, which yields both observed failure messages verbatim; with the fix in place the same injected delay passes, and suppressing the emissions entirely still fails both tests with the identical messages — the claims (delivery despite audit failure, and attempts >= N) are unchanged in force, only the ordering assumption is gone. Test-only change; the update-then-audit ordering predates the remediation (#23 M4) and is correct as written.
2026-08-14 23:12:06 -04:00
parent b1de9dfdd4
commit c4caebe9b4
2 changed files with 34 additions and 4 deletions
@@ -324,8 +324,24 @@ public class NotifyDispatcherAuditTrailTests : TestKit, IClassFixture<MsSqlMigra
        // The writer was attempted (at least once for the Attempted row, plus
        // once for the Delivered terminal) — proves the dispatcher tried to
        // emit and absorbed the throws rather than aborting the action.
-        Assert.True(throwingWriter.AttemptCount >= 2,
-            $"Expected the dispatcher to attempt audit writes; saw {throwingWriter.AttemptCount}");
+        //
+        // AwaitAssert, not a bare Assert: the dispatcher deliberately persists
+        // the delivery state BEFORE emitting either audit row (DeliverOneAsync
+        // writes the row, then Attempted, then the terminal), so observing
+        // Delivered above establishes NO happens-before edge with the audit
+        // writes — under a loaded parallel run the continuation after the DB
+        // write can be scheduled after the poll that saw Delivered, yielding a
+        // spurious "saw 0". The bounded wait removes the ordering assumption
+        // without weakening the claim: the writer must genuinely be invoked at
+        // least twice inside the timeout or the test still fails.
+        await AwaitAssertAsync(
+            () =>
+            {
+                Assert.True(throwingWriter.AttemptCount >= 2,
+                    $"Expected the dispatcher to attempt audit writes; saw {throwingWriter.AttemptCount}");
+                return Task.CompletedTask;
+            },
+            TimeSpan.FromSeconds(15));
    }

    /// <summary>