diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 37a9e68ece..12f2a78f50 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,7 +17,7 @@ name: PR gate # `make test-laws` finds every one of them without anybody keeping a list. # # The whole suite is not here. The packages a change touched run in full in the -# second job below, on every pull request, and `check` — the one name a person +# concurrent touched legs below, on every pull request, and `check` — the one name a person # and a ruleset look at — is green only when both are; everything else runs on # the way into `staging`, and nightly against `dev` — see ci-full.yml. # docs/rules/ci.md says why the line is drawn in that place. @@ -70,9 +70,33 @@ jobs: # which a shallow clone does not contain. fetch-depth: 0 - uses: actions/setup-go@v5 + id: go with: go-version-file: go.mod - cache: true + cache: false + + # UTC-day keys refresh each namespace on its first dev push of the day. + # PRs only restore; the undated prefix finds the newest compatible entry. + - name: Cache paths + id: cache-paths + run: | + echo "build=$(go env GOCACHE)" >> "$GITHUB_OUTPUT" + echo "modules=$(go env GOMODCACHE)" >> "$GITHUB_OUTPUT" + echo "today=$(date -u +%Y-%m-%d)" >> "$GITHUB_OUTPUT" + - uses: actions/cache/restore@v4 + id: build-cache + with: + path: ${{ steps.cache-paths.outputs.build }} + key: codeaf-go-v1-light-build-${{ runner.os }}-${{ runner.arch }}-${{ steps.go.outputs.go-version }}-${{ steps.cache-paths.outputs.today }} + restore-keys: | + codeaf-go-v1-light-build-${{ runner.os }}-${{ runner.arch }}-${{ steps.go.outputs.go-version }}- + - uses: actions/cache/restore@v4 + id: module-cache + with: + path: ${{ steps.cache-paths.outputs.modules }} + key: codeaf-go-v1-modules-${{ runner.os }}-${{ runner.arch }}-${{ steps.go.outputs.go-version }}-${{ hashFiles('go.sum') }} + restore-keys: | + codeaf-go-v1-modules-${{ runner.os }}-${{ runner.arch }}-${{ steps.go.outputs.go-version }}- # Several sessions work this tree at once and a half-finished file breaks # the build for everyone. This is the cheapest way to find that out, and @@ -184,33 +208,31 @@ jobs: mkdir -p "$TMPDIR" make test-laws - # THE PACKAGES THIS CHANGE TOUCHED, IN FULL. The laws catch a shape; this - # catches a behaviour, in the one place a change can have broken it. It is a - # job of its own so that the light gate's answer arrives in its few minutes - # while this one takes as long as the slowest touched package — internal/tui3 - # is about eight minutes on this runner — and the two run side by side rather - # than one after the other. - # - # It reads the same ledger and the same timeout as `make test`, through - # `make test`, so a red here is a red on a clean laptop too. AND IT BLOCKS. - # It was neutral for a day and then, for a day, off pull requests altogether - # (#499); in that day #523 merged red on cmd/codeaf with `check` green, the - # way #437, #439 and #483 had before the job existed. A red that reaches - # nobody before the merge is the whole story of this file, so the owner's - # ruling is that this runs on every pull request and `check` needs it. - # THE JOB'S OWN CEILING SITS ABOVE THE GO ONE. `make test` gives each - # package fifteen minutes so a hang panics with the package's name; a job - # killed by GitHub first names nothing. A wide change can touch the three - # heaviest packages at once, so this is three of those with room. - # - # - # A change with no Go file and no module file in it — docs, a bench record, - # a manual page — runs nothing here and is green in a minute. A change to - # go.mod or go.sum touches every package and runs the whole tree. - touched: - name: touched packages + # THE CACHE BUDGET IS DAILY, NOT PER COMMIT. At most five build + # namespaces a day plus one module entry per go.sum cost about 2 GB a day + # at the measured ~385 MB. GitHub's least-recently-used eviction at the + # 10 GB repository limit removes old days without a pruning script. + # Only dev pushes that miss the exact key save; PRs never save. + - uses: actions/cache/save@v4 + if: always() && github.event_name == 'push' && github.ref == 'refs/heads/dev' && steps.cache-paths.outcome == 'success' && steps.build-cache.outputs.cache-hit != 'true' + with: + path: ${{ steps.cache-paths.outputs.build }} + key: ${{ steps.build-cache.outputs.cache-primary-key }} + - uses: actions/cache/save@v4 + if: always() && github.event_name == 'push' && github.ref == 'refs/heads/dev' && steps.cache-paths.outcome == 'success' && steps.module-cache.outputs.cache-hit != 'true' + with: + path: ${{ steps.cache-paths.outputs.modules }} + key: ${{ steps.module-cache.outputs.cache-primary-key }} + + # The selector does no compilation unless module files changed. A docs-only + # change creates no test runner; the light gate and the aggregate still run. + select: + name: select touched packages runs-on: ubuntu-latest - timeout-minutes: 60 + outputs: + matrix: ${{ steps.select.outputs.matrix }} + has-tests: ${{ steps.select.outputs.has-tests }} + base: ${{ steps.select.outputs.base }} steps: - uses: actions/checkout@v4 with: @@ -218,64 +240,136 @@ jobs: - uses: actions/setup-go@v5 with: go-version-file: go.mod - cache: true - - name: Test the packages this change touched + cache: false + - name: Select and partition packages + id: select env: - # A pull request diffs against its base; a push against what the - # branch was before it. A first push has no before, and diffs one - # commit. BASE: ${{ github.event.pull_request.base.sha || github.event.before }} + run: | + set -euo pipefail + case "${BASE:-}" in ''|0000000000000000000000000000000000000000) BASE="$(git rev-parse HEAD~1)";; esac + export BASE + echo "base=$BASE" >> "$GITHUB_OUTPUT" + ./scripts/touched-packages.sh | python3 scripts/touched-matrix.py >> "$GITHUB_OUTPUT" + + # THE LEGS START TOGETHER ON SEPARATE RUNNERS. A failure never cancels another + # leg's evidence. Five named failures at most receive one focused retry each + # and then a base probe; unnamed failures fail immediately. No test is skipped. + touched-leg: + name: touched ${{ matrix.leg }} + needs: select + if: needs.select.outputs.has-tests == 'true' + runs-on: ubuntu-latest + timeout-minutes: 60 + strategy: + fail-fast: false + matrix: ${{ fromJSON(needs.select.outputs.matrix) }} + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - uses: actions/setup-go@v5 + id: go + with: + go-version-file: go.mod + cache: false + + # UTC-day keys refresh each namespace on its first dev push of the day. + # PRs only restore; the undated prefix finds the newest compatible entry. + - name: Cache paths + id: cache-paths + run: | + echo "build=$(go env GOCACHE)" >> "$GITHUB_OUTPUT" + echo "modules=$(go env GOMODCACHE)" >> "$GITHUB_OUTPUT" + echo "today=$(date -u +%Y-%m-%d)" >> "$GITHUB_OUTPUT" + - uses: actions/cache/restore@v4 + id: build-cache + with: + path: ${{ steps.cache-paths.outputs.build }} + key: codeaf-go-v1-${{ matrix.leg }}-build-${{ runner.os }}-${{ runner.arch }}-${{ steps.go.outputs.go-version }}-${{ steps.cache-paths.outputs.today }} + restore-keys: | + codeaf-go-v1-${{ matrix.leg }}-build-${{ runner.os }}-${{ runner.arch }}-${{ steps.go.outputs.go-version }}- + codeaf-go-v1-light-build-${{ runner.os }}-${{ runner.arch }}-${{ steps.go.outputs.go-version }}- + - uses: actions/cache/restore@v4 + id: module-cache + with: + path: ${{ steps.cache-paths.outputs.modules }} + key: codeaf-go-v1-modules-${{ runner.os }}-${{ runner.arch }}-${{ steps.go.outputs.go-version }}-${{ hashFiles('go.sum') }} + restore-keys: | + codeaf-go-v1-modules-${{ runner.os }}-${{ runner.arch }}-${{ steps.go.outputs.go-version }}- + + - name: Runner capacity + run: | + nproc + free -g + # Offline module-listing tests need the whole module cache; each leg compiles only part of the tree. + - name: Download modules + run: go mod download + - name: Test and attribute failures + env: + BASE: ${{ needs.select.outputs.base }} + PACKAGES: ${{ matrix.packages }} + LEG: ${{ matrix.leg }} TMPDIR: /tmp/codeaf-ci run: | set -euo pipefail mkdir -p "$TMPDIR" - case "${BASE:-}" in ''|0000000000000000000000000000000000000000) BASE="$(git rev-parse HEAD~1)";; esac - changed="$(git diff --name-only "$BASE" HEAD -- '*.go' go.mod go.sum)" - if printf '%s\n' "$changed" | grep -qxE 'go\.(mod|sum)'; then - # A module change reaches every package; the tree is the touched set. - pkgs="./..." - else - # `|| true`: a change with no Go file makes this grep exit 1, and - # under pipefail that would turn a docs pull request red. - dirs="$(printf '%s\n' "$changed" | { grep '\.go$' || true; } | xargs -r -n1 dirname | sort -u)" - pkgs="" - for dir in $dirs; do - # A DIRECTORY UNDER ITS OWN go.mod IS ANOTHER MODULE, not a package - # of this one, and `go test ./that/dir` from the root answers - # "main module does not contain package" and fails the job as a - # setup error. The bench fixtures under bench/bashloop/fixtures - # are exactly that: small programs the task door edits, each with - # a go.mod of its own. Walking up from the directory to the first - # go.mod is what tells the two apart. This walk is the same one - # the Makefile's test-touched target does, and the two must keep - # deriving the same set or `make pr-ready` stops predicting CI. - nested=0; walk="$dir" - while [ "$walk" != "." ] && [ "$walk" != "/" ]; do - if [ -f "$walk/go.mod" ]; then nested=1; break; fi - walk="$(dirname "$walk")" - done - if [ "$nested" = 1 ]; then continue; fi - # A directory the change emptied has no package left to test. - if ls "$dir"/*.go >/dev/null 2>&1; then pkgs="$pkgs ./$dir"; fi - done - fi - if [ -z "$pkgs" ]; then - echo 'No Go file and no module file changed; nothing to run.' - exit 0 - fi - echo "touched:$pkgs" - # -p 1 for the reason ci-full.yml gives: this runner has been shut - # down under the link load of the heavy binaries twice, and a change - # touching tui3, session and cmd/codeaf links all three at once. - # -count=1 so a cached pass never stands in for a run. - make test TEST_FLAGS='-count=1 -p 1' PKGS="$pkgs" + # -p 2 permits concurrent compilation/testing in rest on the public + # 4-vCPU / 16-GB runner, leaving headroom for the linker. tui3 and + # session use four shards; codeaf runs its suite without sharding. + ./scripts/touched-verdict.sh run --base "$BASE" --shards 4 --report "$TMPDIR/$LEG.json" $PACKAGES + - uses: actions/upload-artifact@v4 + if: always() + with: + name: touched-verdict-${{ matrix.leg }} + path: /tmp/codeaf-ci/${{ matrix.leg }}.json + retention-days: 7 + - uses: actions/cache/save@v4 + if: always() && github.event_name == 'push' && github.ref == 'refs/heads/dev' && steps.cache-paths.outcome == 'success' && steps.build-cache.outputs.cache-hit != 'true' + with: + path: ${{ steps.cache-paths.outputs.build }} + key: ${{ steps.build-cache.outputs.cache-primary-key }} + + # KEEP THIS NAME. Consumers of `touched packages` see the aggregate even when + # no leg was needed; selector failures and failed/cancelled legs remain red. + touched: + name: touched packages + needs: [select, touched-leg] + if: always() + runs-on: ubuntu-latest + permissions: + contents: read + issues: write + steps: + - uses: actions/checkout@v4 + - uses: actions/download-artifact@v4 + if: needs.select.outputs.has-tests == 'true' + continue-on-error: true + with: + pattern: touched-verdict-* + merge-multiple: true + path: /tmp/codeaf-verdicts + - name: Record flaky and inherited failures + if: always() + continue-on-error: true + env: + GH_TOKEN: ${{ github.token }} + TOUCHED_REPORT_ISSUE: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }} + run: ./scripts/touched-verdict.sh report-issues /tmp/codeaf-verdicts + - name: Every needed leg passed + if: always() + env: + SELECT: ${{ needs.select.result }} + HAS_TESTS: ${{ needs.select.outputs.has-tests }} + LEGS: ${{ needs.touched-leg.result }} + run: | + set -euo pipefail + echo "selection: $SELECT; needed: $HAS_TESTS; touched legs: $LEGS" + [ "$SELECT" = success ] + if [ "$HAS_TESTS" = true ]; then [ "$LEGS" = success ]; else [ "$LEGS" = skipped ]; fi - # ONE NAME FOR THE TWO. The name of this job is the name of the required - # status check in .github/rulesets/dev.json and the name every landing script - # and every person reads. Renaming it silently un-protects the branch, so the - # two move together or not at all. It is green only when the light gate and - # the touched packages both are — the same shape `full tests` and `cross - # build` use in ci-full.yml for a matrix a rule cannot spell. + # The single required name stays check. Rulesets and landing scripts need + # only this result, which requires both the light gate and the aggregate. check: name: check needs: [light, touched] diff --git a/CLAUDE.md b/CLAUDE.md index 7e20d64430..26a544aea1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -106,15 +106,14 @@ now. `docs/rules/changelog.md` says why it cannot be generated from the diff. Read on demand, not up front: [docs/rules/branching.md](docs/rules/branching.md) for the model and why promotion is a fast-forward, -[docs/rules/ci.md](docs/rules/ci.md) for what runs where and the known-red ledger -in `.github/known-red.txt`, [docs/rules/changelog.md](docs/rules/changelog.md) +[docs/rules/ci.md](docs/rules/ci.md) for what runs where and how a red is +attributed, [docs/rules/changelog.md](docs/rules/changelog.md) for what an entry carries, [docs/rules/promotion.md](docs/rules/promotion.md) for the promote-and-release runbook. -Server enforcement depends on the repository's visibility or plan — the org is -on the free plan, and a private repository gets no branch rules there. Until the -org moves to GitHub Team or the repository is public, every line above is -convention. `.github/rulesets/` holds the rules ready to apply. +The repository is public now, so the free plan's former private-repository +restriction no longer prevents branch rules. `.github/rulesets/` holds the +intended rules; inspect live enforcement before assuming they were applied. ## Build and ship — the owner's standing orders @@ -305,18 +304,35 @@ packages after an abrupt end, and sorts completed tests slowest-first; a cut run still writes that report and still exits non-zero. The quick target checks build, vet, formatting, the packed manual, well-formed change entries, the manual gates, and laws; it does not replace acceptance. `make test-touched` derives the same -package set as the pull-request gate and runs it through the known-red ledger with `-count=1`; -`make pr-ready` combines that proof with the light gate. Pass `BASE=` +package set as CI through `scripts/touched-packages.sh` and runs the same +failure classifier with `-count=1`; `make pr-ready` combines that proof with the +light gate and touched-only tooling acceptance when scripts/, the Makefile or +covered benchmark paths changed. Pass `BASE=` when the comparison should not be `origin/dev`. The target refuses uncommitted Go or module files: commit the candidate first so the local diff is exactly the diff CI will test, without absorbing another session's edits. -**The tests that fail on a clean tree are listed in `.github/known-red.txt` and -nowhere else.** `make test` skips them by name, and so does CI, through the same -target — so `make check` passes on a clean tree and a red in either place means -the change caused it. The ledger only shrinks (`internal/ci` ratchets its count): -fix a test, delete its line, lower `knownRedEntries` in the same commit. Never -add a line. Confirm any other red with a stash-and-rerun before chasing it. +**`touched packages` checks a failing test again before it blames your change.** +A classifier reads the plain output of the same `make test` run everyone else +gets; initial suites, head retries and base probes all avoid `-json`. Go 1.26 +replaces `os.Stderr` under that flag, and re-executed helper binaries inherit +`-test.v=test2json` framing. Those changes can manufacture head/base failures +and hide a real regression. Ordinary `-v` on focused probes reveals absent or +skipped tests; package lines and shard summaries establish ownership. Unclear +ownership stays red, including panics whose owning test cannot be proved from +plain text. +A failing test is run once more on your branch; if it passes, it is reported as +flaky. If it fails again, it is run at the base commit; if it fails there too, +it is reported as already failing on `dev`. Both stay green and are recorded on +one standing issue, because a flaky test is still a bug, just not yours. Only a +test that fails on your branch and passes on the base turns the job red, and so +do build failures, timeouts, crashes and more than five failures in one leg, +which are never re-run. The touched packages run as separate legs (`tui3`, +`session`, `codeaf`, `rest`) on separate runners, from a build cache refreshed +daily from `dev`. [docs/rules/ci.md](docs/rules/ci.md) has the details. + +There is no known-red ledger any more. It was emptied and deleted in #1012, and a +test that fails on a clean tree is a bug to fix, not a line to add back. There is no longer a "flakes under load" list here. The three that were on it — `TestOnlyADesignsOwnThreadCarriesTheReviseVerb`, diff --git a/Makefile b/Makefile index 8b3bc2bf3b..87c42b913d 100644 --- a/Makefile +++ b/Makefile @@ -198,6 +198,8 @@ test-quick: build-check vet fmt-check test-packed-manual changelog-check manual- # covered benchmark paths, or this Makefile. Together they take about forty # seconds and most pull requests never go near them. test-tooling: + bash scripts/touched-packages_test.sh + bash scripts/touched-verdict_test.sh bash scripts/one-suite_test.sh bash scripts/shard-test_test.sh bash bench/canary/lib/repo_test.sh @@ -215,8 +217,8 @@ manual-gates: # unrelated edits into this run over-tests. Commit the candidate, then prove # exactly what the pull request will send. The preflight is separate so # `pr-ready` refuses before spending anything on its light checks. Tests still -# go through `make test`, the one door for the timeout, known-red ledger and -# full heavy-package lock. +# go through the same classifier as CI, whose first run uses `make test` +# for the timeout and full heavy-package lock. test-touched-preflight: @set -eu; \ base="$${BASE:-origin/dev}"; \ @@ -235,36 +237,18 @@ test-touched-preflight: exit 2; \ fi -# A directory under its own go.mod is another module — a bench fixture the -# task door edits, not a package of this one — and `go test ./that/dir` from -# here answers "does not contain package". The walk skips those. +# The selector is shared with CI; this target adds only the local invocation. +# Every named failure gets one focused rerun and, if needed, a base comparison. test-touched: test-touched-preflight @set -eu; \ base="$${BASE:-origin/dev}"; \ - changed="$$(git diff --name-only "$$base" HEAD -- '*.go' go.mod go.sum | sort -u)"; \ - if printf '%s\n' "$$changed" | grep -qxE 'go\.(mod|sum)'; then \ - pkgs="./..."; \ - else \ - dirs="$$(printf '%s\n' "$$changed" | while IFS= read -r file; do \ - if test "$${file%.go}" != "$$file"; then dirname "$$file"; fi; \ - done | sort -u)"; \ - pkgs=""; \ - for dir in $$dirs; do \ - nested=0; walk="$$dir"; \ - while test "$$walk" != "." && test "$$walk" != "/"; do \ - if test -f "$$walk/go.mod"; then nested=1; break; fi; \ - walk="$$(dirname "$$walk")"; \ - done; \ - if test "$$nested" = 1; then continue; fi; \ - if ls "$$dir"/*.go >/dev/null 2>&1; then pkgs="$$pkgs ./$$dir"; fi; \ - done; \ - fi; \ + pkgs="$$(BASE="$$base" ./scripts/touched-packages.sh)"; \ if test -z "$$pkgs"; then \ echo 'No Go file and no module file changed; nothing to run.'; \ exit 0; \ fi; \ echo "touched:$$pkgs"; \ - $(MAKE) --no-print-directory test TEST_FLAGS='$(TEST_FLAGS) -count=1 -p 1' PKGS="$$pkgs" + printf '%s\n' "$$pkgs" | python3 ./scripts/touched-matrix.py run-local --base "$$base" --shards '$(SHARDS)' --timeout '$(TEST_TIMEOUT)' # One local spelling for the two jobs behind the pull request's required # `check`: first the deterministic light gate, then the exact touched-package diff --git a/cmd/codeaf/vocabulary_test.go b/cmd/codeaf/vocabulary_test.go index bcdb6b7eb9..495e164f2c 100644 --- a/cmd/codeaf/vocabulary_test.go +++ b/cmd/codeaf/vocabulary_test.go @@ -14,6 +14,7 @@ import ( "sort" "strconv" "strings" + "syscall" "testing" "github.com/Agent-Field/codeaf/internal/plan" @@ -410,8 +411,14 @@ func TestARenameNoticeNeverReachesTheJSONOnStdout(t *testing.T) { t.Setenv("CODEAF_HOME", t.TempDir()) // The writer itself, first: nothing else in this test can be right if the // notice is aimed at the wrong stream to begin with. - if renameNotice != os.Stderr { - t.Fatalf("the rename notice is written to %T, want os.Stderr", renameNotice) + // + // THE DESCRIPTOR IS THE FACT, NOT THE VARIABLE. Under `go test -json` the + // testing package swaps os.Stderr for a pipe of its own after this package's + // variables are initialized, so comparing with os.Stderr failed there although + // the notice still went to the process's standard error. The touched-packages + // job and `make test-report` both run that way. + if file, ok := renameNotice.(*os.File); !ok || file.Fd() != uintptr(syscall.Stderr) { + t.Fatalf("the rename notice is written to %T, want the process's standard error", renameNotice) } out, said := captureUsage(t) diff --git a/docs/changes/unreleased/1734-touched-packages-legs-and-attribution.md b/docs/changes/unreleased/1734-touched-packages-legs-and-attribution.md new file mode 100644 index 0000000000..855ce197b7 --- /dev/null +++ b/docs/changes/unreleased/1734-touched-packages-legs-and-attribution.md @@ -0,0 +1,18 @@ +--- +kind: changed +title: touched packages run concurrently and attribute flaky or inherited failures +pr: 1734 +surface: [build, docs] +invalidates: + - "The touched job ran every changed package in sequence and charged its first failure to the PR. It now starts the needed tui3, session, unsharded codeaf and rest legs together, retries each named failure once, then compares persistent failures with the exact base before deciding." + - "The PR gate restored a cache keyed only on go.sum, so weeks of compiled changes were never saved. The first dev push missing each build namespace's UTC-day key now saves it, and light saves modules by go.sum hash; PRs restore the newest compatible entries without saving, and GitHub evicts old days." + - "Local touched proof had its own package walk and no failure attribution. It now uses CI's shared selector, partitions and classifier; tooling acceptance remains conditional on the changed scripts, Makefile or covered benchmark paths." +--- + +A flaky test remains a bug with an owner, but its first failure is not evidence +that this PR introduced it. Unnamed failures and more than five failed tests per +leg stay red without retries; a new persistent head-only failure still blocks +check. The classifier prints human test output, skips no test by name and +records warnings in the lowest-numbered open exact-title standing issue. The +shared ledger guard, suite lock, light-gate prerequisites and full workflow +remain unchanged. diff --git a/docs/rules/ci.md b/docs/rules/ci.md index c5861ddcd0..bc39345a2a 100644 --- a/docs/rules/ci.md +++ b/docs/rules/ci.md @@ -4,7 +4,7 @@ | Where | Workflow | What runs | Roughly | | --- | --- | --- | --- | -| pull request into `dev`, and every push to `dev` | `.github/workflows/ci.yml` | `light gate`: build, gofmt, vet, the packed corpora, the change entry, the manual law, the laws. `touched packages`: the full suite of every package the change touched. `check`: green only when both are | `light gate` a few minutes; `touched packages` as long as the slowest touched package; `check` when both are in | +| pull request into `dev`, and every push to `dev` | `.github/workflows/ci.yml` | `light gate`: build, gofmt, vet, the packed corpora, the change entry, the manual law, the laws. `touched packages`: the full selected suites in concurrent legs with bounded failure attribution. `check`: green only when both are | `light gate` a few minutes; `touched packages` as long as the slowest touched package; `check` when both are in | | pull request into `staging` or `main`, every push to either, and nightly at 09:00 UTC | `.github/workflows/ci-full.yml` | the whole suite, six-platform cross build, the two-machine remote test | tens of minutes | | Friday after the 17:00 Toronto cutoff, or a manual dispatch | `.github/workflows/promote-staging.yml` | plan the cutoff commit, call Full check on that commit, fast-forward staging if it passes, and report the release and production signal | Full check plus the release build | | every push to `dev`, `staging` or `main`; a manual stable or channel dispatch | `.github/workflows/release.yml` | resolve and guard the tag, test the release surface except on dev, build six binaries with furrow, publish | — | @@ -60,36 +60,76 @@ Seven things in `ci.yml`, job name `light gate`, then the touched packages, then it. Six seconds, and it is the law that gets broken most. - **The laws hold.** `make test-laws`: every test in the tree that opens the repository's own source with `go/ast` or `go/parser` — the endings ratchet, - the guard, the taxonomy, the known-red ratchet, the words the e2e suite waits + the guard, the taxonomy, the words the e2e suite waits for. `scripts/laws.sh` finds them by that import, so a new law is on the gate the day it is written and there is no list to forget. About twenty seconds after the link. Every test in a file that carries the marker runs, so a file whose behavioural half is slow should move its laws out rather than argue with the script. -**`touched packages`** is the second job: the full suite of every package the -change touched, through `make test` with `-count=1`, so it reads the same ledger -and the same timeout as a laptop and never a cached pass. It runs on every pull -request and every push to `dev`, beside the light gate rather than after it. A -change with no Go file and no module file runs nothing here and is green in a -minute; a change to `go.mod` or `go.sum` runs the whole tree. **It blocks.** It +**`touched packages`** is the aggregate job: the full suite of every package +this change touched runs with `-count=1` in concurrent tui3, session, codeaf and +rest legs on separate runners, beside the light gate. `scripts/touched-packages.sh` +is the ONE selector shared with `make test-touched`; it compares `BASE..HEAD`, +maps changed `.go` files to surviving root-module packages, excludes nested +modules and emptied directories, and expands root `go.mod` or `go.sum` changes +to the whole tree. No Go or module change means no test leg. **It blocks.** It was neutral for a day, then off pull requests for a day (#499), and in that day #523 merged red on `cmd/codeaf` with `check` green, as #437, #439 and #483 had before the job existed. The owner's ruling is that it runs on the pull request and `check` needs it. -When the touched set contains `internal/tui3` or `internal/session`, `make test` -compiles that heavy package once and runs its sorted top-level tests as -round-robin concurrent shards. `SHARDS` defaults to at most eight and can be set -to `1` to reproduce the serial outcome; the package's whole sharded run still -holds the one-suite-per-box lock. - -**`check`** is the third job and the only required name: it needs the other two -and is green only when both are, the same one-spellable-name shape `full tests` -and `cross build` use in `ci-full.yml`. There is no branch protection on this -repository, so this is as blocking as a check can be here: a red `check` is what -every landing script and every person reads, and nothing merges over it by -convention. +The public standard Ubuntu runner has 4 vCPU and 16 GB; each leg prints `nproc` +and `free -g` so a run records its actual capacity. `-p 2` allows concurrent +compilation/testing in rest while leaving linker headroom. `make test` compiles +`internal/tui3` and `internal/session` once each, then runs sorted top-level +tests as round-robin concurrent shards: four in CI, locally at most eight by +default, or `SHARDS=1` for the serial outcome. Their complete runs still hold +the one-suite-per-box lock. Sharding `cmd/codeaf` was tried and its sharded runs +failed, so it is not sharded. Its codeaf leg still runs beside tui3 and session +on a separate runner. Each suite/shard and focused probe gets 15 minutes; a +leg's 60-minute ceiling leaves room for bounded diagnosis. + +`scripts/touched-verdict.sh` reads the plain output of the same `make test` run +everyone else gets, including package lines and the shard runner's summaries. +The initial run, head retry and base probe all avoid `-json`: Go 1.26 replaces +`os.Stderr` under that flag, and helpers re-executing the test binary inherit +`-test.v=test2json` and print framing that changes their output. Those changes +can manufacture failures at both head and base and hide a real regression. +Focused probes use ordinary `-v` to distinguish a pass from an absent or skipped +test. The job log is the original text. Every test runs in the initial selected +suites: the classifier disables the shared Makefile's ledger skip flags. +Each named failing test gets ONE focused retry on +head, then, if it fails again, ONE focused probe at the exact base (PR base SHA, +previous push tip, or `HEAD~1` for a first push or dispatch). Subtests use their +full names so passing siblings are not retried; a temporary detached base +worktree is created only when needed and removed afterward. + +- A passing head retry is **flaky**; another failure at base is **already failing + on the base**. Both warn with package/test names, appear in the step summary, + and stay green. The first failure remains in the log. +- A base pass or a test/package absent at base makes the failure this change's + own and stays red. A skipped test or unbuildable base is inconclusive and red. +- Build/setup failures, timeout panics, panics outside a test, killed processes, + shard errors and other unnamed failures stay red without retries, even beside + named failures. Plain text cannot prove a panic's test ownership, so all + panics stay red; missing package lines or shard summaries also stay red. + **More than five failed tests per leg** fail with their list + and no probes; ten focused runs is the maximum diagnostic budget. + +A flaky test is still a bug with an owner; it just is not evidence that this PR +introduced it. The aggregate appends ONE comment per same-repository run with +warnings (run link, SHA, package, test, classification) to the lowest-numbered +open issue with the exact title **touched packages: flaky or already-failing +tests**, creating it if absent. A rare find-or-create race may create duplicates; +later runs converge on the lowest number. The issue is never closed automatically. +Forks make no issue call; API/artifact-reporting errors do not change the result. + +**`check`** remains the only required name: it needs the light gate and the +`touched packages` aggregate and is green only when both are. The aggregate +requires a successful selector and every needed leg; an empty selection is +green, while failed selection or failed/cancelled needed legs stay red. Matrix +fail-fast is disabled so one leg cannot cancel another's evidence. Run the same thing before you push: @@ -97,15 +137,34 @@ Run the same thing before you push: make pr-ready ``` -`make pr-ready` is the local spelling of `check`: it runs the light-gate pieces -above, then `make test-touched`. The touched target compares `BASE..HEAD`, maps -changed `.go` files to their surviving package directories, treats `go.mod` or -`go.sum` as a whole-tree change, and runs the result through `make test` with -`-count=1 -p 1`. It refuses when a Go or module file is staged, unstaged or -untracked: commit the candidate first so the proof sees exactly what CI will -see, without absorbing another session's work. Pass `BASE=` to -reproduce a pull request's exact base. A docs-only change has no touched package -and still runs the light half. +`make pr-ready` runs the light-gate pieces above, tooling acceptance only when +scripts/, the Makefile or the covered benchmark paths changed, then +`make test-touched`. The touched target uses the same selector, partitions and +classifier as CI with `-count=1 -p 2`; local legs run sequentially on one box, +preserving the per-leg failure cap. It refuses staged, unstaged or untracked Go +or module files: commit the candidate so the proof sees exactly what CI sees, +without absorbing another session's work. Pass `BASE=` for the exact +PR base; locally it defaults to `origin/dev`. A docs-only change still runs the +light half. Local classification makes no issue API call. `test-quick` keeps +its existing prerequisites; tooling is not an unconditional light-gate step. + +setup-go's implicit cache is disabled in `ci.yml`. Build keys are +`codeaf-go-v1--build----` for light, +tui3, session, codeaf and rest, with no SHA. The undated restore prefix chooses +the newest compatible cache, whatever its age; legs then fall back to light's +prefix. The first dev push of each UTC day that misses the exact key saves; +later exact hits do not, and PRs never save. Modules use +`codeaf-go-v1-modules----`, restore +without the hash, and are saved only by light on a dev push that misses the key. +Every touched leg runs `go mod download` after restoring caches and before +testing: it compiles only part of the tree, but offline module-listing tests +such as codeaf-notices need all dependencies available. Light builds the whole +tree and continues saving the shared module cache. +At most five build namespaces a day plus one module entry per go.sum cost about +2 GB a day at the measured ~385 MB; GitHub's least-recently-used eviction at the +10 GB repository limit removes old days without any pruning script. Cache +saves remain useful even after a test failure. Hosted scheduling, cache hits +and archive sizes still need an actual workflow run to confirm. This is pull-request parity, not the full-tree ritual. `make check` remains for Spark, staging, or an intentional full laptop run; it runs the whole test tree, @@ -119,10 +178,10 @@ builds the shipped binary and enforces its size budget. across three runners. Within each runner, `make test` also runs `internal/tui3` and `internal/session` as concurrent test-binary shards from one compile; the outer three-way split still assigns each package to only one - runner. Not for speed first: the - free-plan runner has seven gigabytes, this repository's test binaries are - heavy, and the suite's first run was killed under the link load of its last - eight packages. Three machines carrying a third each stay inside their memory. + runner. The outer split and `-p 1` remain unchanged: they were introduced + when the private free-plan runner had two cores and seven gigabytes and died + under link load. The repository is public now, but this change reworks the + PR gate rather than remeasuring the full gate. It runs through `make test`, so the ledger it skips and the per-shard timeout it carries are the Makefile's and the same as a laptop's. The timeout is measured, not guessed: `internal/tui3` once took about 485 seconds serial on @@ -155,8 +214,8 @@ CI to find, and no gate to name. **So a green gate proves nothing about data races, and this page is where that is said rather than assumed.** The detector makes a test run two to twenty times slower and five to ten times fatter, by Go's own estimate, and `full -tests` is already split three ways to stay inside the free-plan runner's seven -gigabytes. It is also not the light gate's kind of question: a race is a window +tests` retains its conservative three-way split from the private-runner era. +It is also not the light gate's kind of question: a race is a window between two goroutines, so its answer is not the same on every machine — #957 needed `-count=5` to be sure its quiet first run was quiet. What that trade cost is on the record there: the detector went red on clean `dev`, on a fixture @@ -184,11 +243,11 @@ after; #408 took the first five out, and the burn finished on 2026-09-12 when the last entry — the lockdefer scan — was fixed and the file and its `internal/ci` ratchet test were deleted together (#1012). -**Red means this change did it, now with nothing skipped.** Every run — the -nightly, `touched packages`, the laws — goes through `make test` or reads the -file's old location the same way (`Makefile`, `scripts/laws.sh`, both workflows), -and an absent ledger skips nothing, so "green locally" and "green in CI" are one -fact with no debt subtracted. A test that fails on a clean tree today is a bug +The Makefile and laws runner still read the ledger's old location as the +owner's guard, with their existing comments and skip flags; the file remains +absent and skips nothing. The touched classifier explicitly clears those flags +and never skips by name, using bounded attribution instead. Full/nightly runs +retain their strict first-run result. A failure on a clean tree today is a bug report, not a line to add back — the ledger does not return. ## A test that fails only beside another suite @@ -216,8 +275,10 @@ the SCHEDULER and called it the road: primary that must answer *after* the caller has acted says exactly that, rather than being scripted a few milliseconds past a bound a busy machine eats. -- **Never `t.Skip`, never a retry, never a wider timeout.** A bound is for - failing honestly when the fact never arrives, not for passing. +- **Never fix a test with `t.Skip`, a retry loop or a wider timeout.** A bound + is for failing honestly when the fact never arrives. The touched gate's ONE + diagnostic retry and base probe attribute the failure while keeping it owned; + they do not repair a test or authorize rerunning until green. ## What blocks a merge @@ -234,9 +295,6 @@ in a row.** That is the next piece of work here, not a someday. ## When GitHub enforces it -While `Agent-Field` is on the free plan and the repository is private, that -combination has no branch rules — the API answers `403 Upgrade to GitHub Pro`; -the checks above run and show red, but nothing stops a merge on top of red, and -nothing stops a direct push. **Until the org moves to GitHub Team or the -repository is public, all of this is convention.** `.github/rulesets/README.md` -has the state of that. +The repository is now public, so the free plan's former private-repository +restriction no longer prevents branch rules. `.github/rulesets/` holds the +intended rules; inspect live enforcement before assuming they were applied. diff --git a/scripts/touched-matrix.py b/scripts/touched-matrix.py new file mode 100755 index 0000000000..03d8a616f4 --- /dev/null +++ b/scripts/touched-matrix.py @@ -0,0 +1,25 @@ +#!/usr/bin/env python3 +"""Partition the shared selector's concrete package list into runner legs.""" +import json +from pathlib import Path +import subprocess +import sys + +named = {"./internal/tui3": "tui3", "./internal/session": "session", "./cmd/codeaf": "codeaf"} +groups = {} +for package in sys.stdin.read().split(): + groups.setdefault(named.get(package, "rest"), []).append(package) +matrix = {"include": [{"leg": leg, "packages": " ".join(packages)} + for leg, packages in sorted(groups.items())]} +if sys.argv[1:2] == ["run-local"]: + # A laptop has one box, so legs run sequentially there. Partitioning still + # matters: the failure cap is per leg, exactly as on separate CI runners. + status = 0 + for leg, packages in sorted(groups.items()): + print(f"touched {leg}: {' '.join(packages)}", flush=True) + runner = Path(__file__).resolve().with_name("touched-verdict.sh") + result = subprocess.run([str(runner), "run", *sys.argv[2:], *packages]) + status = max(status, int(result.returncode != 0)) + sys.exit(status) +print("matrix=" + json.dumps(matrix, separators=(",", ":"))) +print("has-tests=" + ("true" if groups else "false")) diff --git a/scripts/touched-packages.sh b/scripts/touched-packages.sh new file mode 100755 index 0000000000..47192481f7 --- /dev/null +++ b/scripts/touched-packages.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash +# ONE WALK FOR THE LAPTOP AND CI. Only surviving directories in the root +# module are targets; a fixture with its own go.mod belongs to another tree. +set -euo pipefail +cd "$(git rev-parse --show-toplevel)" + +base="${BASE:-origin/dev}" +case "$base" in '' | 0000000000000000000000000000000000000000) base=HEAD~1 ;; esac +git rev-parse --verify "$base^{commit}" >/dev/null +changed="$(git diff --name-only "$base" HEAD -- '*.go' go.mod go.sum)" +if printf '%s\n' "$changed" | grep -qxE 'go\.(mod|sum)'; then + # go list excludes nested modules and returns concrete packages, so the same + # list can be partitioned without a second interpretation of ./.... + go list -f '{{if .GoFiles}}{{.Dir}}{{else if .CgoFiles}}{{.Dir}}{{else if .TestGoFiles}}{{.Dir}}{{else if .XTestGoFiles}}{{.Dir}}{{end}}' ./... | + while IFS= read -r dir; do + [ -n "$dir" ] || continue + if [ "$dir" = "$PWD" ]; then printf './\n'; else printf './%s\n' "${dir#"$PWD"/}"; fi + done | LC_ALL=C sort -u +else + printf '%s\n' "$changed" | while IFS= read -r file; do + case "$file" in *.go) dirname "$file" ;; esac + done | LC_ALL=C sort -u | while IFS= read -r dir; do + [ -n "$dir" ] || continue + nested=; walk="$dir" + while [ "$walk" != . ] && [ "$walk" != / ]; do + if [ -f "$walk/go.mod" ]; then nested=yes; break; fi + walk="$(dirname "$walk")" + done + [ -z "$nested" ] || continue + # Deleting the final source file deletes the target, even if an asset or + # an empty directory remains in the checkout. + if compgen -G "$dir/*.go" >/dev/null; then + if [ "$dir" = . ]; then printf './\n'; else printf './%s\n' "$dir"; fi + fi + done +fi diff --git a/scripts/touched-packages_test.sh b/scripts/touched-packages_test.sh new file mode 100644 index 0000000000..9108c90224 --- /dev/null +++ b/scripts/touched-packages_test.sh @@ -0,0 +1,128 @@ +#!/usr/bin/env bash +# Real commits in a throwaway repository pin the selection contract. The +# working repository's index and history are never touched by this fixture. +set -euo pipefail +export GOFLAGS=-buildvcs=false +root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +tmp="$(mktemp -d /tmp/codeaf-touched-packages-test.XXXXXX)" +trap 'rm -rf -- "$tmp"' EXIT +export GIT_AUTHOR_NAME='tooling fixture' GIT_AUTHOR_EMAIL=fixture@example.invalid +export GIT_COMMITTER_NAME="$GIT_AUTHOR_NAME" GIT_COMMITTER_EMAIL="$GIT_AUTHOR_EMAIL" + +fixture() { + mkdir -p "$tmp/$1" + cd "$tmp/$1" + git init -q + printf 'module example.invalid/fixture\n\ngo 1.23\n' >go.mod + for package in internal/tui3 internal/session cmd/codeaf internal/one internal/two emptied; do + mkdir -p "$package" + printf 'package fixture\n' >"$package/file.go" + done + mkdir -p bench/bashloop/fixtures/nested/deep + printf 'module example.invalid/nested\n\ngo 1.23\n' >bench/bashloop/fixtures/nested/go.mod + printf 'package fixture\n' >bench/bashloop/fixtures/nested/deep/file.go + printf 'docs\n' >README.md + git add go.mod README.md internal cmd emptied bench + git commit -qm base + BASE="$(git rev-parse HEAD)"; export BASE +} + +finish() { + git add "$@" + git commit -qm change +} + +expect() { + local actual + actual="$("$root/scripts/touched-packages.sh")" + [ "$actual" = "$1" ] || { printf 'packages: got <%s>, want <%s>\n' "$actual" "$1" >&2; exit 1; } + printf '%s\n' "$actual" | python3 "$root/scripts/touched-matrix.py" >"$tmp/matrix" + python3 - "$tmp/matrix" "$2" <<'PY' +import json, sys +outputs = dict(line.strip().split('=', 1) for line in open(sys.argv[1])) +legs = json.loads(outputs['matrix'])['include'] +assert {row['leg'] for row in legs} == set(sys.argv[2].split()), legs +assert outputs['has-tests'] == ('true' if legs else 'false'), outputs +selected = [package for row in legs for package in row['packages'].split()] +assert len(selected) == len(set(selected)), legs +PY +} + +fixture docs +printf 'more docs\n' >>README.md +finish README.md +expect '' '' + +for leg in tui3 session codeaf; do + fixture "$leg" + case "$leg" in codeaf) package=cmd/codeaf ;; *) package=internal/$leg ;; esac + printf '// Changed source.\n' >>"$package/file.go" + finish "$package/file.go" + expect "./$package" "$leg" +done + +fixture rest +printf '// Changed source.\n' >>internal/one/file.go +printf '// Changed source.\n' >>internal/two/file.go +finish internal/one/file.go internal/two/file.go +expect $'./internal/one\n./internal/two' rest + +fixture mixed +for package in internal/tui3 internal/session cmd/codeaf internal/one; do + printf '// Changed source.\n' >>"$package/file.go" +done +finish internal/tui3/file.go internal/session/file.go cmd/codeaf/file.go internal/one/file.go +expect $'./cmd/codeaf\n./internal/one\n./internal/session\n./internal/tui3' 'codeaf rest session tui3' + +for file in go.mod go.sum; do + fixture "$file" + if [ "$file" = go.mod ]; then printf '\n' >>go.mod; else touch go.sum; fi + finish "$file" + expect $'./cmd/codeaf\n./emptied\n./internal/one\n./internal/session\n./internal/tui3\n./internal/two' 'codeaf rest session tui3' +done + +fixture excluded +rm emptied/file.go +printf '// Changed fixture.\n' >>bench/bashloop/fixtures/nested/deep/file.go +finish emptied/file.go bench/bashloop/fixtures/nested/deep/file.go +expect '' '' + +# The real local target must consume exactly the same shared script. Substitute +# only the costly suite runner, then compare its package arguments with C2. +fixture local +mkdir scripts +cp "$root/scripts/touched-packages.sh" scripts/touched-packages.sh +cp "$root/scripts/touched-matrix.py" scripts/touched-matrix.py +cat >scripts/touched-verdict.sh <<'SH' +#!/usr/bin/env bash +printf '%s\n' "$@" >>"$TOUCHED_ARGS" +SH +chmod +x scripts/touched-verdict.sh +printf '// Changed source.\n' >>internal/tui3/file.go +finish internal/tui3/file.go +TOUCHED_ARGS="$tmp/local-args" make -s -f "$root/Makefile" test-touched >/dev/null +python3 - "$tmp/local-args" <<'PY' +import sys +arguments = open(sys.argv[1]).read().splitlines() +assert arguments[0] == 'run', arguments +assert arguments[-1] == './internal/tui3', arguments +assert len([arg for arg in arguments if arg.startswith('./')]) == 1, arguments +PY + +# C5 also holds across all four legs: no package is lost or repeated when the +# laptop executes the partition sequentially instead of on separate runners. +: >"$tmp/local-args" +for package in cmd/codeaf internal/session internal/one internal/two; do + printf '// Changed source.\n' >>"$package/file.go" +done +finish cmd/codeaf/file.go internal/session/file.go internal/one/file.go internal/two/file.go +TOUCHED_ARGS="$tmp/local-args" make -s -f "$root/Makefile" test-touched >/dev/null +python3 - "$tmp/local-args" <<'PYTEST' +import sys +arguments = open(sys.argv[1]).read().splitlines() +assert arguments.count('run') == 4, arguments +packages = [argument for argument in arguments if argument.startswith('./')] +assert sorted(packages) == ['./cmd/codeaf', './internal/one', './internal/session', './internal/tui3', './internal/two'], arguments +PYTEST + +printf 'touched-package selection acceptance: ok\n' diff --git a/scripts/touched-verdict.py b/scripts/touched-verdict.py new file mode 100755 index 0000000000..4211131d68 --- /dev/null +++ b/scripts/touched-verdict.py @@ -0,0 +1,331 @@ +#!/usr/bin/env python3 +"""Run touched suites once, then attribute bounded, named failures.""" +import argparse +import json +import os +from pathlib import Path +import re +import subprocess +import sys +import tempfile + +# FIVE FAILURES IS THE DIAGNOSTIC BUDGET. Ten focused runs plus at most one +# base checkout are useful evidence; a broken tree must not spawn hundreds. +FAILURE_CAP = 5 +ISSUE_TITLE = "touched packages: flaky or already-failing tests" + + +def annotation(level, message): + escaped = message.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") + print(f"::{level}::{escaped}", flush=True) + + +def capture(command, cwd=None): + print("+ " + " ".join(command), flush=True) + process = subprocess.Popen(command, cwd=cwd, stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, text=True, errors="replace") + lines = [] + for line in process.stdout: + sys.stdout.write(line) + sys.stdout.flush() + lines.append(line) + return process.wait(), "".join(lines) + + +def plain_results(output): + tests, packages, errors = [], [], [] + pending = [] + shard_package = None + for line in output.splitlines(): + shard = re.match(r"^(ok|FAIL)\s+(\S+)\s+shard \d+/\d+\s+\d+s$", line) + summary = re.match(r"^(ok|FAIL)\s+(\S+)\s+\d+ shards\s+\d+s(?:\s+failing: (.*))?$", line) + package = re.match(r"^(ok|FAIL|\?)\s+(\S+)\s+(?:[\d.]+s|\(cached\)|\[[^\]]+\])(?:\s.*)?$", line) + test = re.match(r"^\s*--- (PASS|FAIL|SKIP): (\S+) \([\d.]+s\)\s*$", line) + if shard: + if pending: + errors.append("test results have no terminal package line before a shard") + pending = [] + if shard_package and shard_package != shard[2]: + errors.append("sharded package has no terminal summary: " + shard_package) + # The sharder prints a header BEFORE each failing shard's output. + # Ordinary Go package output instead ends with its package line. + shard_package = shard[2] + elif summary: + if shard_package and shard_package != summary[2]: + errors.append("shard summary names a different package") + packages.append((summary[2], summary[1])) + if summary[1] == "FAIL": + names = summary[3] + if not names or names == "unknown": + errors.append("shard summary has no attributable test name") + else: + tests.extend((summary[2], name, "FAIL") for name in names.split(",")) + shard_package = None + elif test: + # Keep the full name from Go, including every indented subtest. + # Parent propagation is removed only after package ownership is known. + if shard_package: + tests.append((shard_package, test[2], test[1])) + else: + pending.append((test[2], test[1])) + elif package: + if shard_package: + errors.append("sharded package has no terminal summary: " + shard_package) + shard_package = None + packages.append((package[2], package[1])) + tests.extend((package[2], name, action) for name, action in pending) + pending = [] + if pending: + errors.append("test results have no terminal package line") + if shard_package: + errors.append("sharded package has no terminal summary: " + shard_package) + return {"tests": tests, "packages": packages, "errors": errors} + + +def infrastructure(output, results, status): + if status < 0 or status in (137, 143): + return "killed process" + for pattern, reason in [ + (r"\[setup failed\]", "package setup failure ([setup failed])"), + (r"\[build failed\]|^# \S+", "build failure ([build failed])"), + (r"^shard-test:", "shard runner error"), + (r"signal: killed|\bKilled\b|(?:exit status|Error) (137|143)", "killed process"), + (r"^panic: test timed out", "package-level timeout (panic: test timed out)"), + (r"^panic:|^fatal error:", "panic outside a test or panic ownership unclear in plain output"), + ]: + if re.search(pattern, output, re.MULTILINE): + return reason + # Plain output cannot prove a panic belongs to an assertion's test, even + # when a named failure appears nearby. Uncertain ownership MUST stay red. + if results["errors"]: + return results["errors"][0] + if not results["packages"]: + return "runner produced no terminal package result" + failures = failed_tests(results) + if status and not failures: + return "runner failed without an attributable test name" + failed_packages = {package for package, action in results["packages"] if action == "FAIL"} + attributed = {package for package, _ in failures} + if failed_packages - attributed: + return "package failed without an attributable test name" + if attributed - failed_packages: + return "failed test has no failing terminal package result" + if not status and failed_packages: + return "runner succeeded despite a failing package result" + return None + + +def failed_tests(results): + failed = sorted({(package, name) for package, name, action in results["tests"] if action == "FAIL"}) + # Go reports each failed child and its parents. Retry the leaves so a + # propagated parent failure does not rerun passing siblings as well. + return [(package, name) for package, name in failed + if not any(other_package == package and other.startswith(name + "/") + for other_package, other in failed)] + + +def selector(name): + # Go splits -run on slashes before matching each component. Escape only + # RE2 metacharacters; Python's escaped hyphens are not valid RE2 escapes. + return "/".join("^(" + re.sub(r"([\\.\[\]{}()*+?^$|])", r"\\\1", part) + ")$" + for part in name.split("/")) + + +def focused(package, name, timeout, cwd=None): + # Ordinary -v exposes pass/skip names, so an absent or skipped target cannot + # masquerade as a pass. -json changes stderr and helper-process behaviour. + status, output = capture(["go", "test", "-v", "-count=1", "-p", "2", + "-timeout", timeout, "-run", selector(name), package], cwd) + results = plain_results(output) + reason = infrastructure(output, results, status) + if reason and any(re.search(pattern, output) for pattern in ( + r"no required module provides package\s+" + re.escape(package) + r"(?:;|\s)", + r"package " + re.escape(package) + r" is not in std", + r"main module .* does not contain package\s+" + re.escape(package) + r"(?:\s|$)", + )): + return "absent", "package absent (cannot resolve target)" + target = [action for owner, test, action in results["tests"] if owner == package and test == name] + if reason: + return "error", reason + if not target: + return "absent", "test or package absent (no matching test result)" + if target[-1] == "SKIP": + return "absent", "test did not run (skipped)" + if target[-1] == "FAIL": + if any(owner != package or (test != name and not test.startswith(name + "/")) + for owner, test in failed_tests(results)): + return "error", "focused run failed outside the selected test" + return "fail", None + if status or failed_tests(results): + return "error", "focused run failed outside the selected test" + return "pass", None + + +def write_report(path, rows, reasons): + if path: + Path(path).parent.mkdir(parents=True, exist_ok=True) + Path(path).write_text(json.dumps({"tests": rows, "errors": reasons}, indent=2) + "\n") + if not rows and not reasons: + print("Every selected test passed on its first run.", flush=True) + summary = os.environ.get("GITHUB_STEP_SUMMARY") + if summary: + with open(summary, "a") as out: + out.write("\n### Touched package results\n\n") + for reason in reasons: + out.write(f"- {reason}\n") + for row in rows: + out.write(f"- `{row['package']}` / `{row['test']}`: {row['verdict']}\n") + if not rows and not reasons: + out.write("Every selected test passed on its first run.\n") + + +def run(args): + rows, reasons = [], [] + base_tree = None + result = 0 + try: + status, output = capture(["make", "--no-print-directory", "-s", "test", + "PKGS=" + " ".join(args.packages), + "KNOWN_RED=", "TEST_SKIP=", + "TEST_FLAGS=-count=1 -p 2", "SHARDS=" + args.shards, + "TEST_TIMEOUT=" + args.timeout]) + results = plain_results(output) + reason = infrastructure(output, results, status) + failures = failed_tests(results) + if reason: + reasons.append(reason + "; no retry") + annotation("error", reasons[-1]) + return 1 + if not status and not failures: + return 0 + if len(failures) > FAILURE_CAP: + reasons.append(f"{len(failures)} failing tests exceed the cap of {FAILURE_CAP}; " + "no retries or base probes: mass breakage needs investigation at head and base") + annotation("error", reasons[-1]) + for package, name in failures: + rows.append({"package": package, "test": name, "verdict": "failure cap exceeded"}) + annotation("error", f"{package}: {name} (failure cap exceeded)") + return 1 + for package, name in failures: + retry, detail = focused(package, name, args.timeout) + if retry == "pass": + verdict = "flaky" + elif retry != "fail": + verdict = "head retry could not execute: " + detail + result = 1 + else: + if base_tree is None: + base_tree = tempfile.TemporaryDirectory(prefix="codeaf-touched-base-") + status, _ = capture(["git", "worktree", "add", "--detach", + str(Path(base_tree.name) / "tree"), args.base]) + if status: + reasons.append("cannot create base worktree; stopping without a verdict") + annotation("error", reasons[-1]) + return 1 + base, detail = focused(package, name, args.timeout, str(Path(base_tree.name) / "tree")) + if base == "fail": + verdict = "already failing on the base" + elif base == "pass": + verdict = "introduced by this change" + result = 1 + elif base == "absent" and "absent" in detail: + verdict = "introduced by this change; base could not run the test: " + detail + result = 1 + else: + verdict = "attribution inconclusive; base could not run the test: " + detail + result = 1 + rows.append({"package": package, "test": name, "verdict": verdict}) + level = "warning" if verdict in ("flaky", "already failing on the base") else "error" + annotation(level, f"{package}: {name}: {verdict}") + if retry == "error": + reasons.append("head retry infrastructure failure; stopping further probes") + return 1 + return result + except OSError as error: + reasons.append("runner error: " + str(error) + "; no retry") + annotation("error", reasons[-1]) + return 1 + finally: + if base_tree is not None: + tree = str(Path(base_tree.name) / "tree") + if Path(tree).exists(): + status, _ = capture(["git", "worktree", "remove", "--force", tree]) + if status: + # Do not erase a checkout whose git registration could not + # be removed. Leave the exact path for a person to repair. + base_tree._finalizer.detach() + reasons.append("cannot remove base worktree: " + tree) + annotation("error", reasons[-1]) + write_report(args.report, rows, reasons) + raise RuntimeError(reasons[-1]) + base_tree.cleanup() + write_report(args.report, rows, reasons) + + +def gh(arguments): + return subprocess.check_output(["gh", *arguments], text=True, stderr=subprocess.PIPE, timeout=30) + + +def report_issues(args): + # This runs once in the aggregate job, so four concurrent legs make one + # comment and cannot race each other to create the standing issue. + if os.environ.get("TOUCHED_REPORT_ISSUE") != "true": + print("Issue reporting disabled for this run (fork or local run).") + return 0 + try: + rows = [] + for path in sorted(Path(args.directory).glob("*.json")): + rows.extend(row for row in json.loads(path.read_text())["tests"] + if row["verdict"] in ("flaky", "already failing on the base")) + if not rows: + return 0 + repository = os.environ["GITHUB_REPOSITORY"] + matches = json.loads(gh(["issue", "list", "--repo", repository, "--state", "open", + "--search", '"' + ISSUE_TITLE + '" in:title', "--limit", "100", + "--json", "number,title"])) + # Concurrent runs may create duplicates. Converge on the oldest open + # exact-title issue instead of relying on the API's result ordering. + issue = min((row["number"] for row in matches if row["title"] == ISSUE_TITLE), default=None) + if issue is None: + url = gh(["issue", "create", "--repo", repository, "--title", ISSUE_TITLE, + "--body", "These tests remain bugs with an owner. Each comment records " + "a flaky or inherited failure; no test is skipped and this issue never closes automatically."]).strip() + issue = url.rsplit("/", 1)[-1] + run_url = (os.environ.get("GITHUB_SERVER_URL", "https://github.com") + "/" + repository + + "/actions/runs/" + os.environ["GITHUB_RUN_ID"]) + body = f"Run: {run_url}\nSHA: `{os.environ['GITHUB_SHA']}`\n\n" + body += "\n".join(f"- `{row['package']}` / `{row['test']}`: {row['verdict']}" for row in rows) + with tempfile.NamedTemporaryFile(mode="w", suffix=".md") as out: + out.write(body + "\n") + out.flush() + gh(["issue", "comment", str(issue), "--repo", repository, "--body-file", out.name]) + except (OSError, ValueError, KeyError, TypeError, subprocess.SubprocessError) as error: + annotation("warning", "Could not record touched-test ownership; test result is unchanged: " + str(error)) + return 0 + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + subcommands = parser.add_subparsers(dest="command", required=True) + runner = subcommands.add_parser("run") + runner.add_argument("--base", required=True) + runner.add_argument("--report") + # The Makefile owns the timeout for initial suites and focused diagnosis. + # Reading its literal avoids a second policy number drifting in this file. + makefile = (Path(__file__).resolve().parent.parent / "Makefile").read_text() + timeout = re.search(r"^TEST_TIMEOUT\s*:?=\s*(\S+)", makefile, re.MULTILINE).group(1) + runner.add_argument("--timeout", default=timeout) + runner.add_argument("--shards", default=os.environ.get("SHARDS", "4")) + runner.add_argument("packages", nargs="+") + reporter = subcommands.add_parser("report-issues") + reporter.add_argument("directory") + args = parser.parse_args() + try: + return run(args) if args.command == "run" else report_issues(args) + except (OSError, RuntimeError) as error: + annotation("error", str(error)) + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/touched-verdict.sh b/scripts/touched-verdict.sh new file mode 100755 index 0000000000..1c1ff31765 --- /dev/null +++ b/scripts/touched-verdict.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash +# The classifier consumes Go's event stream, rather than guessing test names +# from assertion prose or hiding the first failure behind a successful retry. +set -euo pipefail +exec python3 "$(dirname "${BASH_SOURCE[0]}")/touched-verdict.py" "$@" diff --git a/scripts/touched-verdict_test.sh b/scripts/touched-verdict_test.sh new file mode 100644 index 0000000000..0fed4bc8f3 --- /dev/null +++ b/scripts/touched-verdict_test.sh @@ -0,0 +1,267 @@ +#!/usr/bin/env bash +# PATH stubs replay failures without building the product or changing the real +# git worktree. Calls are recorded so a green cannot hide an unbounded retry. +set -euo pipefail +export GOFLAGS=-buildvcs=false +root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +tmp="$(mktemp -d /tmp/codeaf-touched-verdict-test.XXXXXX)" +trap 'rm -rf -- "$tmp"' EXIT +mkdir "$tmp/bin" +cat >"$tmp/bin/stub" <<'PY' +#!/usr/bin/env python3 +import json, os +from pathlib import Path +import sys + +tool = Path(sys.argv[0]).name +directory = Path(os.environ['CASE_DIR']) +with (directory / 'calls').open('a') as out: + out.write(json.dumps({'tool': tool, 'args': sys.argv[1:], 'cwd': os.getcwd()}) + '\n') +scenario = os.environ['SCENARIO'] +package = 'example.invalid/fixture/pkg' +name = 'TestOne' + +def test_result(test, action='FAIL'): + # Go prints full subtest names at an indentation matching their depth. + print(' ' * test.count('/') + f'--- {action}: {test} (0.00s)', flush=True) + +def package_result(pkg=package, failed=True): + print(('FAIL' if failed else 'ok ') + '\t' + pkg + '\t0.001s', flush=True) + +if tool == 'git': + if sys.argv[1:3] == ['worktree', 'add']: + Path(sys.argv[-2]).mkdir(parents=True) + sys.exit(0) +if tool == 'gh': + if scenario == 'api-malformed': + print('null') + sys.exit(0) + if scenario == 'api-error': + print('API unavailable', file=sys.stderr) + sys.exit(1) + if sys.argv[1:3] == ['issue', 'list']: + assert sys.argv[sys.argv.index('--state') + 1] == 'open', sys.argv + # A close title is not the exact standing issue. + print(json.dumps([{'number': 6, 'title': 'touched packages: flaky or already-failing tests extra'}, + {'number': 9, 'title': 'touched packages: flaky or already-failing tests'}, + {'number': 7, 'title': 'touched packages: flaky or already-failing tests'}] + if scenario != 'create-issue' else [])) + elif sys.argv[1:3] == ['issue', 'create']: + print('https://github.com/example/fixture/issues/8') + elif sys.argv[1:3] == ['issue', 'comment']: + body = Path(sys.argv[sys.argv.index('--body-file') + 1]).read_text() + (directory / 'issue-body').write_text(body) + sys.exit(0) +if tool in ('make', 'go'): + assert not any('-json' in arg or 'test2json' in arg for arg in sys.argv), sys.argv +if tool == 'make': + # No name-based skip may sneak back into the first run. + assert not any('-skip' in arg for arg in sys.argv) + assert 'KNOWN_RED=' in sys.argv and 'TEST_SKIP=' in sys.argv + assert 'TEST_FLAGS=-count=1 -p 2' in sys.argv, sys.argv + if scenario == 'green': + package_result(failed=False) + print('? \texample.invalid/fixture/empty\t[no test files]') + sys.exit(0) + if scenario == 'no-results': + sys.exit(0) + if scenario == 'killed-silent': + test_result(name) + sys.exit(137) + if scenario in ('sharded', 'sharded-subtest', 'shard-missing-summary', 'shard-unknown'): + print(f'ok {package} shard 1/2 0s') + print(f'FAIL {package} shard 2/2 0s') + print('=== RUN TestOne') + test_result(name) + if scenario == 'sharded-subtest': test_result('TestOne/child.with+marks') + print('FAIL') + if scenario != 'shard-missing-summary': + print(f'FAIL {package} 2 shards 0s failing: ' + ('unknown' if scenario == 'shard-unknown' else name)) + package_result(pkg='example.invalid/fixture/other', failed=False) + sys.exit(1) + if scenario in ('build', 'setup', 'timeout', 'panic', 'named-panic', 'killed', 'shard'): + test_result(name) + package_result() + prose = {'build': '# example.invalid/fixture/broken\nFAIL\texample.invalid/fixture/broken [build failed]', + 'setup': 'FAIL\texample.invalid/fixture/broken [setup failed]', + 'timeout': 'panic: test timed out after 15m', + 'panic': 'panic: outside a test', 'named-panic': 'panic: test panic', + 'killed': 'signal: killed', 'shard': 'shard-test: lost terminal test results'}[scenario] + print(prose, flush=True) + elif scenario == 'unnamed': + package_result() + package_result(pkg='example.invalid/fixture/other', failed=False) + elif scenario in ('cap', 'cap-five'): + for i in range(6 if scenario == 'cap' else 5): test_result('Test' + str(i)) + package_result() + elif scenario == 'subtest': + test_result('TestOne') + test_result('TestOne/child.with+marks') + test_result('TestOne/passing-sibling', 'PASS') + package_result() + elif scenario == 'mixed': + test_result('TestOne') + test_result('TestTwo') + package_result() + elif scenario == 'multi-package': + # -p 2 buffers each package's output as a block. Repeated test names + # must keep separate owners, with a passing block between the failures. + test_result(name) + package_result() + package_result(pkg='example.invalid/fixture/other', failed=False) + test_result(name) + package_result(pkg='example.invalid/fixture/second') + elif scenario == 'readable-log': + print('plain runner diagnostic', flush=True) + test_result('TestX') + package_result() + else: + print('FIRST FAILURE REMAINS IN THE LOG', flush=True) + test_result(name) + if scenario != 'no-terminal': package_result() + if scenario == 'regression-with-ok': package_result(pkg='example.invalid/fixture/other', failed=False) + sys.exit(1) +assert tool == 'go', tool +assert '-count=1' in sys.argv and '-run' in sys.argv and '-v' in sys.argv, sys.argv +assert '-p' in sys.argv and sys.argv[sys.argv.index('-p') + 1] == '2', sys.argv +assert '-skip' not in ' '.join(sys.argv) +pattern = sys.argv[sys.argv.index('-run') + 1] +assert pattern.startswith('^(') and pattern.endswith(')$'), pattern +package = sys.argv[-1] +if scenario in ('subtest', 'sharded-subtest'): + assert pattern == r'^(TestOne)$/^(child\.with\+marks)$', pattern + name = 'TestOne/child.with+marks' +if scenario == 'mixed' and pattern == '^(TestTwo)$': name = 'TestTwo' +if scenario == 'cap-five': name = pattern[2:-2] +if scenario == 'readable-log': name = 'TestX' +base = '/codeaf-touched-base-' in os.getcwd() +if base and scenario in ('absent-test', 'base-skipped'): + if scenario == 'base-skipped': test_result(name, 'SKIP') + else: print('testing: warning: no tests to run') + print('PASS') + package_result(failed=False) + sys.exit(0) +if base and scenario == 'absent-package': + print('no required module provides package ' + package + '; to add it:') + print('FAIL\t' + package + ' [setup failed]') + sys.exit(1) +if base and scenario == 'base-build': + print('# ' + package) + print('FAIL\t' + package + ' [build failed]') + sys.exit(1) +if not base and scenario == 'head-build': + print('# ' + package) + print('FAIL\t' + package + ' [build failed]') + sys.exit(1) +fails = scenario in ('inherited', 'cap-five', 'regression', 'regression-with-ok', 'multi-package', + 'absent-test', 'absent-package', 'base-build', 'base-skipped') +if scenario == 'mixed': fails = name == 'TestTwo' +if base and scenario in ('regression', 'regression-with-ok', 'multi-package', 'mixed'): fails = False +print('=== RUN ' + name) +if '/' in name: test_result(name.split('/')[0], 'FAIL' if fails else 'PASS') +test_result(name, 'FAIL' if fails else 'PASS') +print('FAIL' if fails else 'PASS') +package_result(pkg=package, failed=fails) +if scenario == 'regression-with-ok': package_result(pkg='example.invalid/fixture/other', failed=False) +sys.exit(1 if fails else 0) +PY +chmod +x "$tmp/bin/stub" +for tool in make go git gh; do ln -s stub "$tmp/bin/$tool"; done +export PATH="$tmp/bin:$PATH" + +run_case() { + local scenario="$1" expected="$2" go_calls="$3" verdict="$4" status + export SCENARIO="$scenario" CASE_DIR="$tmp/$scenario" + mkdir "$CASE_DIR" + export GITHUB_STEP_SUMMARY="$CASE_DIR/summary" + if "$root/scripts/touched-verdict.sh" run --base base-sha --report "$CASE_DIR/report.json" ./pkg >"$CASE_DIR/log" 2>&1; then status=0; else status=$?; fi + if [ "$status" -ne "$expected" ]; then cat "$CASE_DIR/log" >&2; printf '%s: status %s, want %s\n' "$scenario" "$status" "$expected" >&2; exit 1; fi + python3 - "$CASE_DIR" "$go_calls" "$verdict" <<'PY' +import json, sys +from pathlib import Path +directory = Path(sys.argv[1]) +calls = [json.loads(line) for line in (directory / 'calls').read_text().splitlines()] +assert len([call for call in calls if call['tool'] == 'make']) == 1, calls +assert len([call for call in calls if call['tool'] == 'go']) == int(sys.argv[2]), calls +assert sys.argv[3] in (directory / 'log').read_text(), (directory / 'log').read_text() +assert sys.argv[3] in (directory / 'summary').read_text(), (directory / 'summary').read_text() +json.loads((directory / 'report.json').read_text()) +PY +} + +run_case green 0 0 'Every selected test passed' +run_case flaky 0 1 flaky +grep -q 'FIRST FAILURE REMAINS IN THE LOG' "$CASE_DIR/log" +grep -q '::warning::example.invalid/fixture/pkg: TestOne: flaky' "$CASE_DIR/log" +run_case readable-log 0 1 flaky +grep -q '^--- FAIL: TestX (0.00s)$' "$CASE_DIR/log" +grep -q '^plain runner diagnostic$' "$CASE_DIR/log" +if grep -q '{"Action"' "$CASE_DIR/log"; then cat "$CASE_DIR/log" >&2; exit 1; fi +run_case inherited 0 2 'already failing on the base' +run_case regression 1 2 'introduced by this change' +run_case regression-with-ok 1 2 'introduced by this change' +run_case multi-package 1 4 'introduced by this change' +python3 - "$CASE_DIR/report.json" <<'PY' +import json, sys +rows = json.load(open(sys.argv[1]))['tests'] +assert {(row['package'], row['test']) for row in rows} == { + ('example.invalid/fixture/pkg', 'TestOne'), ('example.invalid/fixture/second', 'TestOne')}, rows +PY +grep -q '::error::example.invalid/fixture/pkg: TestOne' "$CASE_DIR/log" +run_case build 1 0 'build failure' +run_case setup 1 0 'package setup failure' +run_case timeout 1 0 'package-level timeout' +run_case panic 1 0 'panic outside a test' +run_case named-panic 1 0 'panic ownership unclear' +run_case killed 1 0 'killed process' +run_case killed-silent 1 0 'killed process' +run_case shard 1 0 'shard runner error' +run_case sharded 0 1 flaky +run_case sharded-subtest 0 1 flaky +run_case shard-missing-summary 1 0 'no terminal summary' +run_case shard-unknown 1 0 'no attributable test name' +run_case no-terminal 1 0 'no terminal package line' +run_case no-results 1 0 'no terminal package result' +run_case head-build 1 1 'head retry could not execute' +run_case unnamed 1 0 'without an attributable test name' +run_case cap 1 0 'exceed the cap of 5' +run_case cap-five 0 10 'already failing on the base' +run_case absent-test 1 2 'absent' +run_case absent-package 1 2 'introduced by this change' +run_case base-build 1 2 'base could not run' +run_case base-skipped 1 2 'skipped' +run_case subtest 0 1 flaky +run_case mixed 1 3 'introduced by this change' + +# API success and failure belong to the aggregate job, after test results are +# fixed. One comment contains every leg's finding; a fork makes no API call. +mkdir "$tmp/reports" +cp "$tmp/flaky/report.json" "$tmp/reports/tui3.json" +cp "$tmp/inherited/report.json" "$tmp/reports/session.json" +export GITHUB_REPOSITORY=example/fixture GITHUB_RUN_ID=42 GITHUB_SHA=head-sha +for scenario in api-error api-malformed existing-issue create-issue fork; do + export SCENARIO="$scenario" CASE_DIR="$tmp/$scenario" TOUCHED_REPORT_ISSUE=true + mkdir "$CASE_DIR" + if [ "$scenario" = fork ]; then TOUCHED_REPORT_ISSUE=false; export TOUCHED_REPORT_ISSUE; fi + "$root/scripts/touched-verdict.sh" report-issues "$tmp/reports" >"$CASE_DIR/log" 2>&1 + python3 - "$CASE_DIR" "$scenario" <<'PY' +import json, sys +from pathlib import Path +directory, scenario = Path(sys.argv[1]), sys.argv[2] +if scenario == 'fork': + assert not (directory / 'calls').exists() +else: + calls = [json.loads(line) for line in (directory / 'calls').read_text().splitlines()] + if scenario in ('api-error', 'api-malformed'): + assert '::warning::' in (directory / 'log').read_text() + else: + comments = [call for call in calls if call['args'][:2] == ['issue', 'comment']] + assert len(comments) == 1, calls + assert comments[0]['args'][2] == ('8' if scenario == 'create-issue' else '7'), calls + body = (directory / 'issue-body').read_text() + assert 'flaky' in body and 'already failing on the base' in body, body + assert '/actions/runs/42' in body and 'head-sha' in body, body + assert not any('close' in call['args'] for call in calls), calls +PY +done +printf 'touched-verdict acceptance: ok\n'