From 68310fbaac211d88cf5ad34c79ba293167820790 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 04:05:10 +0300 Subject: [PATCH] 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"