test(auditlog): close register row 95 — stop the three remaining leaked fast-tick actors

Row 82 (da23f2dc) fixed this leak shape in PartitionPurgeTests and
NotifyDispatcherAuditTrailTests; three classes in the same assembly still had
it. Each creates an actor with a fast periodic tick (100 ms purge override,
~0 s reconciliation override) against a shared MsSqlMigrationFixture database
and never stops it deterministically, relying on TestKit's Dispose -> Shutdown
— which is timeout-bounded and does NOT fail when it expires. A survivor keeps
mutating the fixture DB while LATER test classes run.

All three now use the same Watch(actor) / Sys.Stop(actor) /
ExpectTerminated(actor) helper da23f2dc established, applied at EVERY
actor-creation site — 11 in AuditLogPurgeActorTests, 10 in
SiteAuditReconciliationActorTests, 2 in OutageReconciliationTests.

The two real-DB cases are the ones that could corrupt another class:
AuditLogPurgeActorTests' EndToEnd_RealPartition_… keeps switching partitions
out of the shared DB, and SiteAuditReconciliationActorTests'
Tick_Duplicates_NotDoubleInserted holds a live AuditLogRepository over a
DbContext the test is about to dispose. The stub-repo cases are cheaper but not
free: each burns a DI scope and a thread every 100 ms for the rest of the
assembly run.

OutageReconciliationTests needed more than the helper. It already called
Sys.Stop, but (a) fire-and-forget, so the call returns before the in-flight tick
has finished writing to MSSQL, and (b) from the LAST line of the enclosing
`await using` block, so any earlier assertion failure skipped it entirely. Both
stops move to immediately after the last step that needs the actor alive —
before the read-back that counts rows — and the AuditLogIngestActor that
BuildCentralPipeline creates is now stopped too (test 2 was discarding its ref
outright, leaving it holding a service provider the block then disposed).

Where a test's remaining assertions read a collection the actor appends to
(client.Calls indexing, Assert.Empty on a switched-boundaries list), the stop is
placed BEFORE them so they no longer read a moving target.

AuditLog.Tests: 383/383 green (1 m 51 s), 0 skipped.
This commit is contained in:
Joseph Doherty
2026-08-17 05:27:37 -04:00
parent 26e3a1b9a6
commit 34a8079d15
3 changed files with 184 additions and 29 deletions
@@ -165,6 +165,37 @@ public class OutageReconciliationTests : TestKit, IClassFixture<MsSqlMigrationFi
return (sp, ingest);
}
/// <summary>
/// Stops an actor and blocks until it is actually dead.
/// </summary>
/// <remarks>
/// <para>
/// Deferred-work register row 95, the same leak shape row 82 fixed in
/// <c>PartitionPurgeTests</c> / <c>NotifyDispatcherAuditTrailTests</c>. Both tests here
/// already called <c>Sys.Stop</c>, but that is fire-and-forget: it returns before the
/// actor has processed the stop, so an in-flight 100 ms tick could still be pulling and
/// INSERTING into the shared <see cref="MsSqlMigrationFixture"/> database — and into the
/// <see cref="IServiceProvider"/> the enclosing <c>await using</c> is about to dispose.
/// Worse, the call sat at the very END of the block, so any earlier assertion failure
/// skipped it entirely and left the actor ticking for the rest of the assembly run,
/// stopped only by <see cref="TestKit"/>'s <c>Dispose</c> → <c>Shutdown</c>, which is
/// timeout-bounded and does not fail when it expires.
/// </para>
/// <para>
/// Watch/Stop/ExpectTerminated makes the stop synchronous, and each call site now sits
/// immediately after the last step that needs the actor alive rather than at the end.
/// </para>
/// </remarks>
private void StopActor(IActorRef actor)
{
Watch(actor);
Sys.Stop(actor);
// A stop is processed after the in-flight tick completes, so this also guarantees no
// pull or insert is still running against the fixture DB.
ExpectTerminated(actor, TimeSpan.FromSeconds(30));
Unwatch(actor);
}
private static SiteAuditReconciliationOptions FastTickOptions(int batchSize = 256) => new()
{
ReconciliationIntervalSeconds = 300,
@@ -265,6 +296,12 @@ public class OutageReconciliationTests : TestKit, IClassFixture<MsSqlMigrationFi
duration: TimeSpan.FromSeconds(10),
interval: TimeSpan.FromMilliseconds(100));
// Both actors have done everything the test needs; stop them HERE, before the
// read-back, so nothing is still writing to the shared fixture DB while step 5
// counts rows and so a step-5 failure cannot leak them (register row 95).
StopActor(reconciliationActor);
StopActor(ingest);
// Step 5: assert no duplicates by EventId — central must have
// exactly the 200 rows we wrote at the site (one row per EventId).
await using var verify = CreateContext();
@@ -277,10 +314,6 @@ public class OutageReconciliationTests : TestKit, IClassFixture<MsSqlMigrationFi
// And every EventId we wrote at the site is present centrally.
Assert.True(written.All(w => centralIds.Contains(w.EventId)),
"every site-written EventId should be present centrally.");
// Tear the actor down before disposing the harness; the actor's
// PostStop cancels its scheduled timer.
Sys.Stop(reconciliationActor);
}
}
@@ -303,7 +336,7 @@ public class OutageReconciliationTests : TestKit, IClassFixture<MsSqlMigrationFi
await sqliteWriter.WriteAsync(NewEvent(siteId, baseOccurred.AddMilliseconds(i)));
}
var (sp, _) = BuildCentralPipeline();
var (sp, ingest) = BuildCentralPipeline();
await using (sp as IAsyncDisposable ?? throw new InvalidOperationException())
{
var pullClient = new QueueBackedPullClient(sqliteWriter);
@@ -338,6 +371,13 @@ public class OutageReconciliationTests : TestKit, IClassFixture<MsSqlMigrationFi
Assert.True(pullClient.CallCount > callsAfterDrain,
$"expected additional pull calls after drain to validate idempotency, got {pullClient.CallCount} after {callsAfterDrain}");
// Both actors have done everything the test needs; stop them HERE, before the
// read-back, so nothing is still writing to the shared fixture DB while the
// duplicate check counts rows and so a failure below cannot leak them
// (register row 95).
StopActor(reconciliationActor);
StopActor(ingest);
// Central count must still be exactly totalEvents — no duplicates
// even though the cursor + read-Reconciled-too semantics could
// theoretically re-fetch on the second cycle.
@@ -347,8 +387,6 @@ public class OutageReconciliationTests : TestKit, IClassFixture<MsSqlMigrationFi
.ToListAsync();
Assert.Equal(totalEvents, rows.Count);
Assert.Equal(totalEvents, rows.Select(r => r.EventId).Distinct().Count());
Sys.Stop(reconciliationActor);
}
}
}