diff --git a/docs/test_infra/test_infra.md b/docs/test_infra/test_infra.md index f78828d4..9aad5b4f 100644 --- a/docs/test_infra/test_infra.md +++ b/docs/test_infra/test_infra.md @@ -46,6 +46,64 @@ Each service has a dedicated document with configuration details, verification s - [test_infra_playwright.md](test_infra_playwright.md) — Playwright browser server (Central UI testing) - Traefik LB — see `docker/README.md` and `docker/traefik/` (runs with the cluster, not in `infra/`) +## Test Parallelism Policy + +There are **two independent parallelism axes**, and they are bounded by two different mechanisms. +Conflating them is what made the row-82/91/96 flake family hard to read: a fix on one axis leaves the +other untouched. + +| Axis | What runs at once | Bounded by | +|------|-------------------|-----------| +| **In-process** — xunit collection threads | Test collections *within one test assembly*. Default is `Environment.ProcessorCount` (14 on the current dev box). | `xunit.runner.json` → `maxParallelThreads`, per suite. This section. | +| **Cross-process** — `dotnet test` on the solution | One test *process* per test project, several at a time. xunit cannot see across them. | For the MS SQL migration path only: the machine-wide `sp_getapplock` counting semaphore in `MsSqlMigrationFixture` (4 slots, `SCADABRIDGE_MSSQL_TEST_MIGRATE_SLOTS`) — see [test_infra_db.md](test_infra_db.md). Nothing bounds it in general. | + +### Which suites carry a runner config, and why + +The cap is **per suite, not global**: pure unit suites (Commons, ScriptAnalysis, TemplateEngine, +Transport, …) stay uncapped for speed. A suite gets a config only when a test *class* costs a real +external or OS-level resource, so that `ProcessorCount` of them at once is a resource question rather +than a CPU question. + +| Suite | Setting | Why | +|-------|---------|-----| +| `ConfigurationDatabase.Tests` | `maxParallelThreads: 4` | Owns `MsSqlMigrationFixture`: every MSSQL-backed test **class** does a `CREATE DATABASE` plus the full ~100-migration chain against the one shared dev SQL Server. 4 matches the `sp_getapplock` slot count so the in-process and cross-process limits agree. | +| `AuditLog.Tests` | `maxParallelThreads: 4` | Same fixture (by project reference), plus most classes stand up an Akka TestKit `ActorSystem` with fast-ticking actors. | +| `SiteCallAudit.Tests` | `maxParallelThreads: 4` | Same fixture (by project reference) plus a TestKit `ActorSystem`. | +| `Host.Tests` | `maxParallelThreads: 4` | Classes boot real ASP.NET Core hosts and real Akka `ActorSystem`s — Akka.Remote binds real sockets, Kestrel binds real ports, gRPC channels dial them. | +| `IntegrationTests` | `parallelizeAssembly: false`, `parallelizeTestCollections: false` | Pre-existing. Fully serialized; a thread cap would be redundant. | +| `CentralUI.PlaywrightTests` | `maxParallelThreads: 1` | Pre-existing, and the strictest setting in the repo — the suite drives a browser against a **live rig**, so shared UI state, not CPU, is the constraint. | +| `StoreAndForward.Tests` | `parallelizeTestCollections: true` | Pre-existing; that is the default, so this file is a no-op pin. Uncapped by choice — SQLite files are per-test. | + +### Rules + +- **Cap threads, do not serialize.** `maxParallelThreads` throttles how many collections run at once; + `parallelizeTestCollections: false` runs them one at a time forever. Reach for the second only when a + suite has a genuinely global resource it cannot partition (`IntegrationTests` and the Playwright suite + are the only two, and both predate this policy). +- **A cap is not a substitute for test isolation.** Rows 82/91/95 were all *correctness* bugs — leaked + ticking actors, absolute assertions on process-global counters — that a cap only makes rarer. Fix the + test; the cap is for resource pressure. +- **The json must be copied to the output directory.** The .NET SDK does not do this automatically: + each capped `.csproj` carries + ``. Without it the file + is silently ignored and the suite runs uncapped. +- **Default for a new test project: no runner config.** Add one only when the project acquires a + per-class external resource (an MSSQL fixture, a real host/port, a browser). Record the reason in the + `.csproj` comment next to the `Content` item. + +### Measured cost + +The cap is close to free, because these suites are latency-bound on an external resource rather than +CPU-bound (CPU utilisation stayed at 14–25 % for the two slowest). Measured standalone on a 14-core dev +box, uncapped vs. `maxParallelThreads: 4`: + +| Suite | Tests | Uncapped | Capped at 4 | +|-------|-------|----------|-------------| +| `AuditLog.Tests` | 383 | 1 m 52 s | 1 m 51 s | +| `Host.Tests` | 520 | 4 m 5 s | 4 m 8 s | +| `ConfigurationDatabase.Tests` | 396 | 10 s | 12 s | +| `SiteCallAudit.Tests` | 66 | 8 s | 8 s | + ## Remote Test Infrastructure In addition to the local Docker services, the following remote services are available for testing against real AVEVA System Platform hardware. diff --git a/docs/test_infra/test_infra_db.md b/docs/test_infra/test_infra_db.md index 1cda1c23..249ed0ee 100644 --- a/docs/test_infra/test_infra_db.md +++ b/docs/test_infra/test_infra_db.md @@ -43,7 +43,10 @@ Three behaviours are worth knowing when you look at this server during a test ru solution-level `dotnet test` runs several MSSQL-backed test *processes* at once, so the throttle has to live on the shared server. Without it, dozens of simultaneous fresh-database migrations saturated the container and individual DDL statements blew past EF's default 30-second command timeout, - failing whole test classes at fixture construction (deferred-work register row 82). + failing whole test classes at fixture construction (deferred-work register row 82). The + *in-process* half of the same problem is bounded separately, by a `maxParallelThreads: 4` cap in + each MSSQL-backed suite's `xunit.runner.json` — deliberately the same number, so the two limits + agree. See [test_infra.md → Test Parallelism Policy](test_infra.md#test-parallelism-policy). - **Migration commands use a 300s timeout**, and CREATE/DROP DATABASE use 120s. Under a bounded queue the server is busy but progressing, so the correct response to a slow statement is to wait. - **Strays are swept.** Each test process drops, once at start and best-effort, any diff --git a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/AuditLogPurgeActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/AuditLogPurgeActorTests.cs index 6ac9dabc..457cba2f 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/AuditLogPurgeActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/AuditLogPurgeActorTests.cs @@ -138,6 +138,38 @@ public class AuditLogPurgeActorTests : TestKit, IClassFixture.Instance))); } + /// + /// Stops a purge actor and blocks until it is actually dead. + /// + /// + /// + /// Deferred-work register row 95, the same leak shape row 82 fixed in + /// PartitionPurgeTests / NotifyDispatcherAuditTrailTests. Every actor this + /// class creates ticks every 100 ms + /// () and was only ever stopped + /// implicitly by 's Dispose → Shutdown — which is + /// bounded by a timeout and does NOT fail when that timeout expires. So on a loaded + /// machine an actor kept ticking while LATER test classes ran: the stub-repo tests burn a + /// DI scope and a thread every 100 ms for the rest of the assembly run, and + /// EndToEnd_RealPartition_…'s actor keeps switching partitions out of the shared + /// database that other classes in this assembly seed + /// against. + /// + /// + /// Stopping the actor inside the test that created it removes the dependency on + /// ActorSystem-shutdown timing entirely. + /// + /// + private void StopPurgeActor(IActorRef actor) + { + Watch(actor); + Sys.Stop(actor); + // A stop is processed after the in-flight tick completes, so this also guarantees no + // partition switch is still running against the fixture DB. + ExpectTerminated(actor, TimeSpan.FromSeconds(30)); + Unwatch(actor); + } + private static AuditLogPurgeOptions FastTickOptions(TimeSpan? interval = null) => new() { IntervalHours = 24, @@ -163,7 +195,7 @@ public class AuditLogPurgeActorTests : TestKit, IClassFixture= 1 enumerator call, got {repo.ThresholdQueries.Count}"), duration: TimeSpan.FromSeconds(3), interval: TimeSpan.FromMilliseconds(50)); + + StopPurgeActor(actor); } // --------------------------------------------------------------------- @@ -192,7 +226,7 @@ public class AuditLogPurgeActorTests : TestKit, IClassFixture 42L, }; - CreateActor(repo, FastTickOptions()); + var actor = CreateActor(repo, FastTickOptions()); AwaitAssert( () => @@ -202,6 +236,8 @@ public class AuditLogPurgeActorTests : TestKit, IClassFixture() }; - CreateActor(repo, FastTickOptions()); + var actor = CreateActor(repo, FastTickOptions()); // Wait for at least one tick (visible via the enumerator call) then // assert no switch happened. @@ -226,6 +262,10 @@ public class AuditLogPurgeActorTests : TestKit, IClassFixture(TimeSpan.FromSeconds(5)); + StopPurgeActor(actor); Assert.Equal(boundary, msg.MonthBoundary); Assert.Equal(1234L, msg.RowsDeleted); Assert.True(msg.DurationMs >= 0, @@ -269,7 +310,7 @@ public class AuditLogPurgeActorTests : TestKit, IClassFixture @@ -281,6 +322,8 @@ public class AuditLogPurgeActorTests : TestKit, IClassFixture new AuditLogPurgeActor( + var actor = Sys.ActorOf(Props.Create(() => new AuditLogPurgeActor( sp, Options.Create(purgeOptions), Options.Create(auditOptions), @@ -363,6 +406,11 @@ public class AuditLogPurgeActorTests : TestKit, IClassFixture m.MonthBoundary == janBoundary, max: TimeSpan.FromSeconds(30)); + // This is the one actor in the class that mutates the SHARED fixture database, so it + // must not outlive the test and start switching partitions out from under the next + // class's seed rows (register row 95). + StopPurgeActor(actor); + Assert.True(matched.RowsDeleted >= 1, $"Expected RowsDeleted >= 1 for the Jan-2026 partition; got {matched.RowsDeleted}."); @@ -394,7 +442,7 @@ public class AuditLogPurgeActorTests : TestKit, IClassFixture new AuditLogPurgeActor( + var actor = Sys.ActorOf(Props.Create(() => new AuditLogPurgeActor( sp, purgeOptionsWrapped, auditOptions, @@ -448,6 +498,8 @@ public class AuditLogPurgeActorTests : TestKit, IClassFixture p.Channel == "ApiOutbound"); Assert.Equal(1234, purge.BatchSize); @@ -483,7 +535,7 @@ public class AuditLogPurgeActorTests : TestKit, IClassFixture new AuditLogPurgeActor( + var actor = Sys.ActorOf(Props.Create(() => new AuditLogPurgeActor( sp, purgeOptions, auditOptions, @@ -496,6 +548,10 @@ public class AuditLogPurgeActorTests : TestKit, IClassFixture new AuditLogPurgeActor( + var actor = Sys.ActorOf(Props.Create(() => new AuditLogPurgeActor( sp, Options.Create(purgeOptions), auditOptions, @@ -542,6 +598,8 @@ public class AuditLogPurgeActorTests : TestKit, IClassFixture new AuditLogPurgeActor( + var actor = Sys.ActorOf(Props.Create(() => new AuditLogPurgeActor( sp, Options.Create(FastTickOptions()), Options.Create(new AuditLogOptions()), @@ -588,5 +646,7 @@ public class AuditLogPurgeActorTests : TestKit, IClassFixture= 1, got {snapshot.PurgeFailures}"), duration: TimeSpan.FromSeconds(3), interval: TimeSpan.FromMilliseconds(50)); + + StopPurgeActor(actor); } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/SiteAuditReconciliationActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/SiteAuditReconciliationActorTests.cs index 411cc2c4..3cd50a0c 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/SiteAuditReconciliationActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Central/SiteAuditReconciliationActorTests.cs @@ -252,6 +252,37 @@ public class SiteAuditReconciliationActorTests : TestKit, IClassFixture.Instance))); } + /// + /// Stops a reconciliation actor and blocks until it is actually dead. + /// + /// + /// + /// Deferred-work register row 95, the same leak shape row 82 fixed in + /// PartitionPurgeTests / NotifyDispatcherAuditTrailTests. Every actor this + /// class creates ticks on a ~0 s repeat interval () and was + /// only ever stopped implicitly by 's Dispose → + /// Shutdown, which is bounded by a timeout and does NOT fail when it expires. A + /// survivor keeps pulling and INSERTING audit rows: in + /// Tick_Duplicates_NotDoubleInserted it holds a real + /// over the shared + /// database (and over a DbContext the test is about to dispose), so it writes into + /// the DB LATER test classes read from. + /// + /// + /// Stopping the actor inside the test that created it removes the dependency on + /// ActorSystem-shutdown timing entirely. + /// + /// + private void StopReconciliationActor(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); + } + /// /// Subscribes to the EventStream and collects every /// publication into a list @@ -278,7 +309,7 @@ public class SiteAuditReconciliationActorTests : TestKit, IClassFixture Assert.True(client.Calls.Count >= 1, $"expected >= 1 pull call, got {client.Calls.Count}"), duration: TimeSpan.FromSeconds(3), interval: TimeSpan.FromMilliseconds(50)); + + StopReconciliationActor(actor); } // --------------------------------------------------------------------- @@ -304,7 +337,7 @@ public class SiteAuditReconciliationActorTests : TestKit, IClassFixture { @@ -313,6 +346,8 @@ public class SiteAuditReconciliationActorTests : TestKit, IClassFixture Assert.Equal(2, repo.InsertCallCount), duration: TimeSpan.FromSeconds(3), interval: TimeSpan.FromMilliseconds(50)); + + StopReconciliationActor(actor); + Assert.Contains(repo.Inserted, e => e.EventId == e1.EventId); Assert.Contains(repo.Inserted, e => e.EventId == e2.EventId); } @@ -371,13 +409,18 @@ public class SiteAuditReconciliationActorTests : TestKit, IClassFixture Assert.True(client.Calls.Count >= 1), duration: TimeSpan.FromSeconds(3)); + // Stop before reading: this actor holds a real repository over the SHARED fixture + // database (and over the DbContext this test is about to dispose), so it must not + // outlive the test (register row 95). + StopReconciliationActor(actor); + // Even though the pull returned 2 events, only 1 fresh row should // exist in MSSQL alongside the pre-existing one — InsertIfNotExistsAsync // is first-write-wins on EventId. @@ -413,7 +456,7 @@ public class SiteAuditReconciliationActorTests : TestKit, IClassFixture Assert.True(client.Calls.Count >= 2, $"need at least 2 pulls, got {client.Calls.Count}"), duration: TimeSpan.FromSeconds(5), interval: TimeSpan.FromMilliseconds(50)); + StopReconciliationActor(actor); + Assert.Equal(t, client.Calls[1].SinceUtc); Assert.Equal(high.EventId.ToString(), client.Calls[1].AfterId); } @@ -475,13 +524,15 @@ public class SiteAuditReconciliationActorTests : TestKit, IClassFixture Assert.True(client.Calls.Count >= 2, $"need at least 2 pulls, got {client.Calls.Count}"), duration: TimeSpan.FromSeconds(5), interval: TimeSpan.FromMilliseconds(50)); + StopReconciliationActor(actor); + Assert.Equal(DateTime.MinValue, client.Calls[1].SinceUtc); Assert.Null(client.Calls[1].AfterId); } @@ -543,7 +594,7 @@ public class SiteAuditReconciliationActorTests : TestKit, IClassFixture { @@ -552,6 +603,8 @@ public class SiteAuditReconciliationActorTests : TestKit, IClassFixture(TimeSpan.FromSeconds(5)); + StopReconciliationActor(actor); Assert.Equal("siteA", msg.SiteId); Assert.True(msg.Stalled); } @@ -606,7 +660,7 @@ public class SiteAuditReconciliationActorTests : TestKit, IClassFixture(TimeSpan.FromSeconds(5)); + StopReconciliationActor(actor); Assert.False(second.Stalled); Assert.Equal("siteA", second.SiteId); } @@ -643,7 +698,7 @@ public class SiteAuditReconciliationActorTests : TestKit, IClassFixture Assert.Contains(client.Calls, c => c.SinceUtc == poisonTime), duration: TimeSpan.FromSeconds(5), interval: TimeSpan.FromMilliseconds(100)); + + StopReconciliationActor(actor); } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/OutageReconciliationTests.cs b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/OutageReconciliationTests.cs index 2104c9a1..c02d5840 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/OutageReconciliationTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/Integration/OutageReconciliationTests.cs @@ -165,6 +165,37 @@ public class OutageReconciliationTests : TestKit, IClassFixture + /// Stops an actor and blocks until it is actually dead. + /// + /// + /// + /// Deferred-work register row 95, the same leak shape row 82 fixed in + /// PartitionPurgeTests / NotifyDispatcherAuditTrailTests. Both tests here + /// already called Sys.Stop, 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 database — and into the + /// the enclosing await using 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 's Dispose → Shutdown, which is + /// timeout-bounded and does not fail when it expires. + /// + /// + /// 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. + /// + /// + 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 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 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 r.EventId).Distinct().Count()); - - Sys.Stop(reconciliationActor); } } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests.csproj b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests.csproj index 337d93b2..492b5526 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests.csproj +++ b/tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests.csproj @@ -46,6 +46,22 @@ + + + + + + + + diff --git a/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/xunit.runner.json b/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/xunit.runner.json new file mode 100644 index 00000000..6366089b --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/xunit.runner.json @@ -0,0 +1,4 @@ +{ + "$schema": "https://xunit.net/schema/current/xunit.runner.schema.json", + "maxParallelThreads": 4 +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ZB.MOM.WW.ScadaBridge.Host.Tests.csproj b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ZB.MOM.WW.ScadaBridge.Host.Tests.csproj index d6fb9a82..d757589c 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ZB.MOM.WW.ScadaBridge.Host.Tests.csproj +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ZB.MOM.WW.ScadaBridge.Host.Tests.csproj @@ -28,6 +28,19 @@ + + + + + diff --git a/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/xunit.runner.json b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/xunit.runner.json new file mode 100644 index 00000000..6366089b --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Host.Tests/xunit.runner.json @@ -0,0 +1,4 @@ +{ + "$schema": "https://xunit.net/schema/current/xunit.runner.schema.json", + "maxParallelThreads": 4 +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests.csproj b/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests.csproj index 8e017f90..293f95b5 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests.csproj +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests.csproj @@ -36,6 +36,21 @@ + + + + +