From 11a716a07fc8a2adf3ae328559b4262d97f450db Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 9 Jul 2026 08:14:55 -0400 Subject: [PATCH] test+fix: eliminate Windows full-suite temp-file-lock flakiness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Full-host tests passed in isolation but failed 2-4/run under parallel execution on Windows with 'the process cannot access the file ... because it is being used by another process' (macOS never sees it — Unix deletes open files). Two shared-state parallel collisions, discovered while verifying TST-08: 1. Self-signed cert generation. GatewayTlsBootstrapTests sets process-global Kestrel/TLS env vars that GatewayApplication.Build reads; a parallel host-building test inherits them mid-run and both generate a cert at the same path, racing on the fixed-name '.tmp'. Fixes: - SelfSignedCertificateProvider: stage the PFX in a unique '..tmp' instead of a fixed name, so concurrent/interrupted writers never collide on the temp file (real robustness: two instances or a restart-during-write no longer clash). The final atomic Move (last-writer-wins) still yields an equivalent cert. - TestHostEnvironmentInitializer: default MxGateway__Tls__SelfSignedCertPath to a per-process temp path so the suite never writes the shared ProgramData default or fights the deployed service; first host-building test generates, the rest load it. - GatewayTlsBootstrapTests: [Collection] with DisableParallelization so its global env-var mutation cannot bleed into parallel tests (new GlobalEnvironmentCollection). 2. AuthStoreHealthCheckTests opened a real SQLite file; Microsoft.Data.Sqlite's connection pool keeps the .db handle open after 'await using', so the finally File.Delete threw. Reuse the existing TempDatabaseDirectory helper, which calls SqliteConnection.ClearAllPools() before deleting. Server build clean; affected classes 17/17 on macOS. Windows full-suite verified separately. --- .../Tls/SelfSignedCertificateProvider.cs | 10 +++++++- .../Diagnostics/AuthStoreHealthCheckTests.cs | 17 ++++++------- .../Gateway/GatewayTlsBootstrapTests.cs | 4 ++++ .../GlobalEnvironmentCollection.cs | 18 ++++++++++++++ .../TestHostEnvironmentInitializer.cs | 24 ++++++++++++++++++- 5 files changed, 63 insertions(+), 10 deletions(-) create mode 100644 src/ZB.MOM.WW.MxGateway.Tests/TestSupport/GlobalEnvironmentCollection.cs diff --git a/src/ZB.MOM.WW.MxGateway.Server/Security/Tls/SelfSignedCertificateProvider.cs b/src/ZB.MOM.WW.MxGateway.Server/Security/Tls/SelfSignedCertificateProvider.cs index 950fdfe..0d41576 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Security/Tls/SelfSignedCertificateProvider.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Security/Tls/SelfSignedCertificateProvider.cs @@ -163,7 +163,15 @@ public sealed class SelfSignedCertificateProvider // temp file empty, harden its permissions, and only then write the PFX into // the already-protected file. The temp path is in the same directory as the // target so the Move is atomic and preserves the hardened DACL/mode. - string temp = path + ".tmp"; + // + // The temp name carries a unique suffix rather than a fixed ".tmp": two + // processes (or two parallel callers) generating to the same target must not + // collide on one temp file. On Windows a fixed name makes the second writer's + // File.Create/Move fail with "the process cannot access the file ... because it + // is being used by another process"; a unique name lets each generation stage + // independently, and the final atomic Move (last-writer-wins) still yields a + // valid, equivalent certificate at the shared path. + string temp = $"{path}.{Guid.NewGuid():N}.tmp"; using (File.Create(temp)) { } HardenPermissions(temp); diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Diagnostics/AuthStoreHealthCheckTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Diagnostics/AuthStoreHealthCheckTests.cs index f2ed7ff..138a800 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Diagnostics/AuthStoreHealthCheckTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Diagnostics/AuthStoreHealthCheckTests.cs @@ -1,6 +1,7 @@ using Microsoft.Extensions.Diagnostics.HealthChecks; using ZB.MOM.WW.Auth.ApiKeys.Sqlite; using ZB.MOM.WW.MxGateway.Server.Diagnostics; +using ZB.MOM.WW.MxGateway.Tests.Security.Authentication; namespace ZB.MOM.WW.MxGateway.Tests.Diagnostics; @@ -17,14 +18,14 @@ public sealed class AuthStoreHealthCheckTests [Fact] public async Task Healthy_WhenStoreReachable() { - var path = Path.Combine(Path.GetTempPath(), $"authcheck-{Guid.NewGuid():N}.db"); - try - { - var check = new AuthStoreHealthCheck(FactoryFor(path)); - var result = await check.CheckHealthAsync(new HealthCheckContext()); - Assert.Equal(HealthStatus.Healthy, result.Status); - } - finally { if (File.Exists(path)) File.Delete(path); } + // Open a real SQLite file via the health check. TempDatabaseDirectory clears the + // Microsoft.Data.Sqlite connection pool on dispose before deleting the file; without + // that the pool keeps the .db handle open and the delete throws "used by another + // process" on Windows (a latent full-suite flake — the file opens fine on macOS). + using TempDatabaseDirectory directory = TempDatabaseDirectory.Create("authcheck"); + var check = new AuthStoreHealthCheck(FactoryFor(directory.DatabasePath())); + var result = await check.CheckHealthAsync(new HealthCheckContext()); + Assert.Equal(HealthStatus.Healthy, result.Status); } /// The health check reports unhealthy when the database path cannot be opened. diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/GatewayTlsBootstrapTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/GatewayTlsBootstrapTests.cs index 38cc3c9..810b9cd 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/GatewayTlsBootstrapTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/GatewayTlsBootstrapTests.cs @@ -9,6 +9,10 @@ using ZB.MOM.WW.MxGateway.Server; namespace ZB.MOM.WW.MxGateway.Tests.Gateway; +// Sets process-global Kestrel/TLS environment variables that GatewayApplication.Build reads; +// serialized against all other collections so a parallel host-building test cannot inherit them +// mid-run and race on the generated-certificate file. See GlobalEnvironmentCollection. +[Collection(TestSupport.GlobalEnvironmentCollection.Name)] public sealed class GatewayTlsBootstrapTests { /// diff --git a/src/ZB.MOM.WW.MxGateway.Tests/TestSupport/GlobalEnvironmentCollection.cs b/src/ZB.MOM.WW.MxGateway.Tests/TestSupport/GlobalEnvironmentCollection.cs new file mode 100644 index 0000000..eca51ca --- /dev/null +++ b/src/ZB.MOM.WW.MxGateway.Tests/TestSupport/GlobalEnvironmentCollection.cs @@ -0,0 +1,18 @@ +namespace ZB.MOM.WW.MxGateway.Tests.TestSupport; + +/// +/// xUnit collection for tests that mutate process-global state (environment variables +/// read by GatewayApplication.Build, e.g. Kestrel__Endpoints__… and +/// MxGateway__Tls__SelfSignedCertPath). DisableParallelization keeps such a test from +/// running concurrently with any other collection: otherwise a parallel host-building test inherits +/// the mutated variables mid-run and the two race on the same generated-certificate file (on Windows, +/// "the process cannot access the file … because it is being used by another process"). Membership is +/// deliberately narrow — only add classes that set/clear real environment variables, not ones that +/// pass configuration through Build([...]) command-line args. +/// +[CollectionDefinition(Name, DisableParallelization = true)] +public sealed class GlobalEnvironmentCollection +{ + /// The collection name applied via [Collection(GlobalEnvironmentCollection.Name)]. + public const string Name = "GlobalEnvironmentMutation"; +} diff --git a/src/ZB.MOM.WW.MxGateway.Tests/TestSupport/TestHostEnvironmentInitializer.cs b/src/ZB.MOM.WW.MxGateway.Tests/TestSupport/TestHostEnvironmentInitializer.cs index 580de0e..5c00e3c 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/TestSupport/TestHostEnvironmentInitializer.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/TestSupport/TestHostEnvironmentInitializer.cs @@ -3,7 +3,8 @@ using System.Runtime.CompilerServices; namespace ZB.MOM.WW.MxGateway.Tests.TestSupport; /// -/// Defaults the host environment to Development for the whole test assembly. +/// Defaults the host environment to Development and isolates the test process's on-disk +/// gateway paths, for the whole test assembly. /// /// /// Many tests build the full gateway host through GatewayApplication.Build against the dev @@ -15,6 +16,15 @@ namespace ZB.MOM.WW.MxGateway.Tests.TestSupport; /// GatewayOptionsValidator with its isProduction constructor (no host environment) and /// are unaffected; a test that needs Production can still pass --environment=Production, which /// overrides this default. +/// +/// The self-signed-cert path is also defaulted to a per-process temp file. Otherwise every +/// full-host test that triggers HTTPS-cert generation writes the shared +/// CommonApplicationData/MxGateway/certs/gateway-selfsigned.pfx default — parallel xUnit +/// test classes then collide on that path's temp file (Windows: "the process cannot access the +/// file ... because it is being used by another process"), and on a shared CI/dev box the suite +/// also fights the deployed gateway service for the same file. Pointing at a per-process path +/// isolates the suite: the first host-building test generates the cert, the rest load it. +/// /// internal static class TestHostEnvironmentInitializer { @@ -25,5 +35,17 @@ internal static class TestHostEnvironmentInitializer { Environment.SetEnvironmentVariable("ASPNETCORE_ENVIRONMENT", "Development"); } + + if (string.IsNullOrEmpty(Environment.GetEnvironmentVariable("MxGateway__Tls__SelfSignedCertPath"))) + { + // ProcessId keeps the path stable within this test process (so a valid cert + // generated by the first host-building test is reused by the rest) yet unique + // across processes (so concurrent test runs / the deployed service never share it). + string certPath = Path.Combine( + Path.GetTempPath(), + $"mxgw-tests-{Environment.ProcessId}", + "gateway-selfsigned.pfx"); + Environment.SetEnvironmentVariable("MxGateway__Tls__SelfSignedCertPath", certPath); + } } }