diff --git a/.abcd/work/issues/open/iss-2610251555094324-make-preflight-cannot-land-a-one-record.md b/.abcd/work/issues/resolved/iss-2610251555094324-make-preflight-cannot-land-a-one-record.md similarity index 78% rename from .abcd/work/issues/open/iss-2610251555094324-make-preflight-cannot-land-a-one-record.md rename to .abcd/work/issues/resolved/iss-2610251555094324-make-preflight-cannot-land-a-one-record.md index 832a171532..f89b000b34 100644 --- a/.abcd/work/issues/open/iss-2610251555094324-make-preflight-cannot-land-a-one-record.md +++ b/.abcd/work/issues/resolved/iss-2610251555094324-make-preflight-cannot-land-a-one-record.md @@ -11,6 +11,8 @@ production_mode: hand-written found_at: "Makefile" remedy: "Let preflight mint a receipt without the test and race stages for a diff that touches only the record tiers (.abcd/work/, .abcd/development/), using the same fail-closed path classifier CI uses to stand the race lane down on a pull request confined to those paths, so the receipt's scope matches what CI would run." refines: [iss-2610161639025702] +resolution: "make preflight runs precheck's tests in place of the unit and race stages for a committed change confined to the record tiers that no test names, judged fail-closed by scripts/records-only.sh against the merge base with origin's default branch; the run says so and its receipt names the scope, the verdict and the merge base" +impact: internal --- make preflight cannot land a one-record change on a busy machine: on 2026-10-24 a branch whose whole diff is one new issue record (iss-2610241228359748) ran preflight twice, every gate before the test stages passed both times, and both runs then waited the load gate's full two hours (abcd implement load --wait --timeout 2h) behind two to four running preflights and six to eight queued waiters, and failed with 'load gate still shut after 2h0m0s'. With no receipt the pre-push hook refuses the push, so a change that touches no Go code waits behind the full test and race stages of every other session and in practice cannot be pushed. diff --git a/Makefile b/Makefile index b38567e4b8..9c0a0386e0 100644 --- a/Makefile +++ b/Makefile @@ -279,6 +279,27 @@ asking-sync: asking-sync-check: @go run ./cmd/asking-sync -check +# A records-only change (iss-2610251555094324) takes a shorter preflight: when +# every path HEAD changes against the merge base with origin's default branch +# lies in .abcd/development/ or .abcd/work/, no test names one of them (by its +# path, its file name or its record id), and the +# tree matches HEAD, scripts/records-only.sh says so and the second preflight +# rule below runs in place of the first. It runs the same gates, build and vet, +# then precheck's test lines in place of the unit and race stages, and says so +# before its tests and in the receipt it mints. Anything else, a failed check included, +# is the full rule. The verdict is read while this Makefile is parsed, as the +# starting state is, and only yes or nothing reaches make: the reason can name a +# changed path, so it is printed by the script itself, never placed in a recipe +# line. The records rule's first step and its mint each run the classifier +# again, so a verdict that no longer holds fails the run rather than skipping +# its tests. `make preflight PREFLIGHT_RECORDS_ONLY=` takes the full rule +# whatever the change. TestPreflightRecordsRuleRunsPrechecksTests holds the +# second rule's gates to the first's and its test lines to precheck's. +ifneq ($(filter preflight,$(MAKECMDGOALS)),) +PREFLIGHT_RECORDS_ONLY := $(shell scripts/records-only.sh >/dev/null 2>&1 && echo yes) +endif + +ifneq ($(PREFLIGHT_RECORDS_ONLY),yes) # Pre-push gate (run before a push, never by it: .githooks/pre-push checks the # receipt the last step mints, below): the load check first (a # warning, never a failure: load-check), then the format gate (fmt-check), the @@ -360,11 +381,25 @@ preflight: load-check fmt-check lint-reviews lint-issues record-lint issue-drift go test -race -p "$$p" -timeout 40m ./internal/surface/cli ./internal/core/reading ./internal/core/launch ./internal/core/lifeboat ./internal/core/lint ./internal/adapter/scanner ./internal/core/implement/loop ./internal/core/interview ./internal/core/capture ./internal/core/ahoy ./internal/core/site ./internal/... @scripts/preflight-receipt.sh mint "$(PREFLIGHT_BEGAN)" +else +preflight: load-check fmt-check lint-reviews lint-issues record-lint issue-drift docs-lint check-attribution secrets-check vulncheck site-render smoke evals-cold-reading + @scripts/records-only.sh + @echo "preflight: a records-only change, so the unit and race stages are skipped and precheck's tests run in their place; CI's unit lane still runs every test" + go build ./... + go vet ./... + go test -timeout 20m ./internal/core/lint ./internal/core/launch/scaffold ./internal/core/mdrecord ./internal/core/issueschema ./internal/core/guard ./internal/core/glossary ./internal/core/reading ./internal/core/recordid ./internal/core/changelog ./internal/core/intent ./internal/core/release ./internal/core/releasedigest ./internal/core/decisions ./internal/core/record/match ./internal/core/lifeboat + go test -timeout 20m -run '$(PRECHECK_AUDIT_TESTS)' ./internal/surface/cli ./internal/core/site + @scripts/preflight-receipt.sh mint "$(PREFLIGHT_BEGAN)" records-only +endif + # The precheck (iss-2610230016523625): the minutes-long pass a lane runs before # it queues a preflight, so a test that reads the whole tree fails before the # queue rather than after it. It runs preflight's gates (all but the load check, # which reads the machine and only warns), build and vet, the packages whose # tests audit the record, the generated pages and the agent definitions, in full, +# and the packages whose tests read the committed record tree whole (record ids +# and slugs, the changelog derivation, intents, release pages, decisions, the +# brief's lifeboat table), # and the command and site packages' audit tests, chosen by name: a test in those # two packages that reads the committed tree carries one of the PRECHECK_AUDIT_TESTS # words in its name, which is how a new one joins without a list to keep. It skips @@ -377,7 +412,7 @@ PRECHECK_AUDIT_TESTS := Surface|Page|Brief|Parity|Reference|Snapshot|Appendix|Ap precheck: fmt-check lint-reviews lint-issues record-lint issue-drift docs-lint check-attribution secrets-check vulncheck site-render smoke evals-cold-reading go build ./... go vet ./... - go test -timeout 20m ./internal/core/lint ./internal/core/launch/scaffold ./internal/core/mdrecord ./internal/core/issueschema ./internal/core/guard ./internal/core/glossary ./internal/core/reading + go test -timeout 20m ./internal/core/lint ./internal/core/launch/scaffold ./internal/core/mdrecord ./internal/core/issueschema ./internal/core/guard ./internal/core/glossary ./internal/core/reading ./internal/core/recordid ./internal/core/changelog ./internal/core/intent ./internal/core/release ./internal/core/releasedigest ./internal/core/decisions ./internal/core/record/match ./internal/core/lifeboat go test -timeout 20m -run '$(PRECHECK_AUDIT_TESTS)' ./internal/surface/cli ./internal/core/site precheck: export GOTOOLCHAIN := go$(GO_TOOLCHAIN_VERSION) diff --git a/internal/core/lint/precheck_test.go b/internal/core/lint/precheck_test.go index 3c3cecf9c9..a9d3d2190c 100644 --- a/internal/core/lint/precheck_test.go +++ b/internal/core/lint/precheck_test.go @@ -95,7 +95,9 @@ func TestPrecheckRunsTheAuditTests(t *testing.T) { "go build ./...", "go vet ./...", "go test -timeout 20m ./internal/core/lint ./internal/core/launch/scaffold ./internal/core/mdrecord " + - "./internal/core/issueschema ./internal/core/guard ./internal/core/glossary ./internal/core/reading", + "./internal/core/issueschema ./internal/core/guard ./internal/core/glossary ./internal/core/reading " + + "./internal/core/recordid ./internal/core/changelog ./internal/core/intent ./internal/core/release " + + "./internal/core/releasedigest ./internal/core/decisions ./internal/core/record/match ./internal/core/lifeboat", "go test -timeout 20m -run '$(PRECHECK_AUDIT_TESTS)' ./internal/surface/cli ./internal/core/site", } { if !strings.Contains(recipe, want) { diff --git a/internal/core/lint/recordsonly_test.go b/internal/core/lint/recordsonly_test.go new file mode 100644 index 0000000000..43bd045332 --- /dev/null +++ b/internal/core/lint/recordsonly_test.go @@ -0,0 +1,300 @@ +package lint_test + +import ( + "errors" + "os/exec" + "path/filepath" + "slices" + "strings" + "testing" + + "github.com/intentdriven/abcd/internal/gittest" +) + +// recordsOnly runs scripts/records-only.sh in r and returns its line and +// whether it classified HEAD records-only (exit 0) or the full run (exit 1). +func recordsOnly(t *testing.T, r *gittest.Repo) (string, bool) { + t.Helper() + script, err := filepath.Abs(filepath.Join("..", "..", "..", "scripts", "records-only.sh")) + if err != nil { + t.Fatal(err) + } + cmd := exec.Command("bash", script) + cmd.Dir, cmd.Env = r.Root(), r.Env() + out, err := cmd.CombinedOutput() + var exit *exec.ExitError + switch { + case err == nil: + return string(out), true + case errors.As(err, &exit) && exit.ExitCode() == 1: + return string(out), false + } + t.Fatalf("records-only.sh: %v\n%s", err, out) + return "", false +} + +// recordsRepo is a repository whose origin/main is its first commit, holding +// a code file, a test that names one record by its path, and that record. +func recordsRepo(t *testing.T) *gittest.Repo { + t.Helper() + r := gittest.NewRepo(t) + r.Write("internal/x/x.go", "package x\n") + r.Write("internal/x/x_test.go", "package x\n\nconst fixture = \".abcd/work/fixture.md\"\n") + r.Write(".abcd/work/fixture.md", "a record a test reads\n") + r.Write(".abcd/work/issues/open/iss-1.md", "one\n") + r.Commit("base") + r.Git("update-ref", "refs/remotes/origin/main", "HEAD") + return r +} + +// TestRecordsOnlyClassifiesARecordTierChange (iss-2610251555094324): a change +// whose every path lies in .abcd/development/ or .abcd/work/, named by no +// test, is records-only, and the line says why. +func TestRecordsOnlyClassifiesARecordTierChange(t *testing.T) { + r := recordsRepo(t) + r.Write(".abcd/work/issues/open/iss-2.md", "two\n") + r.Write(".abcd/development/intents/drafts/itd-1.md", "an intent\n") + r.Write(".abcd/work/issues/open/iss-1.md", "one, edited\n") + r.Commit("records") + base := r.Git("rev-parse", "origin/main") + out, ok := recordsOnly(t, r) + if want := "records-only: all 3 changed paths since the merge base with origin/main, " + base + ", are in the record tiers, and no test names one\n"; !ok || out != want { + t.Fatalf("a records change = %v, %q; want records-only, %q", ok, out, want) + } + // A deleted record is a changed path in the tiers too. + r.Remove(".abcd/work/issues/open/iss-2.md") + r.Commit("drop one") + if out, ok := recordsOnly(t, r); !ok { + t.Fatalf("a records change with a delete = full run, %q; want records-only", out) + } +} + +// TestRecordsOnlyRunsEverythingElseInFull: the classifier is fail-closed. A +// code change mixed into a records change, a path outside the two tiers (the +// docs and the conventions file included, which CI's own classifier treats +// differently), a record a test names, a file moved out of code into a tier, +// an empty diff, and a checkout with no origin default branch each take the +// full run, and the line names why. +func TestRecordsOnlyRunsEverythingElseInFull(t *testing.T) { + for _, c := range []struct { + name string + edit func(r *gittest.Repo) + want string + }{ + {"code mixed in", func(r *gittest.Repo) { + r.Write(".abcd/work/issues/open/iss-2.md", "two\n") + r.Write("internal/x/x.go", "package x\n\nvar Y = 1\n") + }, "full run: internal/x/x.go is outside the record tiers"}, + {"docs", func(r *gittest.Repo) { r.Write("docs/guide.md", "a page\n") }, "full run: docs/guide.md is outside the record tiers"}, + {"conventions file", func(r *gittest.Repo) { r.Write("AGENTS.md", "rules\n") }, "full run: AGENTS.md is outside the record tiers"}, + {"another .abcd tier", func(r *gittest.Repo) { r.Write(".abcd/config.json", "{}\n") }, "full run: .abcd/config.json is outside the record tiers"}, + {"a record a test names", func(r *gittest.Repo) { r.Write(".abcd/work/fixture.md", "edited\n") }, + "full run: .abcd/work/fixture.md is named by a test (its file name fixture.md), so it is that test's fixture"}, + {"a record a test names by its file name", func(r *gittest.Repo) { + r.Write("internal/x/y_test.go", "package x\n\nvar parts = []string{\".abcd\", \"work\", \"personas.json\"}\n") + r.Commit("a test that builds the path") + r.Git("update-ref", "refs/remotes/origin/main", "HEAD") + r.Write(".abcd/work/personas.json", "[]\n") + }, "full run: .abcd/work/personas.json is named by a test (its file name personas.json), so it is that test's fixture"}, + {"a record a test finds by its id", func(r *gittest.Repo) { + r.Write("internal/x/z_test.go", "package x\n\nconst glob = \"shipped/itd-7-*.md\"\n") + r.Commit("a test that globs for a record by its id") + r.Git("update-ref", "refs/remotes/origin/main", "HEAD") + r.Write(".abcd/development/intents/shipped/itd-7-a-real-intent.md", "an intent\n") + }, "full run: .abcd/development/intents/shipped/itd-7-a-real-intent.md is named by a test (its record id itd-7), so it is that test's fixture"}, + {"a record a test finds by its bare number", func(r *gittest.Repo) { + r.Write("internal/x/n_test.go", "package x\n\nconst id = \"adr-261015072933\"\n") + r.Commit("a test that globs for an ADR by its number") + r.Git("update-ref", "refs/remotes/origin/main", "HEAD") + r.Write(".abcd/development/decisions/adrs/261015072933-a-real-decision.md", "a decision\n") + }, "full run: .abcd/development/decisions/adrs/261015072933-a-real-decision.md is named by a test (its record id 261015072933), so it is that test's fixture"}, + {"a local branch named like origin's", func(r *gittest.Repo) { + r.Write("internal/x/x.go", "package x\n\nvar Y = 1\n") + r.Commit("unmerged code") + r.Git("branch", "origin/main") + r.Write(".abcd/work/issues/open/iss-2.md", "two\n") + }, "full run: internal/x/x.go is outside the record tiers"}, + {"moved out of code", func(r *gittest.Repo) { r.Git("mv", "internal/x/x.go", ".abcd/work/x.go") }, + "full run: internal/x/x.go is outside the record tiers"}, + {"empty", func(r *gittest.Repo) { r.Git("commit", "-q", "--allow-empty", "-m", "nothing") }, + "full run: the diff against the merge base with origin/main is empty"}, + {"no origin", func(r *gittest.Repo) { + r.Write(".abcd/work/issues/open/iss-2.md", "two\n") + r.Git("update-ref", "-d", "refs/remotes/origin/main") + }, "full run: there is no origin/main to diff against"}, + } { + t.Run(c.name, func(t *testing.T) { + r := recordsRepo(t) + c.edit(r) + if r.Git("status", "--porcelain") != "" { + r.Commit(c.name) + } + out, ok := recordsOnly(t, r) + if ok || !strings.HasPrefix(out, c.want) { + t.Fatalf("records-only = %v, %q; want the full run, %q", ok, out, c.want) + } + }) + } +} + +// TestRecordsOnlyDiffsAgainstOriginsDefaultBranch: the merge base is taken +// with origin/HEAD when it names the default branch, so a repository whose +// default branch is not main is judged against it. +func TestRecordsOnlyDiffsAgainstOriginsDefaultBranch(t *testing.T) { + r := recordsRepo(t) + r.Git("update-ref", "refs/remotes/origin/trunk", "HEAD") + r.Git("update-ref", "-d", "refs/remotes/origin/main") + r.Git("symbolic-ref", "refs/remotes/origin/HEAD", "refs/remotes/origin/trunk") + r.Write(".abcd/work/issues/open/iss-2.md", "two\n") + r.Commit("records") + if out, ok := recordsOnly(t, r); !ok || !strings.Contains(out, "merge base with origin/trunk") { + t.Fatalf("against origin/trunk = %v, %q; want records-only against it", ok, out) + } +} + +// TestRecordsOnlyKeepsControlBytesOffTheTerminal: the line can name a +// changed path, which a writer chose, so a control byte in it never reaches +// the terminal. +func TestRecordsOnlyKeepsControlBytesOffTheTerminal(t *testing.T) { + for _, c := range []struct{ name, file, want string }{ + {"a C0 escape", "x\x1b[2Jy.go", "x[2Jy.go is outside"}, + {"a C1 control sequence introducer", "x\u009b2Jy.go", "x2Jy.go is outside"}, + {"a bidirectional override", "x\u202egp.go", "xgp.go is outside"}, + } { + t.Run(c.name, func(t *testing.T) { + r := recordsRepo(t) + r.Write(c.file, "package x\n") + r.Commit("hostile") + out, ok := recordsOnly(t, r) + for _, b := range []byte(strings.TrimSuffix(out, "\n")) { + if b < 0x20 || b > 0x7e { + t.Fatalf("a hostile path printed byte %#x: %q", b, out) + } + } + if ok || !strings.Contains(out, c.want) { + t.Fatalf("a hostile path = %v, %q; want the full run naming %q", ok, out, c.want) + } + }) + } +} + +// TestRecordsOnlyLetsACitationInACommentPass: a test that cites a record's id +// in prose (itd-8, or itd-8)) does not make that record its fixture; only the +// id followed by a dash, the start of its file name, does. +func TestRecordsOnlyLetsACitationInACommentPass(t *testing.T) { + r := recordsRepo(t) + r.Write("internal/x/c_test.go", "package x\n\n// The rule itd-8 states (see itd-8).\n") + r.Commit("a test that cites an intent") + r.Git("update-ref", "refs/remotes/origin/main", "HEAD") + r.Write(".abcd/development/intents/drafts/itd-8-a-cited-intent.md", "an intent\n") + r.Commit("the intent") + if out, ok := recordsOnly(t, r); !ok { + t.Fatalf("a record a test only cites = full run, %q; want records-only", out) + } +} + +// TestRecordsOnlyReadsANewlineInAPathAsOnePath: the changed paths are read +// NUL-delimited, so a path holding a newline is one path, judged whole: a code +// file whose name holds a newline is not split into names that could each look +// like a record. A record whose name holds one cannot be searched for whole +// (git grep splits a pattern at a newline), so it takes the full run too. +func TestRecordsOnlyReadsANewlineInAPathAsOnePath(t *testing.T) { + r := recordsRepo(t) + r.Write("x\n.abcd/work/d.md", "package x\n") + r.Commit("a code file named with a newline") + if out, ok := recordsOnly(t, r); ok || !strings.HasPrefix(out, "full run: x.abcd/work/d.md is outside") { + t.Fatalf("a code file named with a newline = %v, %q; want the full run naming it whole", ok, out) + } + r = recordsRepo(t) + r.Write(".abcd/work/a\nb.md", "a record\n") + r.Commit("a record with a newline in its name") + if out, ok := recordsOnly(t, r); ok || !strings.HasPrefix(out, "full run: .abcd/work/ab.md holds a newline in its name") { + t.Fatalf("a record named with a newline = %v, %q; want the full run, saying why", ok, out) + } +} + +// TestRecordsOnlyRunsAnUncommittedEditInFull: the change is judged by its +// commits, so a tree that differs from HEAD (an edited code file, an untracked +// one, an edit an index flag hides) takes the full run, or the edit's tests +// would be skipped. +func TestRecordsOnlyRunsAnUncommittedEditInFull(t *testing.T) { + for _, c := range []struct { + name string + edit func(r *gittest.Repo) + }{ + {"an edited code file", func(r *gittest.Repo) { r.Write("internal/x/x.go", "package x\n\nvar Y = 1\n") }}, + {"an untracked code file", func(r *gittest.Repo) { r.Write("internal/x/z.go", "package x\n") }}, + {"a hidden edit", func(r *gittest.Repo) { + r.Git("update-index", "--skip-worktree", "internal/x/x.go") + r.Write("internal/x/x.go", "package x\n\nvar Y = 1\n") + }}, + {"an assume-unchanged edit", func(r *gittest.Repo) { + r.Git("update-index", "--assume-unchanged", "internal/x/x.go") + r.Write("internal/x/x.go", "package x\n\nvar Y = 1\n") + }}, + } { + t.Run(c.name, func(t *testing.T) { + r := recordsRepo(t) + r.Write(".abcd/work/issues/open/iss-2.md", "two\n") + r.Commit("records") + c.edit(r) + want := "full run: the working tree differs from HEAD" + if out, ok := recordsOnly(t, r); ok || !strings.HasPrefix(out, want) { + t.Fatalf("records-only = %v, %q; want the full run, %q", ok, out, want) + } + }) + } +} + +// TestPreflightRecordsRuleRunsPrechecksTests: the Makefile carries two +// preflight rules, the full one first, so every reader of the first rule (the +// load gates, the race lane, the receipt) reads the full run. The second, taken +// for a records-only change, runs the same gates, checks the verdict again +// before anything else, runs precheck's build, vet and test lines and nothing +// else, and mints a receipt that says it is records-only. +func TestPreflightRecordsRuleRunsPrechecksTests(t *testing.T) { + makefile := readRepoFile(t, filepath.Join("..", "..", ".."), "Makefile") + lines := strings.Split(makefile, "\n") + var rules []int + for i, l := range lines { + if strings.HasPrefix(l, "preflight:") && !strings.HasPrefix(l, "preflight: export ") { + rules = append(rules, i) + } + } + if len(rules) != 2 { + t.Fatalf("the Makefile declares %d preflight rules, want 2 (the full run, then the records-only run)", len(rules)) + } + if lines[rules[0]] != lines[rules[1]] { + t.Errorf("the records-only rule's gates differ from the full rule's:\n%s\n%s", lines[rules[1]], lines[rules[0]]) + } + var body []string + for _, l := range lines[rules[1]+1:] { + if !strings.HasPrefix(l, "\t") { + break + } + body = append(body, strings.TrimPrefix(l, "\t")) + } + if !slices.Contains(lines[rules[0]:rules[1]], "else") || !slices.Contains(lines[rules[1]:], "endif") { + t.Fatal("the two preflight rules are not the two branches of one conditional") + } + precheck, ok := makeRecipe(makefile, "precheck") + if !ok { + t.Fatal("Makefile declares no `precheck:` recipe") + } + var want []string + for _, l := range strings.Split(precheck, "\n") { + want = append(want, strings.TrimPrefix(l, "\t")) + } + if len(body) != len(want)+3 { + t.Fatalf("the records-only recipe runs %q; want the verdict checked, the skip announced, precheck's %q, then the mint", body, want) + } + if body[0] != "@scripts/records-only.sh" || !strings.HasPrefix(body[1], `@echo "preflight: a records-only change, so the unit and race stages are skipped`) { + t.Errorf("the records-only recipe does not check the verdict and say what it skips before it runs anything: %q", body[:2]) + } + if got := body[2 : len(body)-1]; !slices.Equal(got, want) { + t.Errorf("the records-only recipe runs %q; want precheck's lines, %q", got, want) + } + if last := body[len(body)-1]; last != `@scripts/preflight-receipt.sh mint "$(PREFLIGHT_BEGAN)" records-only` { + t.Errorf("the records-only recipe ends %q; want the records-only receipt minted", last) + } +} diff --git a/internal/surface/cli/githook_prepush_test.go b/internal/surface/cli/githook_prepush_test.go index 8e7f4221b0..22dcb5ee91 100644 --- a/internal/surface/cli/githook_prepush_test.go +++ b/internal/surface/cli/githook_prepush_test.go @@ -7,6 +7,7 @@ import ( "path/filepath" "regexp" "runtime" + "slices" "strings" "testing" @@ -67,7 +68,7 @@ func newPrePushCaseIn(t *testing.T, format string) *prePushCase { c.git("config", "user.name", "Alice Example") c.git("config", "user.email", "alice@example.com") - for _, rel := range []string{".githooks/pre-push", "scripts/preflight-receipt.sh"} { + for _, rel := range []string{".githooks/pre-push", "scripts/preflight-receipt.sh", "scripts/records-only.sh"} { src, err := os.ReadFile(filepath.Join(root, filepath.FromSlash(rel))) if err != nil { t.Fatalf("read the committed %s: %v", rel, err) @@ -156,13 +157,13 @@ func (c *prePushCase) receipt(dir string, args ...string) (string, error) { // preflight stands in for `make preflight`: the state before, the gates, the mint // after. The Makefile wiring that does the same is pinned separately below. -func (c *prePushCase) preflight(dir string) string { +func (c *prePushCase) preflight(dir string, scope ...string) string { c.t.Helper() began, err := c.receipt(dir, "state") if err != nil { c.t.Fatalf("state: %v\n%s", err, began) } - out, err := c.receipt(dir, "mint", strings.TrimSpace(began)) + out, err := c.receipt(dir, append([]string{"mint", strings.TrimSpace(began)}, scope...)...) if err != nil { c.t.Fatalf("mint: %v\n%s", err, out) } @@ -497,7 +498,9 @@ func TestMakePreflightMintsTheReceiptAfterItsLastGate(t *testing.T) { if err != nil { t.Skip("not in a git checkout") } - cmd := exec.Command("make", "-n", "preflight") + // The full rule whatever this checkout's change is: a records-only one would + // read the second rule, which the lint package pins. + cmd := exec.Command("make", "-n", "preflight", "PREFLIGHT_RECORDS_ONLY=") cmd.Dir = strings.TrimSpace(string(out)) cmd.Env = gittest.Env(t) dry, err := cmd.CombinedOutput() @@ -523,3 +526,109 @@ func TestMakePreflightMintsTheReceiptAfterItsLastGate(t *testing.T) { t.Errorf("the receipt is not minted after the race-enabled tests\n%s", dry) } } + +// receiptLines is the receipt the clone holds for a commit, line by line. +func (c *prePushCase) receiptLines(commit string) []string { + c.t.Helper() + b, err := os.ReadFile(filepath.Join(c.dir, ".abcd", ".work.local", "preflight-receipts", commit)) + if err != nil { + c.t.Fatalf("read the receipt for %s: %v", commit, err) + } + return strings.Split(strings.TrimSuffix(string(b), "\n"), "\n") +} + +// TestPreflightReceiptNamesItsScope (iss-2610251555094324): a receipt says +// whether the unit and race stages ran. A records-only one carries the +// classifier's verdict and the merge base it was taken at, so a later reader +// can redo the check, and it lets the push through like a full one. +func TestPreflightReceiptNamesItsScope(t *testing.T) { + t.Run("full", func(t *testing.T) { + c := newPrePushCase(t) + head := c.commit("feature.md", "a feature\n") + c.preflight(c.dir) + if got := c.receiptLines(head); len(got) != 3 || got[2] != "scope full" { + t.Fatalf("a full preflight's receipt is %q; want it to name the full scope", got) + } + }) + t.Run("records-only", func(t *testing.T) { + c := newPrePushCase(t) + base := c.git("rev-parse", "origin/main") + head := c.commit(".abcd/work/issues/open/iss-1.md", "one\n") + out := c.preflight(c.dir, "records-only") + if !strings.Contains(out, "receipt minted for "+head[:12]+", records-only") { + t.Fatalf("a records-only preflight does not say its receipt is records-only\n%s", out) + } + want := []string{ + "scope records-only", + "verdict records-only: all 1 changed paths since the merge base with origin/main, " + base + + ", are in the record tiers, and no test names one", + "base " + base, + } + if got := c.receiptLines(head); len(got) != 5 || !slices.Equal(got[2:], want) { + t.Fatalf("the records-only receipt is %q; want it to end %q", got, want) + } + if out, err := c.push("origin", "feature"); err != nil { + t.Fatalf("a commit with a records-only receipt was refused\n%s", out) + } + }) +} + +// TestPreflightReceiptRefusesARecordsOnlyRunOverACodeChange: a records-only +// receipt is minted only when the classifier, run again at the mint, still +// reads the change as records-only, so a code change mixed into a records +// change gets no receipt for a run that skipped its tests. +func TestPreflightReceiptRefusesARecordsOnlyRunOverACodeChange(t *testing.T) { + c := newPrePushCase(t) + c.commit(".abcd/work/issues/open/iss-1.md", "one\n") + head := c.commit("feature.go", "package feature\n") + out := c.preflight(c.dir, "records-only") + if strings.Contains(out, "receipt minted for") || !strings.Contains(out, "no longer reads as records-only (full run: feature.go is outside") { + t.Fatalf("a records-only mint over a code change = %q; want it refused, naming the code file", out) + } + if _, err := os.Stat(filepath.Join(c.dir, ".abcd", ".work.local", "preflight-receipts", head)); err == nil { + t.Fatal("a records-only run over a code change left a receipt") + } + if pushed, err := c.push("origin", "feature"); err == nil { + t.Fatalf("a code change whose preflight skipped its tests was pushed\n%s", pushed) + } +} + +// TestMakePreflightTakesTheRecordsRuleOnlyForARecordsChange runs the +// committed Makefile, dry, in a clone whose change is records alone, then the +// same change with a code file mixed in: the first takes the records-only rule +// (no race stage, a records-only receipt) and the second the full one. +func TestMakePreflightTakesTheRecordsRuleOnlyForARecordsChange(t *testing.T) { + if _, err := exec.LookPath("make"); err != nil { + t.Skip("make unavailable") + } + c := newPrePushCase(t) + root := prePushSourceRoot(t) + // go.mod too: the Makefile reads the toolchain it declares as it parses. + for _, rel := range []string{"Makefile", "go.mod", "scripts/pinned-toolchain.sh"} { + src, err := os.ReadFile(filepath.Join(root, filepath.FromSlash(rel))) + if err != nil { + t.Fatalf("read the committed %s: %v", rel, err) + } + c.write(rel, string(src), 0o755) + } + c.git("add", "-A") + c.git("-c", "core.hooksPath=/dev/null", "commit", "-q", "-m", "the committed Makefile") + c.git("update-ref", "refs/remotes/origin/main", "HEAD") + dry := func() string { + t.Helper() + out, err := c.run(c.dir, "make", "-n", "preflight") + if err != nil { + t.Fatalf("make -n preflight: %v\n%s", err, out) + } + return out + } + c.commit(".abcd/work/issues/open/iss-1.md", "one\n") + if out := dry(); strings.Contains(out, "go test -race") || !strings.Contains(out, "\nscripts/records-only.sh\n") || + !regexp.MustCompile(`(?m)^scripts/preflight-receipt\.sh mint "[0-9a-f]{40} clean" records-only$`).MatchString(out) { + t.Fatalf("a records change did not take the records-only rule:\n%s", out) + } + c.commit("feature.go", "package feature\n") + if out := dry(); !strings.Contains(out, "go test -race") || strings.Contains(out, "records-only") { + t.Fatalf("a code change mixed into a records change did not take the full rule:\n%s", out) + } +} diff --git a/scripts/preflight-receipt.sh b/scripts/preflight-receipt.sh index ed803da67d..584da2ff6f 100755 --- a/scripts/preflight-receipt.sh +++ b/scripts/preflight-receipt.sh @@ -37,9 +37,16 @@ # Usage: # preflight-receipt.sh state print "
clean|dirty|hidden" for this # tree (hidden: an index flag hides edits) -# preflight-receipt.sh mint "