From d6fe3ae62a200221dc7e68028aae18aaf17cd6e8 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 11 Aug 2026 08:52:53 -0400 Subject: [PATCH] test(secrets): the pre-host regression test was asserting the weaker claim, and vacuously Two defects in the 0.6.1 regression test, the first raised by the mxaccessgw session from its own adoption, the second exposed by acting on it. Assertion order. The test asserted the throw before the artifact. That hides the stronger finding: a regression that stops the throw but still creates the store fails with "no exception was thrown" and never mentions the database sitting at the rejected path -- which IS the defect. Now captures with Record.ExceptionAsync, asserts no file created first, and asserts the exception type last. Notable because the defect under test was itself "a boot outcome was observed instead of the artifact being probed", so closing it with an outcome-first assertion would have repeated the mistake. Vacuous assertions. Simulating the regression -- replacing the 4-argument call with the 3-argument one, which still compiles, exactly as the call-site comments warn -- showed the test failing on the exception assertion rather than the file assertion. No file had been created in EITHER case: the test resolved SecretsSqliteConnectionFactory, whose constructor never touches the filesystem. The File.Exists assertions could not fail. The test now resolves SqliteSecretsStoreMigrator and runs MigrateAsync, which is what a pre-host expander actually does and what actually creates the database. Verified by simulating the regression again: the test now fails with "a rejected path must not be left holding a store file" rather than "no exception was thrown". Reverted after measuring. No library change; 0.6.1 as published is unaffected. Suite 502 passing. --- .../SecretsStorePathRulesTests.cs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Configuration/SecretsStorePathRulesTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Configuration/SecretsStorePathRulesTests.cs index f17dd3e..f203ad4 100644 --- a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Configuration/SecretsStorePathRulesTests.cs +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Configuration/SecretsStorePathRulesTests.cs @@ -238,7 +238,7 @@ public sealed class SecretsStorePathRulesTests } [Fact] - public void A_pre_host_container_given_the_content_root_refuses_before_the_store_is_created() + public async Task A_pre_host_container_given_the_content_root_refuses_before_the_store_is_created() { // REGRESSION (found by the HistorianGateway adoption of 0.6.0). An application that expands // ${secret:} references before building its host composes this subsystem in a throwaway @@ -265,13 +265,24 @@ public sealed class SecretsStorePathRulesTests services.AddZbSecrets(config, "Secrets", contentRoot); // the content root, explicitly using ServiceProvider provider = services.BuildServiceProvider(); - Assert.Throws( - () => _ = provider.GetRequiredService()); + // Mirror what a pre-host expander actually does: resolve the MIGRATOR and run it. + // Resolving the connection factory alone proves nothing here — its constructor never + // touches the filesystem, so a "no file created" assertion around it passes whether + // the guard works or not. The migration is what creates the database. + Exception? thrown = await Record.ExceptionAsync( + () => provider.GetRequiredService().MigrateAsync(default)); - // The point of the test: refused BEFORE anything touched the filesystem. + // ORDER MATTERS, and the obvious order is wrong. Asserting the throw FIRST would mask + // the stronger finding: a regression that stops the throw but still creates the store + // fails with "no exception was thrown" and never mentions the database sitting at the + // rejected path — which IS the defect. "It threw" is the weaker claim, so it is + // asserted last. (The defect this test guards was itself "a boot outcome was observed + // instead of the artifact being probed", so closing it with an outcome-first assertion + // would have repeated the mistake.) Assert.False(File.Exists(dbPath), "a rejected path must not be left holding a store file"); Assert.False(File.Exists(dbPath + "-wal")); Assert.False(File.Exists(dbPath + "-shm")); + Assert.IsType(thrown); } finally {