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.
This commit is contained in:
+14
@@ -330,8 +330,22 @@ public class AuditWriteFailureSafetyTests : TestKit, IClassFixture<MsSqlMigratio
|
|||||||
Assert.NotNull(row.DeliveredAt);
|
Assert.NotNull(row.DeliveredAt);
|
||||||
}, TimeSpan.FromSeconds(15));
|
}, TimeSpan.FromSeconds(15));
|
||||||
|
|
||||||
|
// AwaitAssert, not a bare Assert: the dispatcher persists the delivery
|
||||||
|
// state BEFORE emitting the audit rows (DeliverOneAsync updates the
|
||||||
|
// row, then emits Attempted, then the terminal), so seeing Delivered
|
||||||
|
// above orders nothing with respect to the audit write — on a loaded
|
||||||
|
// parallel run the post-write continuation can land after the poll that
|
||||||
|
// observed Delivered, producing a spurious "saw 0". The bounded wait
|
||||||
|
// keeps the assertion's force: the writer must actually be invoked
|
||||||
|
// within the timeout or the test fails exactly as before.
|
||||||
|
await AwaitAssertAsync(
|
||||||
|
() =>
|
||||||
|
{
|
||||||
Assert.True(throwingWriter.Attempts >= 1,
|
Assert.True(throwingWriter.Attempts >= 1,
|
||||||
$"Expected dispatcher to attempt audit write at least once; saw {throwingWriter.Attempts}.");
|
$"Expected dispatcher to attempt audit write at least once; saw {throwingWriter.Attempts}.");
|
||||||
|
return Task.CompletedTask;
|
||||||
|
},
|
||||||
|
TimeSpan.FromSeconds(15));
|
||||||
}
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------
|
// ---------------------------------------------------------------------
|
||||||
|
|||||||
+16
@@ -324,8 +324,24 @@ public class NotifyDispatcherAuditTrailTests : TestKit, IClassFixture<MsSqlMigra
|
|||||||
// The writer was attempted (at least once for the Attempted row, plus
|
// The writer was attempted (at least once for the Attempted row, plus
|
||||||
// once for the Delivered terminal) — proves the dispatcher tried to
|
// once for the Delivered terminal) — proves the dispatcher tried to
|
||||||
// emit and absorbed the throws rather than aborting the action.
|
// emit and absorbed the throws rather than aborting the action.
|
||||||
|
//
|
||||||
|
// 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,
|
Assert.True(throwingWriter.AttemptCount >= 2,
|
||||||
$"Expected the dispatcher to attempt audit writes; saw {throwingWriter.AttemptCount}");
|
$"Expected the dispatcher to attempt audit writes; saw {throwingWriter.AttemptCount}");
|
||||||
|
return Task.CompletedTask;
|
||||||
|
},
|
||||||
|
TimeSpan.FromSeconds(15));
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
|
|||||||
Reference in New Issue
Block a user