From 68310fbaac211d88cf5ad34c79ba293167820790 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 04:05:10 +0300 Subject: [PATCH 1/7] build: make the gate reproducible, pinned and complete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate was written down in three places and executed in one, and the one execution left several of the repo's own tools unreached. A Makefile now holds the definition and .github/workflows/gate.yml calls its targets, so `make gate` and the CI job are the same commands rather than two lists that agree until somebody edits one. check-coverage.sh mints its coverage profile with mktemp instead of a fixed relative cover.out. Two runs sharing that path interleaved into a profile that was neither run's: three concurrent runs on one tree reported 15220, 15219 and 15215 statements for a tree whose real total is 4942. It also cds to the repo root, so running it from a subdirectory can no longer measure a subtree and report a pass. The suite now runs under -race, in the same execution the coverage profile comes from rather than a second one. scripts/fuzz.sh gives every fuzz target a bounded search, and scripts/bench.sh runs the benchmarks and refuses a run that measured nothing. golangci-lint is pinned to the release CI resolves today, with the version defined once in the Makefile and read back by the workflow. CheckPath's read-error branch is covered by a unix socket rather than a chmod 0o000 file, so it no longer depends on not being root; a pristine checkout previously failed the 100% gate under euid 0 by exactly one statement. BenchmarkAnchorWalk asked to be compared against a whole compile, which did not exist; BenchmarkCompile_Petstore is that number, alongside marshal, unmarshal and validate benchmarks. BenchmarkAnchorWalk itself had never executed — its fixture path was two levels short and it skipped, silently and with exit 0. --- .github/workflows/gate.yml | 35 ++++++- CLAUDE.md | 14 +-- Makefile | 74 +++++++++++++++ compilers/openapi/compile_bench_test.go | 94 +++++++++++++++++++ .../internal/schema/anchorindex_bench_test.go | 11 ++- docs/micro-compiler-plan.md | 4 +- internal/harness/path_test.go | 38 ++++++-- ir/bigval_property_test.go | 12 +-- ir/naming_property_test.go | 15 +-- pass/validate_bench_test.go | 54 +++++++++++ scripts/bench.sh | 54 +++++++++++ scripts/check-coverage.sh | 38 +++++++- scripts/fuzz.sh | 90 ++++++++++++++++++ 13 files changed, 489 insertions(+), 44 deletions(-) create mode 100644 Makefile create mode 100644 compilers/openapi/compile_bench_test.go create mode 100644 pass/validate_bench_test.go create mode 100755 scripts/bench.sh create mode 100755 scripts/fuzz.sh diff --git a/.github/workflows/gate.yml b/.github/workflows/gate.yml index a840d3d2..2c40b312 100644 --- a/.github/workflows/gate.yml +++ b/.github/workflows/gate.yml @@ -3,6 +3,11 @@ on: push: branches: [main] pull_request: +# Every check below runs a Makefile target, so `make gate` on a developer machine +# is these same commands. Lint is the one step that does not: the action installs +# the pinned release and runs it, reading the pin back from the Makefile so the +# version still has a single definition. Add a check to the Makefile and give it +# a step here; do not spell a command out in this file. jobs: gate: runs-on: ubuntu-latest @@ -12,12 +17,32 @@ jobs: with: go-version: "1.26.3" - name: gofmt - run: test -z "$(gofmt -l $(git ls-files '*.go'))" + run: make fmt - name: vet - run: go vet ./... + run: make vet + - name: lint version + id: lint-version + run: echo "version=$(make -s print-lint-version)" >>"$GITHUB_OUTPUT" - name: lint uses: golangci/golangci-lint-action@v8.0.0 + with: + version: ${{ steps.lint-version.outputs.version }} - name: build - run: go build ./... - - name: coverage - run: ./scripts/check-coverage.sh + run: make build + - name: test (race detector, exactly 100% coverage) + run: make coverage + - name: fuzz + id: fuzz + run: make fuzz + - name: benchmarks + run: make bench-smoke + # A crash the fuzz step finds is written to /testdata/fuzz// + # in the runner's workspace and would otherwise die with it, leaving a red + # gate and no reproducer to commit. + - name: upload fuzz findings + if: failure() && steps.fuzz.outcome == 'failure' + uses: actions/upload-artifact@v4 + with: + name: fuzz-findings + path: "**/testdata/fuzz/**" + if-no-files-found: ignore diff --git a/CLAUDE.md b/CLAUDE.md index 7dc8d414..2bce37cf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -206,17 +206,17 @@ below are the ones most likely to bite in this codebase — the full guide gover - **Logging:** `log/slog` only, injected — but note the stronger repo invariant: pipeline stages don't log at all; they return diagnostics. -Before claiming any Go work done, run and pass the same checks CI's `gate` job runs -(`.github/workflows/gate.yml`), in that order: +Before claiming any Go work done, run and pass the gate: ```bash -gofmt -l $(git ls-files '*.go') # must print nothing -go vet ./... -golangci-lint run # must pass clean -go build ./... -./scripts/check-coverage.sh # go test ./... + exactly 100% statement coverage +make gate ``` +That is not a summary of CI — it is what CI runs. Every check in `.github/workflows/gate.yml` runs a +`Makefile` target, so the local command and the job are the same commands by construction. Read the +`Makefile` for the step list rather than restating it here; `make coverage`, `make fuzz`, `make +bench` and the rest are individually runnable while iterating. + **Coverage is a gate at exactly 100%, not a target.** `scripts/check-coverage.sh` counts statements from the profile rather than reading `go test`'s rounded percentage, so one uncovered statement fails the build — `go test ./...` passing locally is not evidence the gate passes. diff --git a/Makefile b/Makefile new file mode 100644 index 00000000..3bab2b3d --- /dev/null +++ b/Makefile @@ -0,0 +1,74 @@ +# The gate, defined once. +# +# .github/workflows/gate.yml runs these targets, so `make gate` and the CI job +# are the same commands by construction rather than by two lists that agree until +# somebody edits one. Add a check here and the workflow gains a step that calls +# it; there is no second place to keep in step. +# +# make the whole gate, in CI order +# make coverage the suite once, under -race, at exactly 100% coverage +# make bench benchmark timings (the gate only smoke-runs them) +# make fuzz FUZZTIME=5m a longer search than the gate's + +GO ?= go + +# The golangci-lint release CI installs. The workflow reads it back from +# `make print-lint-version`, so the pin has one definition and no copy: without +# it the action installs whatever it resolves as latest that day, and an +# upstream release reddens main with no change to this repo. +GOLANGCI_LINT_VERSION ?= v2.12.2 + +# Per-target fuzz budget, bounded on purpose. The gate's job is to keep every +# target executable and to search a little on every change; a campaign is what +# `make fuzz FUZZTIME=5m` is for. +FUZZTIME ?= 10s + +.DEFAULT_GOAL := gate + +.PHONY: gate fmt vet lint build coverage fuzz bench bench-smoke print-lint-version + +gate: fmt vet lint build coverage fuzz bench-smoke + +fmt: + @unformatted="$$(gofmt -l $$(git ls-files '*.go'))"; \ + if [ -n "$$unformatted" ]; then \ + echo "gofmt: not formatted:" >&2; \ + echo "$$unformatted" >&2; \ + exit 1; \ + fi + +vet: + $(GO) vet ./... + +# CI installs the pinned release through golangci-lint-action; here the binary +# is whatever is on PATH. A mismatch is reported rather than fatal — a developer +# on a newer release should know their finding set is not CI's, but a tool +# version is not a reason to refuse to run the gate at all. +lint: + @have="$$(golangci-lint version --short 2>/dev/null || true)"; \ + want="$(GOLANGCI_LINT_VERSION)"; \ + if [ "$$have" != "$${want#v}" ]; then \ + echo "warning: local golangci-lint is $${have:-absent}, CI pins $$want" >&2; \ + echo "warning: go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$$want" >&2; \ + fi + golangci-lint run + +build: + $(GO) build ./... + +coverage: + ./scripts/check-coverage.sh + +fuzz: + ./scripts/fuzz.sh $(FUZZTIME) + +bench: + ./scripts/bench.sh + +# One iteration of every benchmark: enough to prove they still build, still find +# their fixtures and still complete, too few to mean anything as a timing. +bench-smoke: + ./scripts/bench.sh 1x + +print-lint-version: + @echo $(GOLANGCI_LINT_VERSION) diff --git a/compilers/openapi/compile_bench_test.go b/compilers/openapi/compile_bench_test.go new file mode 100644 index 00000000..b6f9c6b2 --- /dev/null +++ b/compilers/openapi/compile_bench_test.go @@ -0,0 +1,94 @@ +package openapi_test // external test package — exercises only the public API + +import ( + "encoding/json" + "os" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/dexpace/morphic/compilers" + "github.com/dexpace/morphic/compilers/openapi" + "github.com/dexpace/morphic/ir" +) + +// BenchmarkCompile_Petstore measures one whole compile of the golden petstore — +// parse, lower, assemble — which is the pipeline stage every other cost is +// judged against. +// +// It is also the denominator BenchmarkAnchorWalk asks for. That benchmark's +// claim is a ratio: the $dynamicAnchor walk is small next to a compile, which is +// why the index stays a memo instead of being derived at entry. A ratio needs +// both numbers, measured over the same corpus in the same way. +func BenchmarkCompile_Petstore(b *testing.B) { + data := petstoreSpec(b) + c := openapi.New() + src := []compilers.Source{{Path: "petstore.yaml", Data: data}} + + b.ReportAllocs() + b.ResetTimer() + for range b.N { + doc, _, err := c.Compile(b.Context(), src, compilers.Options{}) + if err != nil { + b.Fatalf("compile: %v", err) + } + if doc == nil { + b.Fatal("compile produced no document") + } + } +} + +// BenchmarkMarshalDocument_Petstore measures serializing a compiled document. +// The IR's sum types and BigVal carry hand-written MarshalJSON, and every golden +// snapshot, IR diff and cache entry pays this cost, so it is worth watching +// separately from the compile that produced the document. +func BenchmarkMarshalDocument_Petstore(b *testing.B) { + doc := compilePetstore(b) + + b.ReportAllocs() + b.ResetTimer() + for range b.N { + if _, err := json.Marshal(doc); err != nil { + b.Fatalf("marshal: %v", err) + } + } +} + +// BenchmarkUnmarshalDocument_Petstore measures reading a document back. It is +// the other half of the round-trip invariant, and the half a consumer of a +// cached or piped IR document pays. +func BenchmarkUnmarshalDocument_Petstore(b *testing.B) { + encoded, err := json.Marshal(compilePetstore(b)) + require.NoError(b, err) + + b.ReportAllocs() + b.ResetTimer() + for range b.N { + var back ir.Document + if err := json.Unmarshal(encoded, &back); err != nil { + b.Fatalf("unmarshal: %v", err) + } + } +} + +// petstoreSpec reads the golden petstore, failing the benchmark rather than +// skipping it: a benchmark that quietly measures nothing is worse than one that +// stops. +func petstoreSpec(b *testing.B) []byte { + b.Helper() + data, err := os.ReadFile(goldenPetstore) + require.NoError(b, err) + require.NotEmpty(b, data) + return data +} + +// compilePetstore compiles the golden petstore once, for the benchmarks whose +// subject is what happens to a document afterwards. +func compilePetstore(b *testing.B) *ir.Document { + b.Helper() + doc, _, err := openapi.New().Compile(b.Context(), + []compilers.Source{{Path: "petstore.yaml", Data: petstoreSpec(b)}}, compilers.Options{}) + require.NoError(b, err) + require.NotNil(b, doc) + return doc +} diff --git a/compilers/openapi/internal/schema/anchorindex_bench_test.go b/compilers/openapi/internal/schema/anchorindex_bench_test.go index 6e648c7c..73a716fc 100644 --- a/compilers/openapi/internal/schema/anchorindex_bench_test.go +++ b/compilers/openapi/internal/schema/anchorindex_bench_test.go @@ -20,14 +20,19 @@ import ( // // Compare against a whole compile rather than reading the number alone — the // claim in the design is a ratio, and a ratio is what has to stay true. +// BenchmarkCompile_Petstore in compilers/openapi is the other half. func BenchmarkAnchorWalk(b *testing.B) { - data, err := os.ReadFile("../../testdata/conformance/openapi/allof-inline-merge.yaml") + // Four levels up, not two: this package sits at compilers/openapi/internal/ + // schema, and the corpus is at the repo root. A missing or unparseable + // fixture stops the benchmark rather than skipping it — a skip is silent + // without -v and exits 0, which is how the shorter path went unnoticed. + data, err := os.ReadFile("../../../../testdata/conformance/openapi/allof-inline-merge.yaml") if err != nil { - b.Skipf("corpus fixture unavailable: %v", err) + b.Fatalf("corpus fixture unavailable: %v", err) } var doc soa.OpenAPI if _, err := marshaller.Unmarshal(b.Context(), strings.NewReader(string(data)), &doc); err != nil { - b.Skipf("fixture does not parse: %v", err) + b.Fatalf("fixture does not parse: %v", err) } root := doc.GetRootNode() diff --git a/docs/micro-compiler-plan.md b/docs/micro-compiler-plan.md index 73a4ba4a..2bc9b78a 100644 --- a/docs/micro-compiler-plan.md +++ b/docs/micro-compiler-plan.md @@ -25,8 +25,8 @@ Every task inherits these. They are not restated per issue. stop-and-explain, never an `-update`. - **Coverage stays at exactly 100%.** `./scripts/check-coverage.sh` counts statements from the profile; one uncovered statement fails the build. -- **The gate, in order:** `gofmt -l`, `go vet ./...`, `golangci-lint run`, `go build ./...`, - `./scripts/check-coverage.sh`. +- **The gate is `make gate`**, which is what `.github/workflows/gate.yml` runs step by step. Read + the `Makefile` for the steps; a list here would only be a copy that goes stale. - **Every new package needs an `internal/archtest` rules entry**, or `TestImportGraph_EveryPackageIsRuledOrExempt` fails. - **Every extracted package ships table-driven unit tests** built without calling `Compile` and diff --git a/internal/harness/path_test.go b/internal/harness/path_test.go index 7f206355..1275cb6d 100644 --- a/internal/harness/path_test.go +++ b/internal/harness/path_test.go @@ -2,6 +2,7 @@ package harness_test import ( "context" + "net" "os" "path/filepath" "testing" @@ -75,19 +76,36 @@ func TestCheckPath_EmptyPathIsError(t *testing.T) { assert.Contains(t, err.Error(), "empty path") } +// TestCheckPath_UnreadableFileIsError drives CheckPath's single-file branch at a +// path that stats cleanly as a non-directory and still cannot be read. +// +// The unreadable thing is a unix socket rather than a chmod 0o000 regular file, +// and the difference is the point. Permission bits are advisory to root, so the +// permission form had to skip under euid 0 — which left CheckPath's `return nil, +// err` uncovered there, and a checkout that fails the 100% gate for anyone +// building as root, as a container commonly does. Refusing to open a socket for +// reading is not a permission check, so no euid bypasses it. func TestCheckPath_UnreadableFileIsError(t *testing.T) { t.Parallel() - if os.Geteuid() == 0 { - t.Skip("root bypasses permission bits, so a chmod 0o000 file stays readable") - } - // A regular file with no read permission stats cleanly (so it is not a - // directory) but fails to read, so CheckPath returns the read error. - dir := t.TempDir() - path := writeSpec(t, dir, "spec.yaml", testspec.Minimal) - require.NoError(t, os.Chmod(path, 0o000)) - t.Cleanup(func() { _ = os.Chmod(path, 0o600) }) + // A short prefix rather than t.TempDir: a unix socket path is capped near + // 104 bytes, and t.TempDir spells this test's whole name into it. + dir, err := os.MkdirTemp("", "harness") + require.NoError(t, err) + t.Cleanup(func() { _ = os.RemoveAll(dir) }) + + path := filepath.Join(dir, "spec.yaml") + ln, err := net.Listen("unix", path) + require.NoError(t, err) + t.Cleanup(func() { _ = ln.Close() }) + + // Establish that the fixture reaches the branch it is written for: a path + // that failed to stat, or that stat called a directory, would leave + // CheckPath before ever calling checkFile. + info, err := os.Stat(path) + require.NoError(t, err) + require.False(t, info.IsDir()) - _, err := harness.CheckPath(context.Background(), path) + _, err = harness.CheckPath(context.Background(), path) require.Error(t, err) assert.Contains(t, err.Error(), "harness: read") } diff --git a/ir/bigval_property_test.go b/ir/bigval_property_test.go index fb917b57..efcf5c27 100644 --- a/ir/bigval_property_test.go +++ b/ir/bigval_property_test.go @@ -58,12 +58,12 @@ var bigValAdversarialSeeds = []string{ // like "05") would have gone unnoticed too, had it not already been fixed by // the time this property was written. // -// What the gate runs is the seed corpus, though: `go test` executes a fuzz -// target's seeds and does not search. So the standing coverage is exactly the -// spellings below plus the two tables', and a class absent from all three is -// unprotected until someone runs `-fuzz` — which is why the seeds are chosen -// adversarially rather than drawn from real specs, the same reasoning -// naming_property_test.go records. +// The seeds still carry most of the weight, though: an ordinary `go test` +// executes them and does not search, and the gate's per-target search is +// bounded to seconds (see scripts/fuzz.sh). So the standing coverage is the +// spellings below plus the two tables', plus what a short mutation run reaches +// from them — which is why the seeds are chosen adversarially rather than drawn +// from real specs, the same reasoning naming_property_test.go records. // // A rejected input carries no claim: NewBigVal is not required to accept // everything, only to never accept something json.Valid would refuse. What diff --git a/ir/naming_property_test.go b/ir/naming_property_test.go index 7d9730b7..3482adca 100644 --- a/ir/naming_property_test.go +++ b/ir/naming_property_test.go @@ -52,12 +52,15 @@ var adversarialRunes = []string{ // output is something the rest of the IR will accept, which is a different // question and the one nothing was asking. // -// What the gate runs is the seed corpus: `go test` executes a fuzz target's seeds -// and does not search. So the standing coverage is exactly the spellings listed -// above plus the table's, and a class absent from both is unprotected until -// someone runs `-fuzz`. That is why the seeds are chosen adversarially rather -// than drawn from real specs — a grammar mishandling one script is invisible to a -// corpus that only contains Latin. +// What runs against this target is its seed corpus: `go test` executes a fuzz +// target's seeds and does not search, and the gate's bounded `-fuzz` sweep holds +// this one target back — it reaches GitHub #336 within seconds and would redden +// every unrelated change until that is fixed (scripts/fuzz.sh names it). So the +// standing coverage is exactly the spellings listed above plus the table's, and +// a class absent from both is unprotected until someone runs `-fuzz` by hand. +// That is why the seeds are chosen adversarially rather than drawn from real +// specs — a grammar mishandling one script is invisible to a corpus that only +// contains Latin. func FuzzCanonicalWords_Properties(f *testing.F) { for _, seed := range adversarialRunes { f.Add(seed) diff --git a/pass/validate_bench_test.go b/pass/validate_bench_test.go new file mode 100644 index 00000000..2f26cae8 --- /dev/null +++ b/pass/validate_bench_test.go @@ -0,0 +1,54 @@ +package pass_test // external test package — imports across layers is legal in tests + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/dexpace/morphic/compilers" + "github.com/dexpace/morphic/compilers/openapi" + "github.com/dexpace/morphic/ir" + "github.com/dexpace/morphic/pass" +) + +// goldenPetstore is the larger real-ish spec the golden snapshot is taken from, +// addressed relative to this test file. +const goldenPetstore = "../testdata/golden/openapi/petstore.yaml" + +// BenchmarkValidate_Petstore measures a referential-integrity pass over a +// compiled document. Validate walks every reference in the document, so its cost +// tracks document size rather than spec size, and it runs on every compile the +// engine drives — a regression here is paid by every consumer. +func BenchmarkValidate_Petstore(b *testing.B) { + doc := compilePetstore(b) + + var diags []ir.Diagnostic + b.ReportAllocs() + b.ResetTimer() + for range b.N { + diags = pass.Validate(doc) + } + b.StopTimer() + + // Measure the clean path, and say so: a document Validate rejects would take + // different branches and the number would not mean what the name claims. + for _, d := range diags { + require.NotEqual(b, ir.SeverityError, d.Severity, "unexpected validate error: %+v", d) + } +} + +// compilePetstore compiles the golden petstore once, so the benchmark measures +// Validate rather than the compile that feeds it. +func compilePetstore(b *testing.B) *ir.Document { + b.Helper() + data, err := os.ReadFile(goldenPetstore) + require.NoError(b, err) + require.NotEmpty(b, data) + + doc, _, err := openapi.New().Compile(b.Context(), + []compilers.Source{{Path: "petstore.yaml", Data: data}}, compilers.Options{}) + require.NoError(b, err) + require.NotNil(b, doc) + return doc +} diff --git a/scripts/bench.sh b/scripts/bench.sh new file mode 100755 index 00000000..f494c65a --- /dev/null +++ b/scripts/bench.sh @@ -0,0 +1,54 @@ +#!/usr/bin/env bash +# +# bench.sh runs every benchmark in the module. +# +# With no argument it measures — that is `make bench`, and the numbers are worth +# reading. With `1x` it runs each benchmark exactly once, which is what the gate +# does: one iteration says nothing about speed and everything about whether the +# benchmarks still build, still find their fixtures, and still complete. +# +# The gate needs that because `go test -bench` reports success for a run that +# measured nothing at all. A benchmark whose fixture path is wrong prints a +# SKIP under -v and nothing without it, and exits 0 either way — which is how +# this module's only benchmark went unexecuted while looking green. +# +# Usage: bench.sh [benchtime] +set -euo pipefail + +cd "$(git -C "$(dirname "${BASH_SOURCE[0]}")" rev-parse --show-toplevel)" + +benchtime="${1:-}" + +# -run '^$' selects no tests: the suite is the coverage step's job, and running +# it again here would double the gate's wall time for nothing. +args=(test ./... -run '^$' -bench . -benchmem -v) +if [ -n "$benchtime" ]; then + args+=("-benchtime=$benchtime") +fi + +# Stream the run and keep a copy: `make bench` is minutes of output somebody is +# watching, and the checks below need the whole of it. +log="$(mktemp "${TMPDIR:-/tmp}/morphic-bench.XXXXXX")" +trap 'rm -f "$log"' EXIT + +set +e +go "${args[@]}" 2>&1 | tee "$log" +status=${PIPESTATUS[0]} +set -e +if [ "$status" -ne 0 ]; then + exit "$status" +fi + +skipped="$(grep '^--- SKIP: Benchmark' "$log" || true)" +if [ -n "$skipped" ]; then + echo "bench.sh: these benchmarks skipped, so they measured nothing:" >&2 + printf '%s\n' "$skipped" >&2 + exit 1 +fi + +# A tree with no benchmarks left, or a -bench pattern that stopped matching, +# reads exactly like a clean run. Require evidence that one actually reported. +if ! grep -q 'ns/op' "$log"; then + echo "bench.sh: no benchmark reported a result; this run proved nothing" >&2 + exit 1 +fi diff --git a/scripts/check-coverage.sh b/scripts/check-coverage.sh index b21ea9f4..560d81ac 100755 --- a/scripts/check-coverage.sh +++ b/scripts/check-coverage.sh @@ -1,6 +1,7 @@ #!/usr/bin/env bash # -# check-coverage.sh enforces exact 100% statement coverage. +# check-coverage.sh runs the whole suite under the race detector and enforces +# exact 100% statement coverage. # # Coverage is counted from the profile, statement by statement, rather than read # from go test's "coverage: N%" summary lines. Those are rounded to one decimal @@ -11,18 +12,45 @@ # profile blocks, so they pass without a special case. set -euo pipefail -cover_file="${COVER_FILE:-cover.out}" +# Work from the repo root, derived from this script's own location rather than +# from the caller's cwd. Invoked from a subdirectory it would otherwise measure +# only that subtree and still report a pass. Same derivation as +# verify-atomic-output.sh. +cd "$(git -C "$(dirname "${BASH_SOURCE[0]}")" rev-parse --show-toplevel)" + +# The profile path is unique per invocation. `go test -coverprofile` truncates +# the file when it starts and appends each package's blocks as that package +# finishes, so two runs sharing one path interleave into a profile that is +# neither run's, with every block counted once per run that reached it. That can +# never fail a fully covered tree — hit and total inflate together — but the +# total a human reads to judge the gate is then several times the real one. +# +# COVER_FILE names a profile to keep for inspection; a path this script mints is +# its own and is removed on exit. +if [ -n "${COVER_FILE:-}" ]; then + cover_file="$COVER_FILE" +else + cover_file="$(mktemp "${TMPDIR:-/tmp}/morphic-cover.XXXXXX")" + trap 'rm -f "$cover_file"' EXIT +fi # Bound the failure output: a broken build can leave hundreds of uncovered # blocks, and the first screenful is what gets read. max_reported=25 +# -race rides along with the coverage run rather than getting a `go test ./...` +# of its own. The two want the same execution of the same tests, and running the +# suite twice to collect two properties from it costs a second full run for +# nothing: the profile is identical with the detector on and off. +# # -timeout is explicit rather than left to go test's 10-minute default. The # compiler refuses documents that would otherwise hang the third-party resolver, # so a regression in that refusal is a test that never returns, not one that -# fails. Ninety seconds is several times the suite's normal wall time and turns -# that failure mode into a prompt stack dump naming the stuck goroutine. -go test ./... -timeout 90s -covermode=atomic -coverprofile="$cover_file" +# fails. The bound clears the slowest package under -race by more than an order +# of magnitude, which leaves room for a CI runner several times slower than a +# developer machine while still turning a hang into a prompt stack dump naming +# the stuck goroutine. +go test ./... -race -timeout 300s -covermode=atomic -coverprofile="$cover_file" # Profile body, one block per line: "/.go: ". # Sorted so the same failure reads the same way on every run. diff --git a/scripts/fuzz.sh b/scripts/fuzz.sh new file mode 100755 index 00000000..bd0a110a --- /dev/null +++ b/scripts/fuzz.sh @@ -0,0 +1,90 @@ +#!/usr/bin/env bash +# +# fuzz.sh runs the module's fuzz targets under a bounded search. +# +# `go test` executes a target's seed corpus on every ordinary run but never +# mutates; only -fuzz searches, and -fuzz takes one package and one target per +# invocation. So the gate needs a loop, and the loop derives its pairs from the +# source: a target written tomorrow is fuzzed the moment it lands, and there is +# no list here to go stale. +# +# Usage: fuzz.sh [fuzztime] (default 10s per target, or $FUZZTIME) +set -euo pipefail + +cd "$(git -C "$(dirname "${BASH_SOURCE[0]}")" rev-parse --show-toplevel)" + +fuzztime="${1:-${FUZZTIME:-10s}}" + +# Targets held out of the search, each naming the issue that must close before +# it comes back. A search is not the place to rediscover a bug that is already +# filed: it spends its whole budget re-finding one input and reddens every +# unrelated change until the fix lands. +# +# FuzzCanonicalWords_Properties — dexpace/morphic#336, CanonicalWords is not +# idempotent for "ℤℤA". Reached within seconds of mutation from the committed +# seeds, so the target has nothing else to report until #336 is fixed. +# +# Ordinary `go test` still runs every one of these targets' seeds, quarantined +# or not; what is held back is the mutation. +quarantined="FuzzCanonicalWords_Properties" + +# The gate's fuzz budget is targets x fuzztime, so the target count is what +# bounds its wall time. A cap makes that bound explicit: crossing it is a +# deliberate decision about how long every CI run should take, not something a +# new target does by accident. +readonly max_targets=16 + +# Minimizing a found crash is bounded too. Left at its 60s default it would +# dominate the run on the one occasion it matters, and the reproducer is written +# out either way. +readonly minimize_time=10s + +found=0 +fuzzed=0 +seen="" +while IFS=: read -r file _ decl; do + target="${decl#func }" + target="${target%%(*}" + pkg="./$(dirname "$file")" + + found=$((found + 1)) + seen="$seen $target" + if [ "$found" -gt "$max_targets" ]; then + echo "fuzz.sh: more than $max_targets fuzz targets; raise max_targets deliberately" >&2 + exit 1 + fi + + case " $quarantined " in + *" $target "*) + printf '=== skip %s (quarantined — see scripts/fuzz.sh)\n' "$target" + continue + ;; + esac + + printf '=== fuzz %s (%s) for %s\n' "$target" "$pkg" "$fuzztime" + go test "$pkg" -run '^$' -fuzz "^${target}\$" \ + -fuzztime="$fuzztime" -fuzzminimizetime="$minimize_time" + fuzzed=$((fuzzed + 1)) +done < <(git grep -n '^func Fuzz' -- '*_test.go') + +# A sweep that matched nothing exits 0 from an empty loop and reads exactly like +# a clean run. Refuse to report one: either the grammar above stopped matching +# the declarations, or the targets are gone. +if [ "$fuzzed" -eq 0 ]; then + echo "fuzz.sh: no fuzz target was searched" >&2 + exit 1 +fi + +# A quarantine entry naming a target that no longer exists silently holds back +# nothing, and reads as if it still does. +for held in $quarantined; do + case " $seen " in + *" $held "*) ;; + *) + echo "fuzz.sh: quarantine names $held, which is not a fuzz target; remove it" >&2 + exit 1 + ;; + esac +done + +printf 'fuzzed %d of %d target(s), %s each\n' "$fuzzed" "$found" "$fuzztime" From 936c352ab6626486b2b31eb11f09803adc02199f Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 13 Aug 2026 21:17:12 +0300 Subject: [PATCH 2/7] build: hold the schema fuzz target back until its bug is fixed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bounded search finds a real compiler defect in about a second from a cleared corpus: an anyOf whose only branch is {"type":"null"} lowers to a union that declares no variants, which irverify rejects and `morphic compile` reports nothing about. It reproduces on main through the CLI and the harness, so it is not this change's to fix, but a gate step that reddens on every run until it is would keep every unrelated PR red. Filed as #416 and quarantined the same way the search's other known finding was. FuzzCanonicalWords_Properties comes out of the quarantine. It was held back for #336, which is closed: CanonicalWords("ℤℤA") is idempotent now, and a 30s search of that target from a cleared corpus runs 2.5M executions and reports nothing. A quarantine that outlives its bug reads as though it were still protecting something while it quietly stops a target from ever running, so the comment now says plainly that closing the issue is what retires the entry — nothing in the script can check that the reason still holds. --- ir/naming_property_test.go | 16 +++++++--------- scripts/fuzz.sh | 15 +++++++++++---- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/ir/naming_property_test.go b/ir/naming_property_test.go index 3c16576e..a40df077 100644 --- a/ir/naming_property_test.go +++ b/ir/naming_property_test.go @@ -57,15 +57,13 @@ var adversarialRunes = []string{ // output is something the rest of the IR will accept, which is a different // question and the one nothing was asking. // -// What runs against this target is its seed corpus: `go test` executes a fuzz -// target's seeds and does not search, and the gate's bounded `-fuzz` sweep holds -// this one target back — it reaches GitHub #336 within seconds and would redden -// every unrelated change until that is fixed (scripts/fuzz.sh names it). So the -// standing coverage is exactly the spellings listed above plus the table's, and -// a class absent from both is unprotected until someone runs `-fuzz` by hand. -// That is why the seeds are chosen adversarially rather than drawn from real -// specs — a grammar mishandling one script is invisible to a corpus that only -// contains Latin. +// The seeds still carry most of the weight: an ordinary `go test` executes them +// and does not search, and the gate's per-target search is bounded to seconds +// (see scripts/fuzz.sh). So the standing coverage is the spellings listed above +// plus the table's, plus what a short mutation run reaches from them. That is +// why the seeds are chosen adversarially rather than drawn from real specs — a +// grammar mishandling one script is invisible to a corpus that only contains +// Latin. func FuzzCanonicalWords_Properties(f *testing.F) { for _, seed := range adversarialRunes { f.Add(seed) diff --git a/scripts/fuzz.sh b/scripts/fuzz.sh index bd0a110a..8085a1ba 100755 --- a/scripts/fuzz.sh +++ b/scripts/fuzz.sh @@ -20,13 +20,20 @@ fuzztime="${1:-${FUZZTIME:-10s}}" # filed: it spends its whole budget re-finding one input and reddens every # unrelated change until the fix lands. # -# FuzzCanonicalWords_Properties — dexpace/morphic#336, CanonicalWords is not -# idempotent for "ℤℤA". Reached within seconds of mutation from the committed -# seeds, so the target has nothing else to report until #336 is fixed. +# FuzzLowerSchema — dexpace/morphic#416, an anyOf whose only branch is +# {"type":"null"} lowers to a union with no variants, which irverify rejects. +# Minimizes to {"anyOf":[{"type":"null"}]} and is reached in about a second +# from a cleared corpus, so the target has nothing else to report until #416 +# is fixed. +# +# Closing the issue is what retires the entry: nothing here can check that the +# reason still holds, and a quarantine that outlives its bug reads as if it were +# still protecting something while it quietly stops a target from ever running. +# When an issue named above closes, delete its line and let the search prove it. # # Ordinary `go test` still runs every one of these targets' seeds, quarantined # or not; what is held back is the mutation. -quarantined="FuzzCanonicalWords_Properties" +quarantined="FuzzLowerSchema" # The gate's fuzz budget is targets x fuzztime, so the target count is what # bounds its wall time. A cap makes that bound explicit: crossing it is a From 70458cf1373d5866a99950f3eeb42bfc98ea1e08 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 13 Aug 2026 21:17:12 +0300 Subject: [PATCH 3/7] build: run the gate one check at a time under make -j Prerequisites of a single target are eligible to run concurrently under -j, so `make -j gate` both lost the CI order this file exists to mirror and let `fuzz` write a reproducer into testdata/ while `coverage` was running `go test ./...` over the same tree. --- Makefile | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Makefile b/Makefile index e83f41dd..e744502b 100644 --- a/Makefile +++ b/Makefile @@ -25,6 +25,12 @@ FUZZTIME ?= 10s .DEFAULT_GOAL := gate +# The gate runs one check at a time even under `make -j`. Prerequisites of a +# single target are otherwise eligible to run concurrently, which would both lose +# the CI order this file exists to mirror and let `fuzz` write a reproducer into +# testdata/ while `coverage` is running `go test ./...` over the same tree. +.NOTPARALLEL: + .PHONY: gate fmt vet lint nolint-grammar nolint build coverage-count coverage \ fuzz bench bench-smoke print-lint-version From f04629029a0c013690e9e9b643ea5fa50eadb564 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 13 Aug 2026 21:17:12 +0300 Subject: [PATCH 4/7] docs: point README's build section at the gate it now runs The block introduced itself as the checks CI runs and listed nine commands, none of them the make targets the workflow now calls, plus a `go test ./...` step the gate does not have and a per-package coverage claim the script does not make. It becomes the same one-command pointer CLAUDE.md and micro-compiler-plan.md carry. Departs from the note in this branch that left README to #64/#65: those cover the wider drift, but this change is what makes this particular block false, so it does not get to leave it that way. Closes #415. --- README.md | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 12f383f4..c74c1b2b 100644 --- a/README.md +++ b/README.md @@ -221,21 +221,17 @@ The design documents are normative — read them before proposing changes to the ## Building -Standard Go tooling. These are the same checks the CI `gate` runs, and all must pass before a -change lands: +One command, and it must pass before a change lands: ```bash -gofmt -l . # must print nothing -go vet ./... -golangci-lint run -./scripts/verify-nolint-grammar.sh # checks how the check below reads a directive -./scripts/check-nolint-linters.sh # every //nolint names a linter that is enabled -go build ./... -go test ./... -./scripts/verify-coverage-count.sh # checks how the gate below counts a profile -./scripts/check-coverage.sh # enforces 100% statement coverage, overall and per package +make gate ``` +That is not a summary of CI — it is what CI runs. Every check in `.github/workflows/gate.yml` runs +a `Makefile` target, so the local command and the job are the same commands in the same order. Read +the `Makefile` for the step list rather than a copy here; `make coverage`, `make fuzz`, `make bench` +and the rest are individually runnable while iterating. + Run a single test with `go test ./ir -run TestName`. Golden IR snapshots are regenerated with the corpus test's `-update` flag after an intentional change. From a53fed9ec87aec3d1cf48152e9b1d72194d47105 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 13 Aug 2026 22:05:27 +0300 Subject: [PATCH 5/7] docs(ci): say the gate keeps CI order, not just its commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The header claimed the same commands and stopped there, which was the weaker half: .NOTPARALLEL is what makes the sequence match too. It also called lint the one step not running a Makefile target, while the step that reads the pin back runs one — it is the one *check* that does not. --- .github/workflows/gate.yml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/gate.yml b/.github/workflows/gate.yml index 5930ef14..c425be59 100644 --- a/.github/workflows/gate.yml +++ b/.github/workflows/gate.yml @@ -4,10 +4,12 @@ on: branches: [main] pull_request: # Every check below runs a Makefile target, so `make gate` on a developer machine -# is these same commands. Lint is the one step that does not: the action installs -# the pinned release and runs it, reading the pin back from the Makefile so the -# version still has a single definition. Add a check to the Makefile and give it -# a step here; do not spell a command out in this file. +# is these same commands in the same order — .NOTPARALLEL in the Makefile keeps +# that true under `make -j`. Lint is the one check that does not run a target: +# the action installs the pinned release and runs it, and the step above reads +# that pin back from the Makefile so the version still has a single definition. +# Add a check to the Makefile and give it a step here; do not spell a command out +# in this file. jobs: gate: runs-on: ubuntu-latest From aacc4692cba0b1b78b2a08ff2598895020b9671f Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 13 Aug 2026 22:07:39 +0300 Subject: [PATCH 6/7] docs(scripts): correct what a shared profile path costs now The comment said two concurrent runs inflate the total several times over. That was true when it was written and is not now: the block merge landed in #381 counts a block once however many times it appears, and five staggered runs against a shared cover.out all report the same 6132 on this tree. What the unique path still buys is that a run judges the blocks it produced. Two runs sharing one path truncate each other mid-write, and a profile missing the blocks another run had already written reads as a pass when those were the uncovered ones. --- scripts/check-coverage.sh | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/scripts/check-coverage.sh b/scripts/check-coverage.sh index 2b96ff15..551de744 100755 --- a/scripts/check-coverage.sh +++ b/scripts/check-coverage.sh @@ -46,10 +46,13 @@ if [ "$#" -eq 1 ]; then else # The minted path is unique per invocation. `go test -coverprofile` truncates # the file when it starts and appends each package's blocks as that package - # finishes, so two runs sharing one path interleave into a profile that is - # neither run's, with every block counted once per run that reached it. That - # can never fail a fully covered tree — hit and total inflate together — but - # the total a human reads to judge the gate is then several times the real one. + # finishes, so two runs sharing one path overwrite each other mid-write and + # each ends up reading a profile that is partly the other's. + # + # The merge below counts a block once however many times it appears, so this + # no longer inflates the total the way it did before that merge landed. What + # is left is a run judging a set of blocks that is not the set it produced, + # which reads as a pass whenever the blocks it lost were the uncovered ones. # # COVER_FILE names a profile to keep for inspection; a path this script mints # is its own and is removed on exit. From 6c7accae2c5da0e13c3fe3b5727afa307037f436 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 13 Aug 2026 22:19:13 +0300 Subject: [PATCH 7/7] docs: name the one gate check that is not a make target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both files said every check in the workflow runs a Makefile target. `lint` does not — the golangci-lint action runs it, at the version the Makefile pins, which is the arrangement gate.yml already spells out and these two flattened into a universal. They also disagreed about whether the order matched; it does, and both now say so. --- CLAUDE.md | 7 ++++--- README.md | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index bf90430f..6eac75d4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -244,9 +244,10 @@ make gate ``` That is not a summary of CI — it is what CI runs. Every check in `.github/workflows/gate.yml` runs a -`Makefile` target, so the local command and the job are the same commands by construction. Read the -`Makefile` for the step list rather than restating it here; `make coverage`, `make fuzz`, `make -bench` and the rest are individually runnable while iterating. +`Makefile` target bar `lint`, which the golangci-lint action runs at the version the `Makefile` +pins, so the local command and the job are the same commands in the same order. Read the `Makefile` +for the step list rather than restating it here; `make coverage`, `make fuzz`, `make bench` and the +rest are individually runnable while iterating. **Coverage is a gate at exactly 100%, not a target.** `scripts/check-coverage.sh` counts statements from the profile rather than reading `go test`'s rounded percentage, so one uncovered diff --git a/README.md b/README.md index c74c1b2b..dda2ab72 100644 --- a/README.md +++ b/README.md @@ -228,9 +228,10 @@ make gate ``` That is not a summary of CI — it is what CI runs. Every check in `.github/workflows/gate.yml` runs -a `Makefile` target, so the local command and the job are the same commands in the same order. Read -the `Makefile` for the step list rather than a copy here; `make coverage`, `make fuzz`, `make bench` -and the rest are individually runnable while iterating. +a `Makefile` target bar `lint`, which the golangci-lint action runs at the version the `Makefile` +pins, so the local command and the job are the same commands in the same order. Read the `Makefile` +for the step list rather than a copy here; `make coverage`, `make fuzz`, `make bench` and the rest +are individually runnable while iterating. Run a single test with `go test ./ir -run TestName`. Golden IR snapshots are regenerated with the corpus test's `-update` flag after an intentional change.