dohertj2/ScadaBridge
1
0
Fork 0
Code Issues 2 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

test-flake: CentralDbTestEnvironment mutates process-wide env vars without synchronization (Host.Tests parallelism) #15

New Issue
Closed
opened 2026-07-16 16:19:58 -04:00 by dohertj2 · 0 comments
dohertj2 commented 2026-07-16 16:19:58 -04:00
Owner
Copy Link
Copy Source

Summary

CentralDbTestEnvironment (in tests/ZB.MOM.WW.ScadaBridge.Host.Tests) sets and later restores/clears process-wide environment variables — ScadaBridge__Database__ConfigurationDb, ScadaBridge__Database__MachineDataDb, ScadaBridge__Security__Ldap__ServiceAccountPassword, ScadaBridge__Security__JwtSigningKey, ScadaBridge__InboundApi__ApiKeyPepper — with no synchronization. xUnit's default collection parallelization is not disabled for the ZB.MOM.WW.ScadaBridge.Host.Tests assembly, so several WebApplicationFactory<Program> Central-boot fixtures can run concurrently. If one fixture's teardown (Dispose/DisposeAsync, which restores/clears those vars) races a concurrently-running fixture's Program boot — which reads env via AddEnvironmentVariables() at an unpredictable moment — the reading fixture can observe a transient null/cleared value.

Why this matters now

Since the secrets adoption (branch feat/adopt-zb-secrets, merged 128f1596), three of those config values are ${secret:...} tokens resolved by the pre-host expander, which fails closed on a missing/empty value. So a race that previously produced a benign empty connection string now surfaces as a hard SecretNotFoundException that aborts the fixture's host boot — i.e. a flaky CI failure rather than a silent soft value. That commit also added two more shared globals (LDAP service-account password, JWT signing key) to the same unsynchronized pattern and made them required for every Central boot to succeed, amplifying a pre-existing exposure (ConfigurationDb / pepper were already managed this way).

This is a test-harness flake, not a product defect. The offline suite currently passes (275 → 279 after the adoption); the race is intermittent and only manifests under parallel load.

Repro (intermittent)

Run the full Host.Tests suite repeatedly / under load:

dotnet test tests/ZB.MOM.WW.ScadaBridge.Host.Tests

Occasionally one Central-boot fixture fails with SecretNotFoundException for one of the ${secret:...} config keys even though it is supplied via a whole-key env override — because a sibling fixture cleared that env var mid-boot.

Affected code

  • tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CentralDbTestEnvironment.cs — set/restore of the 5 env keys, no lock.
  • The six fixtures that consume it: HostStartupTests, HealthCheckTests, MetricsEndpointTests, ActorPathTests, CompositionRootTests, AkkaHostedServiceAuditWiringTests.
  • tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ZB.MOM.WW.ScadaBridge.Host.Tests.csproj — no xunit.runner.json / no [assembly: CollectionBehavior(DisableTestCollectionParallelization = true)].

Fix options

  1. (recommended) Put all six Central-boot fixtures in a single xUnit [Collection] so they never run concurrently with each other. Lowest blast radius — no other collection touches these env vars.
  2. Disable collection parallelization for the Host.Tests assembly (xunit.runner.json with "parallelizeTestCollections": false, or [assembly: CollectionBehavior(DisableTestCollectionParallelization = true)]). Simplest, but slows the assembly.
  3. Wrap CentralDbTestEnvironment set/restore in a static lock — necessary but not sufficient alone (the reader, Program's AddEnvironmentVariables(), is not under the lock), so pair it with (1) or (2).

Origin

Flagged in code review during the ScadaBridge secrets adoption (component: Secrets, gap G-4, task T4). Non-blocking; filed as a follow-up.

## Summary `CentralDbTestEnvironment` (in `tests/ZB.MOM.WW.ScadaBridge.Host.Tests`) sets and later restores/clears **process-wide** environment variables — `ScadaBridge__Database__ConfigurationDb`, `ScadaBridge__Database__MachineDataDb`, `ScadaBridge__Security__Ldap__ServiceAccountPassword`, `ScadaBridge__Security__JwtSigningKey`, `ScadaBridge__InboundApi__ApiKeyPepper` — with no synchronization. xUnit's default collection parallelization is **not** disabled for the `ZB.MOM.WW.ScadaBridge.Host.Tests` assembly, so several `WebApplicationFactory<Program>` Central-boot fixtures can run concurrently. If one fixture's teardown (`Dispose`/`DisposeAsync`, which restores/clears those vars) races a concurrently-running fixture's `Program` boot — which reads env via `AddEnvironmentVariables()` at an unpredictable moment — the reading fixture can observe a transient null/cleared value. ## Why this matters now Since the secrets adoption (branch `feat/adopt-zb-secrets`, merged `128f1596`), three of those config values are `${secret:...}` tokens resolved by the pre-host expander, which **fails closed** on a missing/empty value. So a race that previously produced a benign empty connection string now surfaces as a hard `SecretNotFoundException` that aborts the fixture's host boot — i.e. a flaky CI failure rather than a silent soft value. That commit also added two more shared globals (LDAP service-account password, JWT signing key) to the same unsynchronized pattern and made them **required** for every Central boot to succeed, amplifying a pre-existing exposure (ConfigurationDb / pepper were already managed this way). This is a **test-harness** flake, not a product defect. The offline suite currently passes (275 → 279 after the adoption); the race is intermittent and only manifests under parallel load. ## Repro (intermittent) Run the full Host.Tests suite repeatedly / under load: ``` dotnet test tests/ZB.MOM.WW.ScadaBridge.Host.Tests ``` Occasionally one Central-boot fixture fails with `SecretNotFoundException` for one of the `${secret:...}` config keys even though it is supplied via a whole-key env override — because a sibling fixture cleared that env var mid-boot. ## Affected code - `tests/ZB.MOM.WW.ScadaBridge.Host.Tests/CentralDbTestEnvironment.cs` — set/restore of the 5 env keys, no lock. - The six fixtures that consume it: `HostStartupTests`, `HealthCheckTests`, `MetricsEndpointTests`, `ActorPathTests`, `CompositionRootTests`, `AkkaHostedServiceAuditWiringTests`. - `tests/ZB.MOM.WW.ScadaBridge.Host.Tests/ZB.MOM.WW.ScadaBridge.Host.Tests.csproj` — no `xunit.runner.json` / no `[assembly: CollectionBehavior(DisableTestCollectionParallelization = true)]`. ## Fix options 1. **(recommended)** Put all six Central-boot fixtures in a single xUnit `[Collection]` so they never run concurrently with each other. Lowest blast radius — no other collection touches these env vars. 2. Disable collection parallelization for the Host.Tests assembly (`xunit.runner.json` with `"parallelizeTestCollections": false`, or `[assembly: CollectionBehavior(DisableTestCollectionParallelization = true)]`). Simplest, but slows the assembly. 3. Wrap `CentralDbTestEnvironment` set/restore in a static lock — necessary but **not sufficient alone** (the reader, `Program`'s `AddEnvironmentVariables()`, is not under the lock), so pair it with (1) or (2). ## Origin Flagged in code review during the ScadaBridge secrets adoption (component: Secrets, gap G-4, task T4). Non-blocking; filed as a follow-up.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
dohertj2 (dohertj2)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/ScadaBridge#15
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?