From d2d4db492bce8f40dedf696e7755be4017587c9b Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 17 Aug 2026 05:27:52 -0400 Subject: [PATCH] =?UTF-8?q?test(infra):=20close=20register=20row=2096=20?= =?UTF-8?q?=E2=80=94=20per-suite=20xunit=20parallelism=20caps=20for=20the?= =?UTF-8?q?=20resource-heavy=20suites?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit maxParallelThreads was unset everywhere and only 3 of ~20 test projects carried an xunit.runner.json at all, so xunit ran Environment.ProcessorCount collections at once (14 on the dev box) in every suite. That is the general load multiplier behind the row-82/91/95 flake family. Policy: per-suite caps, NOT a global one. Pure unit suites stay uncapped for speed; a suite gets a config only when a test CLASS costs a real external or OS-level resource, so ProcessorCount of them at once is a resource question rather than a CPU question. Four suites qualify, all capped at maxParallelThreads: 4. - ConfigurationDatabase.Tests, AuditLog.Tests, SiteCallAudit.Tests — verified as the three that actually take MsSqlMigrationFixture (the latter two by project reference). Every MSSQL-backed test class means a CREATE DATABASE plus the ~100-migration chain against the one shared dev SQL Server. 4 is chosen to MATCH the default slot count of row 82's cross-process sp_getapplock migrate semaphore, so the in-process and cross-process limits agree rather than fighting. - Host.Tests — classes boot real ASP.NET Core hosts and real ActorSystems; Akka.Remote binds real sockets, Kestrel binds real ports, gRPC dials them. Threads are capped, collections are NOT serialized. IntegrationTests (parallelizeAssembly/parallelizeTestCollections false) and CentralUI.PlaywrightTests (maxParallelThreads 1) already carry stricter pre-existing settings and are left alone — the Playwright suite drives a browser against a live rig, where shared UI state, not CPU, is the constraint. Each capped csproj gains ; the SDK does not copy it automatically and without it the file is silently ignored. Verified by moving the copied file out of bin/ to take an uncapped baseline: ConfigurationDatabase.Tests ran at 216 % CPU uncapped vs 145 % capped, so the cap demonstrably takes effect. Cost is close to nil — these suites are latency-bound on an external resource, not CPU-bound (14-25 % utilisation for the two slowest). Standalone, uncapped -> capped: AuditLog 383 tests 1m52s -> 1m51s; Host 520 tests 4m05s -> 4m08s; ConfigurationDatabase 396 tests 10s -> 12s; SiteCallAudit 66 tests 8s -> 8s. All green. docs/test_infra/test_infra.md gains a Test Parallelism Policy section: the two axes (in-process collection threads = this cap; cross-process solution-level dotnet test = unbounded in general, bounded on the DB-migration path by the sp_getapplock semaphore), the per-suite table with reasons, the rules (cap threads rather than serialize; a cap is not a substitute for test isolation; the Content copy is required), and the default for new projects (no config until the project acquires a per-class external resource). test_infra_db.md cross-links it from the semaphore bullet. --- docs/test_infra/test_infra.md | 58 +++++++++++++++++++ docs/test_infra/test_infra_db.md | 5 +- ...B.MOM.WW.ScadaBridge.AuditLog.Tests.csproj | 16 +++++ .../xunit.runner.json | 4 ++ ...aBridge.ConfigurationDatabase.Tests.csproj | 17 ++++++ .../xunit.runner.json | 4 ++ .../ZB.MOM.WW.ScadaBridge.Host.Tests.csproj | 13 +++++ .../xunit.runner.json | 4 ++ ....WW.ScadaBridge.SiteCallAudit.Tests.csproj | 15 +++++ .../xunit.runner.json | 4 ++ 10 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 tests/ZB.MOM.WW.ScadaBridge.AuditLog.Tests/xunit.runner.json create mode 100644 tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/xunit.runner.json create mode 100644 tests/ZB.MOM.WW.ScadaBridge.Host.Tests/xunit.runner.json create mode 100644 tests/ZB.MOM.WW.ScadaBridge.SiteCallAudit.Tests/xunit.runner.json 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/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 @@ + + + + +