From ddc0cb85b6b2d27a6c3b3920dd45359238a7a934 Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Tue, 14 Jul 2026 13:50:46 +0100 Subject: [PATCH] fix(coverage): record a fully-carried baseline when a main run measures nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A docs-only merge to main measures nothing: every coverage producer is path-filtered away, and --tests=skip finds no reports. The old code early-returned "no coverage measured" before the record path, leaving that main commit with no baseline at all — so the first PR against it recomputed nothing in the bare worktree, reported every language as [NEW], and gated on nothing (wardnet/wardnet#899). The record-on-main path now runs before the empty-report bailout: the carry-forward fills every detected language from the nearest prior baseline, so a nothing-measured main run still records a full baseline. "no coverage measured" is only printed when there is truly nothing to record — nothing measured and no priors to carry. Merge-base/HEAD resolution is now lenient so a repo with no origin/main still gets the message instead of a merge-base error. Claude-Session: https://claude.ai/code/session_01QMTWVM4pUB4xy6F6P9wWYA --- AGENTS.md | 7 ++- cmd/bulwark/coverage.go | 58 +++++++++++++++++------- cmd/bulwark/coverage_test.go | 87 ++++++++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+), 18 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6c00e17..195ca5a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -136,7 +136,12 @@ generated cache data, not source, needs no PR/review and never pollutes main's h longer *detected*, so its entry still dies with it; only "the code is there but this run didn't measure it" carries forward. The `recorded coverage baseline` line names anything carried, and an unmeasured language *no* prior had is named in a stderr warning (shallow history is the usual - culprit) instead of vanishing silently. + culprit) instead of vanishing silently. This applies even when a main run measures **nothing** + (a docs-only merge: every producer path-filtered away, no reports for `--tests=skip` to read) — + the whole baseline is carried rather than skipping the record, because a main commit with no + baseline forces every PR against it into the recompute-nothing → all-`[NEW]` → gate-on-nothing + hole (wardnet/wardnet#899). "No coverage measured" is only printed when there was truly nothing + to record: nothing measured *and* no priors to carry. - `internal/gitstate.BaseSHA` resolves `git merge-base HEAD origin/main`. - `internal/gitstate.ReadBaseline` fetches `bulwark-state` and reads `.json` via `git show` (no checkout) — a missing branch or missing file is a cache miss, not an error. diff --git a/cmd/bulwark/coverage.go b/cmd/bulwark/coverage.go index 0460346..6cb782e 100644 --- a/cmd/bulwark/coverage.go +++ b/cmd/bulwark/coverage.go @@ -63,14 +63,6 @@ func newCoverageCmd() *cobra.Command { if err != nil { return err } - if len(current) == 0 { - msg := "no coverage measured — no coverage tooling detected/available for any ecosystem" - if mode == coverage.ModeSkip { - msg += " (--tests=skip only reads an existing report — did an earlier CI step produce one at the expected path?)" - } - _, err := fmt.Fprintln(cmd.OutOrStdout(), msg) - return err - } // A partially-measured current tree is just as invisible as an // unmeasured one: the language simply doesn't appear in the report. detected, err := detect.Ecosystems(dir, cfg.AllExcludes()) @@ -82,10 +74,11 @@ func newCoverageCmd() *cobra.Command { return err } - sha, err := gitstate.BaseSHA(ctx, dir) - if err != nil { - return err - } + // Resolved leniently: with nothing measured (a repo without an + // origin/main, say) the answer below is "no coverage measured", + // not a merge-base error — the errors only surface once they + // block an actual gate. + sha, shaErr := gitstate.BaseSHA(ctx, dir) // Running ON the merge-base (a push to main) rather than ahead of it // (a PR): there is no baseline to gate against — the current commit @@ -101,15 +94,24 @@ func newCoverageCmd() *cobra.Command { // worktree were numbers it had already measured, and thrown away, when // this same command ran on main. Recording them costs nothing: no // re-run, no cargo-llvm-cov, no yarn — they are already in hand. - head, err := gitstate.HeadSHA(ctx, dir) - if err != nil { - return err - } - if head == sha { + // + // A main run that measured NOTHING (a docs-only merge: every + // coverage producer path-filtered away, no reports for + // --tests=skip to read) still records — the carry-forward fills + // every detected language from the nearest prior baseline. The + // old early-return here left such a commit with no baseline at + // all, so the first PR against it recomputed nothing, reported + // every language as [NEW], and gated on nothing + // (wardnet/wardnet#899). + head, headErr := gitstate.HeadSHA(ctx, dir) + if shaErr == nil && headErr == nil && head == sha { record, carried, err := carryForwardBaseline(ctx, cmd, dir, sha, ecosystems, current) if err != nil { return err } + if len(record) == 0 { + return printNoCoverage(cmd, mode) + } if err := gitstate.WriteBaseline(ctx, dir, sha, record); err != nil { // Best-effort, as everywhere else: a write race with a concurrent // main build must not fail the build. @@ -124,6 +126,16 @@ func newCoverageCmd() *cobra.Command { return err } + if len(current) == 0 { + return printNoCoverage(cmd, mode) + } + if shaErr != nil { + return shaErr + } + if headErr != nil { + return headErr + } + baseline, hit, err := gitstate.ReadBaseline(ctx, dir, sha) if err != nil { return err @@ -607,6 +619,18 @@ func diffReport(cmd *cobra.Command, current, baseline map[string]float64, detect return nil } +// printNoCoverage reports a run that measured nothing and — on main — had no +// prior baseline entries to carry forward either: there is nothing to gate +// and nothing worth recording. +func printNoCoverage(cmd *cobra.Command, mode coverage.Mode) error { + msg := "no coverage measured — no coverage tooling detected/available for any ecosystem" + if mode == coverage.ModeSkip { + msg += " (--tests=skip only reads an existing report — did an earlier CI step produce one at the expected path?)" + } + _, err := fmt.Fprintln(cmd.OutOrStdout(), msg) + return err +} + // enabledEcosystems drops languages disabled in .bulwark.yml from the // detected set. For the coverage gate, `enabled: false` means "stop gating // this language", so it must behave exactly like source removal: undetected, diff --git a/cmd/bulwark/coverage_test.go b/cmd/bulwark/coverage_test.go index c6315ae..d23fbe9 100644 --- a/cmd/bulwark/coverage_test.go +++ b/cmd/bulwark/coverage_test.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "context" "fmt" "os" "path/filepath" @@ -13,8 +14,94 @@ import ( "wardnet/bulwark/internal/config" "wardnet/bulwark/internal/detect" + "wardnet/bulwark/internal/executil" + "wardnet/bulwark/internal/gitstate" ) +// A push to main that measures NOTHING must still record a baseline — +// carried wholesale from the nearest prior one — not early-return "no +// coverage measured". A docs-only merge measures nothing (every coverage +// producer path-filtered away, no reports for --tests=skip to read), and +// skipping the record leaves that main commit with no baseline at all: the +// first PR against it recomputes nothing in a bare worktree, reports every +// language as [NEW], and the gate enforces nothing (wardnet/wardnet#899). +func TestCoverageOnMainRecordsFullyCarriedBaselineWhenNothingMeasured(t *testing.T) { + ctx := context.Background() + run := func(dir string, args ...string) { + t.Helper() + if r := executil.Run(ctx, dir, "git", args...); !r.Ok() { + t.Fatalf("git %v: %v\n%s", args, r.Err, r.Output) + } + } + revParse := func(dir, ref string) string { + t.Helper() + r := executil.Run(ctx, dir, "git", "rev-parse", ref) + if !r.Ok() { + t.Fatalf("rev-parse %s: %v", ref, r.Err) + } + return strings.TrimSpace(r.Output) + } + + origin := t.TempDir() + run(origin, "init", "--bare", "-b", "main", ".") + + // The consumer checkout: c1 (code) then c2 (docs-only) on main, with + // HEAD == origin/main — the record-on-main shape. + repo := t.TempDir() + run(repo, "init", "-b", "main", ".") + run(repo, "config", "user.email", "t@t") + run(repo, "config", "user.name", "t") + if err := os.WriteFile(filepath.Join(repo, "package.json"), []byte("{}"), 0o600); err != nil { + t.Fatal(err) + } + run(repo, "add", "-A") + run(repo, "commit", "-m", "code") + c1 := revParse(repo, "HEAD") + if err := os.WriteFile(filepath.Join(repo, "README.md"), []byte("docs"), 0o600); err != nil { + t.Fatal(err) + } + run(repo, "add", "-A") + run(repo, "commit", "-m", "docs only") + c2 := revParse(repo, "HEAD") + run(repo, "remote", "add", "origin", origin) + run(repo, "push", "origin", "main") + run(repo, "fetch", "origin") + + // bulwark-state already carries c1's baseline. + seed := t.TempDir() + run(seed, "init", "-b", gitstate.BranchName, ".") + run(seed, "config", "user.email", "t@t") + run(seed, "config", "user.name", "t") + if err := os.WriteFile(filepath.Join(seed, c1+".json"), []byte(`{"typescript":93.8}`), 0o600); err != nil { + t.Fatal(err) + } + run(seed, "add", "-A") + run(seed, "commit", "-m", "baseline") + run(seed, "remote", "add", "origin", origin) + run(seed, "push", "origin", gitstate.BranchName) + + cmd := newCoverageCmd() + var out, errOut bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&errOut) + cmd.SetArgs([]string{"--dir", repo, "--tests", "skip"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("coverage on main with nothing measured: %v\nstdout: %s\nstderr: %s", err, out.String(), errOut.String()) + } + if !strings.Contains(out.String(), "recorded coverage baseline") { + t.Errorf("expected a recorded-baseline line, got stdout: %q", out.String()) + } + + run(repo, "fetch", "origin", gitstate.BranchName) + r := executil.Run(ctx, repo, "git", "show", "origin/"+gitstate.BranchName+":"+c2+".json") + if !r.Ok() { + t.Fatalf("no baseline recorded for %s: %v\nstdout: %s\nstderr: %s", c2, r.Err, out.String(), errOut.String()) + } + if !strings.Contains(r.Output, "93.8") { + t.Errorf("baseline for %s = %s, want typescript 93.8 carried from %s", c2, r.Output, c1) + } +} + // unmeasuredLanguages is the single source of truth for "detected but not // measured this run" — the predicate the unmeasured warning, the // carry-forward trigger, and the merge all share, so it cannot drift between