test(host): serialize Central-boot fixtures to close env-var race

CentralDbTestEnvironment sets five process-wide environment variables, and
Program's AddEnvironmentVariables() reads them at an unpredictable point during
host boot. With xUnit collection parallelization on, one fixture's teardown
could clear a var mid-boot for a sibling. Since the secrets adoption (G-4)
three of those keys are ${secret:...} references that fail closed, turning a
previously benign empty value into a SecretNotFoundException that aborts the
boot — an intermittent CI failure.

Adds a HostBootCollection that serializes every fixture booting a real host
while depending on that shared state, folding in the narrower "ActorSystem"
collection so its members stay serialized with each other as before. Site-role
fixtures stay parallel: they call Configuration.Sources.Clear(), dropping the
env-var provider, so they cannot participate in the race.

CentralDbTestEnvironment now also fails fast if two instances are ever live at
once, making a regression (a fixture added outside the collection) deterministic
rather than intermittent — this is what surfaced the disposed-CTS defect fixed
in the previous commit. Fixture teardown is try/finally so a throwing host
teardown can no longer strand the vars for the rest of the run.

Cost: Host.Tests runs ~2m30s -> ~4m10s; the serialized Central-boot classes are
most of the assembly's parallelism.

Fixes: Gitea #15
This commit is contained in:
Joseph Doherty
2026-07-16 23:31:28 -04:00
parent 9110a4eb01
commit d2a6107cdb
8 changed files with 95 additions and 14 deletions
@@ -10,14 +10,11 @@ using ZB.MOM.WW.ScadaBridge.Host.Actors;
namespace ZB.MOM.WW.ScadaBridge.Host.Tests; namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
[CollectionDefinition("ActorSystem")]
public class ActorSystemCollection : ICollectionFixture<object> { }
/// <summary> /// <summary>
/// Verifies that all expected Central-role actors are created at the correct paths /// Verifies that all expected Central-role actors are created at the correct paths
/// when AkkaHostedService starts. /// when AkkaHostedService starts.
/// </summary> /// </summary>
[Collection("ActorSystem")] [Collection(HostBootCollection.Name)]
public class CentralActorPathTests : IAsyncLifetime public class CentralActorPathTests : IAsyncLifetime
{ {
private WebApplicationFactory<Program>? _factory; private WebApplicationFactory<Program>? _factory;
@@ -92,10 +89,21 @@ public class CentralActorPathTests : IAsyncLifetime
} }
public async Task DisposeAsync() public async Task DisposeAsync()
{
// try/finally: a throwing host teardown must not strand the process-wide
// environment variables. Without it, one failed _factory.Dispose() leaks
// DOTNET_ENVIRONMENT=Central and the CentralDbTestEnvironment vars into
// every later test in the run.
try
{ {
_factory?.Dispose(); _factory?.Dispose();
}
finally
{
Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", _previousEnv); Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", _previousEnv);
_dbEnv?.Dispose(); _dbEnv?.Dispose();
}
await Task.CompletedTask; await Task.CompletedTask;
} }
@@ -149,7 +157,7 @@ public class CentralActorPathTests : IAsyncLifetime
/// Verifies that all expected Site-role actors are created at the correct paths /// Verifies that all expected Site-role actors are created at the correct paths
/// when AkkaHostedService starts. /// when AkkaHostedService starts.
/// </summary> /// </summary>
[Collection("ActorSystem")] [Collection(HostBootCollection.Name)]
public class SiteActorPathTests : IAsyncLifetime public class SiteActorPathTests : IAsyncLifetime
{ {
private IHost? _host; private IHost? _host;
@@ -86,6 +86,7 @@ public class AkkaHostedServiceAuditWiringHoconTests
/// <summary> /// <summary>
/// Verifies Audit Log (#23) services land in the Central composition root. /// Verifies Audit Log (#23) services land in the Central composition root.
/// </summary> /// </summary>
[Collection(HostBootCollection.Name)]
public class CentralAuditWiringTests : IDisposable public class CentralAuditWiringTests : IDisposable
{ {
private readonly WebApplicationFactory<Program> _factory; private readonly WebApplicationFactory<Program> _factory;
@@ -155,11 +156,19 @@ public class CentralAuditWiringTests : IDisposable
} }
public void Dispose() public void Dispose()
{
// try/finally: a throwing host teardown must not strand the process-wide
// environment variables for the rest of the run.
try
{ {
_factory.Dispose(); _factory.Dispose();
}
finally
{
Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", _previousEnv); Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", _previousEnv);
_dbEnv.Dispose(); _dbEnv.Dispose();
} }
}
[Fact] [Fact]
public void Central_Resolves_IAuditWriter_AsFallbackAuditWriter() public void Central_Resolves_IAuditWriter_AsFallbackAuditWriter()
@@ -26,6 +26,14 @@ namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
/// the tokens entirely — so tests that boot the real <c>Program</c> pipeline do /// the tokens entirely — so tests that boot the real <c>Program</c> pipeline do
/// not need a seeded secrets store. All vars are restored on Dispose so tests /// not need a seeded secrets store. All vars are restored on Dispose so tests
/// stay isolated. /// stay isolated.
///
/// Every var below is <b>process-wide</b>, and the reader — <c>Program</c>'s
/// <c>AddEnvironmentVariables()</c> — runs at an unpredictable point during host boot.
/// Two live instances therefore cannot be allowed to overlap: one's Dispose would
/// restore/clear a var while the other's boot is still reading it, and the fail-closed
/// secrets expander turns that into a <c>SecretNotFoundException</c>. Serialization via
/// <see cref="HostBootCollection"/> is what enforces the invariant; the overlap check in
/// the constructor makes a regression fail deterministically instead of intermittently.
/// </summary> /// </summary>
internal sealed class CentralDbTestEnvironment : IDisposable internal sealed class CentralDbTestEnvironment : IDisposable
{ {
@@ -64,8 +72,25 @@ internal sealed class CentralDbTestEnvironment : IDisposable
private readonly string? _previousLdapPassword; private readonly string? _previousLdapPassword;
private readonly string? _previousJwtSigningKey; private readonly string? _previousJwtSigningKey;
/// <summary>
/// Number of live instances. Only ever 0 or 1 while every consuming fixture is a
/// member of <see cref="HostBootCollection"/>; see the overlap check below.
/// </summary>
private static int _liveCount;
public CentralDbTestEnvironment() public CentralDbTestEnvironment()
{ {
if (Interlocked.Increment(ref _liveCount) != 1)
{
Interlocked.Decrement(ref _liveCount);
throw new InvalidOperationException(
$"Two {nameof(CentralDbTestEnvironment)} instances are live at once, so one fixture's " +
"teardown can clear the process-wide environment variables another fixture's host boot " +
$"is reading. Add the offending test class to the \"{HostBootCollection.Name}\" collection " +
$"([Collection({nameof(HostBootCollection)}.Name)]) so xUnit runs it sequentially with the " +
"other host-boot fixtures.");
}
_previousConfig = Environment.GetEnvironmentVariable(ConfigKey); _previousConfig = Environment.GetEnvironmentVariable(ConfigKey);
Environment.SetEnvironmentVariable(ConfigKey, ConfigurationDb); Environment.SetEnvironmentVariable(ConfigKey, ConfigurationDb);
@@ -89,5 +114,7 @@ internal sealed class CentralDbTestEnvironment : IDisposable
Environment.SetEnvironmentVariable(PepperKey, _previousPepper); Environment.SetEnvironmentVariable(PepperKey, _previousPepper);
Environment.SetEnvironmentVariable(LdapPasswordKey, _previousLdapPassword); Environment.SetEnvironmentVariable(LdapPasswordKey, _previousLdapPassword);
Environment.SetEnvironmentVariable(JwtSigningKeyKey, _previousJwtSigningKey); Environment.SetEnvironmentVariable(JwtSigningKeyKey, _previousJwtSigningKey);
Interlocked.Decrement(ref _liveCount);
} }
} }
@@ -80,6 +80,7 @@ internal static class AkkaHostedServiceRemover
/// Verifies every expected DI service resolves from the Central composition root. /// Verifies every expected DI service resolves from the Central composition root.
/// Uses WebApplicationFactory to exercise the real Program.cs pipeline. /// Uses WebApplicationFactory to exercise the real Program.cs pipeline.
/// </summary> /// </summary>
[Collection(HostBootCollection.Name)]
public class CentralCompositionRootTests : IDisposable public class CentralCompositionRootTests : IDisposable
{ {
private readonly WebApplicationFactory<Program> _factory; private readonly WebApplicationFactory<Program> _factory;
@@ -158,11 +159,19 @@ public class CentralCompositionRootTests : IDisposable
} }
public void Dispose() public void Dispose()
{
// try/finally: a throwing host teardown must not strand the process-wide
// environment variables for the rest of the run.
try
{ {
_factory.Dispose(); _factory.Dispose();
}
finally
{
Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", _previousEnv); Environment.SetEnvironmentVariable("DOTNET_ENVIRONMENT", _previousEnv);
_dbEnv.Dispose(); _dbEnv.Dispose();
} }
}
// --- Singletons --- // --- Singletons ---
@@ -17,6 +17,7 @@ namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
/// active-node) rather than by check-name predicates. These are pure route/tag assertions /// active-node) rather than by check-name predicates. These are pure route/tag assertions
/// — they require no database, LDAP, or formed Akka cluster. /// — they require no database, LDAP, or formed Akka cluster.
/// </summary> /// </summary>
[Collection(HostBootCollection.Name)]
public class HealthCheckTests : IDisposable public class HealthCheckTests : IDisposable
{ {
private readonly List<IDisposable> _disposables = new(); private readonly List<IDisposable> _disposables = new();
@@ -0,0 +1,25 @@
namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
/// <summary>
/// Serializes every fixture that boots a real host while depending on process-wide
/// state — the <c>DOTNET_ENVIRONMENT</c> role selector and the environment variables
/// <see cref="CentralDbTestEnvironment"/> sets.
///
/// <c>Program</c>'s configuration builder calls <c>AddEnvironmentVariables()</c> at an
/// unpredictable point during boot, so a sibling fixture's teardown (which restores or
/// clears those vars) can otherwise be observed mid-boot as a transient null. Since the
/// secrets adoption (G-4) three of those keys are <c>${secret:...}</c> references that
/// fail closed, which turns that race into a hard <c>SecretNotFoundException</c>.
/// xUnit runs classes in a shared collection sequentially, which removes the overlap.
///
/// Site-role fixtures are deliberately not members: they call
/// <c>Configuration.Sources.Clear()</c>, dropping the environment-variable provider, so
/// they neither read nor write the shared state and stay free to run in parallel.
///
/// Supersedes the narrower "ActorSystem" collection, whose members are folded in here.
/// </summary>
[CollectionDefinition(HostBootCollection.Name)]
public class HostBootCollection
{
public const string Name = "HostBoot";
}
@@ -9,6 +9,7 @@ using ZB.MOM.WW.ScadaBridge.Host;
namespace ZB.MOM.WW.ScadaBridge.Host.Tests; namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
[Collection(HostBootCollection.Name)]
public class HostStartupTests : IDisposable public class HostStartupTests : IDisposable
{ {
private readonly List<IDisposable> _disposables = new(); private readonly List<IDisposable> _disposables = new();
@@ -13,6 +13,7 @@ namespace ZB.MOM.WW.ScadaBridge.Host.Tests;
/// cluster state. This is a pure route assertion — it requires no database, LDAP, or formed /// cluster state. This is a pure route assertion — it requires no database, LDAP, or formed
/// Akka cluster. The Central-role factory bootstrap mirrors <see cref="HealthCheckTests"/>. /// Akka cluster. The Central-role factory bootstrap mirrors <see cref="HealthCheckTests"/>.
/// </summary> /// </summary>
[Collection(HostBootCollection.Name)]
public class MetricsEndpointTests : IDisposable public class MetricsEndpointTests : IDisposable
{ {
private readonly List<IDisposable> _disposables = new(); private readonly List<IDisposable> _disposables = new();