amadan
app: fix round 1(a) on Task 3 -- make the -seed gate test real
TestSeedRequiresDevelopmentEnv re-implemented main()'s "seedFlag && env != development" expression inline and compared it to itself: it called nothing from main.go, so inverting the real if, reordering its operands, or dropping a clause would still leave it green. That matters because it is the only thing keeping -seed away from a database that might be real, and Seed itself has no idempotency guard on the theory that safety lives entirely at this gate. Extracted the predicate into checkSeedGate(seed bool, env string) error; main() and the test now both call it, so the test fails if the real gate ever changes. Verified by hand: inverted the predicate's comparison, watched TestSeedGateRefusesWithoutDevelopment fail on four of five cases, then restored it and watched it pass again.
2 files changed,
+49
−32
cmd/docs/main.go+30 −13cmd/docs/main_test.go+19 −19
diff --git a/cmd/docs/main.go b/cmd/docs/main.go| index 803cfe0..59164b6 100644 |
| --- a/cmd/docs/main.go |
| +++ b/cmd/docs/main.go |
| @@ -3,6 +3,7 @@ package main |
| import ( |
| "context" |
| + "fmt" |
| "log/slog" |
| "os" |
| "strings" |
| @@ -109,6 +110,29 @@ func chooseEnv(flagEnv, osEnv string) string { |
| return "production" |
| } |
| +// checkSeedGate is -seed's whole safety story, extracted to a single |
| +// named predicate so main() and TestSeedGateRefusesWithoutDevelopment |
| +// (main_test.go) call the exact same code — not a copy of its logic |
| +// compared against itself. Fix round 1 on Task 3: the original test |
| +// re-implemented this expression inline, inverted operand order and |
| +// all, and so could not fail no matter how the real gate changed. |
| +// |
| +// chooseEnv above only ever returns "development" for the literal |
| +// -env flag on this exact invocation — never $DOCS_ENV, never any |
| +// default (chooseEnv's own doc comment, fix round 2's finding on Task |
| +// 1) — so this one check is genuinely the only thing standing between |
| +// -seed and a database that might hold real work: a systemd unit's |
| +// argv that carelessly kept a -seed flag around, or a copy-pasted |
| +// invocation run against a real deployment, must refuse here rather |
| +// than writing six sample documents into it. A nil error means |
| +// seeding may proceed. |
| +func checkSeedGate(seed bool, env string) error { |
| + if !seed || env == "development" { |
| + return nil |
| + } |
| + return fmt.Errorf("-seed requires -env development; refusing to seed a database that might not be a throwaway one (env=%q)", env) |
| +} |
| + |
| func main() { |
| logger := slog.Default() |
| @@ -203,19 +227,12 @@ func main() { |
| // in run-dev.sh's own invocation. |
| env := chooseEnv(envFlag, os.Getenv("DOCS_ENV")) |
| - // -seed's whole safety story lives in this one check. chooseEnv |
| - // above only ever returns "development" for the literal -env |
| - // flag — never $DOCS_ENV, never any default (its own doc comment, |
| - // fix round 2's finding) — so gating -seed on env != "development" |
| - // here means a systemd unit's argv that carelessly kept a -seed |
| - // flag around, or a copy-pasted invocation run against a real |
| - // deployment, refuses to start rather than writing six sample |
| - // documents into a database that might hold somebody's real work. |
| - // This check runs before docs.New even opens the database, so |
| - // there is no window where a wrongly-seeded production process is |
| - // already serving requests by the time this fires. |
| - if seedFlag && env != "development" { |
| - logger.Error("docs: -seed requires -env development; refusing to seed a database that might not be a throwaway one", "env", env) |
| + // checkSeedGate's own doc comment has the full reasoning; this |
| + // call runs before docs.New even opens the database, so there is |
| + // no window where a wrongly-seeded production process is already |
| + // serving requests by the time this fires. |
| + if err := checkSeedGate(seedFlag, env); err != nil { |
| + logger.Error("docs: " + err.Error()) |
| os.Exit(1) |
| } |
diff --git a/cmd/docs/main_test.go b/cmd/docs/main_test.go| index 98d20ce..fac51a8 100644 |
| --- a/cmd/docs/main_test.go |
| +++ b/cmd/docs/main_test.go |
| @@ -135,33 +135,33 @@ func TestExtractSeedFlag(t *testing.T) { |
| } |
| } |
| -// TestSeedRequiresDevelopmentEnv pins -seed's whole safety story at |
| -// the one place main() actually checks it: env != "development" |
| -// (whatever chooseEnv resolved, from whatever combination of -env and |
| -// $DOCS_ENV) must refuse rather than seed. This mirrors |
| -// TestArgvEdgeCasesFailTowardProduction's own shape — asserting the |
| -// fail-safe property directly rather than trusting that it "happens" |
| -// to hold. |
| -func TestSeedRequiresDevelopmentEnv(t *testing.T) { |
| +// TestSeedGateRefusesWithoutDevelopment calls checkSeedGate itself — |
| +// main.go's own predicate, not a copy of its expression — so this |
| +// test fails if the real gate ever changes. Fix round 1 on Task 3: |
| +// the version this replaces re-implemented "seedFlag && env != |
| +// development" inline and compared it to itself; inverting the real |
| +// if, reordering its operands or dropping a clause left it green. |
| +// This calls checkSeedGate directly, so none of that is possible — |
| +// see main.go's own doc comment on checkSeedGate for why this one |
| +// function is the entirety of -seed's safety story. |
| +func TestSeedGateRefusesWithoutDevelopment(t *testing.T) { |
| cases := []struct { |
| name string |
| seed bool |
| env string |
| - wantRun bool |
| + wantErr bool |
| }{ |
| - {name: "seed with development env runs", seed: true, env: "development", wantRun: true}, |
| - {name: "seed with production env is refused", seed: true, env: "production", wantRun: false}, |
| - {name: "seed with empty env is refused", seed: true, env: "", wantRun: false}, |
| - {name: "seed with staging env is refused", seed: true, env: "staging", wantRun: false}, |
| - {name: "no seed flag never runs regardless of env", seed: false, env: "development", wantRun: false}, |
| + {name: "seed with development env runs", seed: true, env: "development", wantErr: false}, |
| + {name: "seed with production env is refused", seed: true, env: "production", wantErr: true}, |
| + {name: "seed with empty env is refused", seed: true, env: "", wantErr: true}, |
| + {name: "seed with staging env is refused", seed: true, env: "staging", wantErr: true}, |
| + {name: "no seed flag never refuses regardless of env", seed: false, env: "production", wantErr: false}, |
| } |
| for _, c := range cases { |
| t.Run(c.name, func(t *testing.T) { |
| - // This is main()'s own gate, "if seedFlag && env != |
| - // \"development\"", inverted into "would seeding run". |
| - run := c.seed && c.env == "development" |
| - if run != c.wantRun { |
| - t.Errorf("seed=%v env=%q: gate allows running = %v, want %v", c.seed, c.env, run, c.wantRun) |
| + err := checkSeedGate(c.seed, c.env) |
| + if (err != nil) != c.wantErr { |
| + t.Errorf("checkSeedGate(%v, %q) = %v, wantErr %v", c.seed, c.env, err, c.wantErr) |
| } |
| }) |
| } |