test(secrets): the positive control was a tautology propping up the negative one

Raised by the mxaccessgw session, which went back to its own tests after I reported a
vacuous assertion in mine and found the same class one step further from the fuse.

The rejecting test asserts no store file is left at a rejected path. That claim is only
meaningful if something proves the write path is live at all: "no file was created" is
indistinguishable from "nothing in this container ever creates files". The positive
control was supposed to establish that and did not -- it asserted Assert.NotNull on a
resolved SecretsSqliteConnectionFactory, which is close to a tautology once no exception
escaped, and it never ran the migration at all. So if the pre-host container ever stopped
touching the store, the rejecting test would have gone quietly vacuous while the positive
control kept passing. Neither would have failed.

The positive control now runs SqliteSecretsStoreMigrator.MigrateAsync and asserts the
database IS created. The pair is mutually checking: the migration writes the file when
allowed and does not when rejected, and neither assertion stands on its own.

Verified the same way as the last one, by simulating the failure rather than reasoning
about it: replacing the migration with a no-op makes the positive control fail with "the
accepted path must actually get a store file". Reverted after measuring.

No library change; 0.6.1 as published is unaffected. Suite 502 passing.
This commit is contained in:
Joseph Doherty
2026-08-11 08:56:46 -04:00
parent d6fe3ae62a
commit 6084465e23
@@ -291,7 +291,7 @@ public sealed class SecretsStorePathRulesTests
} }
[Fact] [Fact]
public void Without_a_content_root_the_pre_host_container_still_cannot_be_told_it_is_fine() public async Task Without_a_content_root_the_pre_host_container_still_cannot_be_told_it_is_fine()
{ {
// Positive control for the test above: the SAME path, with the content root NOT supplied, // Positive control for the test above: the SAME path, with the content root NOT supplied,
// is the pre-0.6.1 behaviour — the rule is skipped and nothing objects. This is what makes // is the pre-0.6.1 behaviour — the rule is skipped and nothing objects. This is what makes
@@ -315,7 +315,15 @@ public sealed class SecretsStorePathRulesTests
// Absolute, so the rooted rule passes; the content-root rule cannot fire. Documented, // Absolute, so the rooted rule passes; the content-root rule cannot fire. Documented,
// and the reason pre-host composition must pass the content root explicitly. // and the reason pre-host composition must pass the content root explicitly.
Assert.NotNull(provider.GetRequiredService<SecretsSqliteConnectionFactory>()); await provider.GetRequiredService<SqliteSecretsStoreMigrator>().MigrateAsync(default);
// Assert the database IS created here, not merely that nothing threw. This is what
// keeps the REJECTING test honest: "no file was created" is indistinguishable from
// "nothing in this container ever creates files" unless something proves the write
// path is live. An Assert.NotNull on a resolved factory proves neither — and if the
// pre-host container ever stopped touching the store, it would silently void the
// rejecting test while continuing to pass itself.
Assert.True(File.Exists(dbPath), "the accepted path must actually get a store file");
} }
finally finally
{ {