From e0f466afedaa9452705277293598fe5141dc8b6f Mon Sep 17 00:00:00 2001 From: Maximilian Date: Fri, 24 Apr 2026 15:36:50 +0200 Subject: [PATCH 01/18] docs: design PR-gated v2 drop-in compat testing Completes the "diffs against goldens" step the 2026-04-23 local-CI unification spec anticipated but never implemented. Goldens captured once on ubuntu-24.04 x86_64 / PHP 8.4, committed to test/compat/testdata/golden/v2/. Per-PR compat-diff runs inside the existing noble/amd64/8.4 pipeline cell; deviations fail the cell and a compat-report job posts (or deletes) a sticky PR comment that surfaces the on-demand golden-refresh escape hatch. A weekly workflow opens a drift PR when v2 or the Ondrej PPA shifts. No separate compat workflow, no v2 install at PR time. --- .../2026-04-24-pr-gated-v2-compat-design.md | 412 ++++++++++++++++++ 1 file changed, 412 insertions(+) create mode 100644 docs/superpowers/specs/2026-04-24-pr-gated-v2-compat-design.md diff --git a/docs/superpowers/specs/2026-04-24-pr-gated-v2-compat-design.md b/docs/superpowers/specs/2026-04-24-pr-gated-v2-compat-design.md new file mode 100644 index 0000000..4be084f --- /dev/null +++ b/docs/superpowers/specs/2026-04-24-pr-gated-v2-compat-design.md @@ -0,0 +1,412 @@ +# PR-gated v2 Drop-in Compat Testing — Design + +**Status:** Draft — 2026-04-24 +**Scope:** Close out the "diffs against goldens" step the 2026-04-23 local-CI unification spec anticipated but did not implement. + +## Context + +`buildrush/setup-php` is a from-scratch reimplementation of `shivammathur/setup-php` +optimized for fast, reproducible PHP setup via prebuilt OCI bundles. Its +explicit goal is **drop-in compatibility** with `shivammathur/setup-php@v2`: +same input shape (`php-version`, `extensions`, `ini-values`, `coverage`, +`ini-file`), semantically-matching output for realistic workflows. + +The 2026-04-23 local-CI unification spec collapsed all prior +`compat-harness.yml`/`integration-test.yml`/`plan-and-build.yml` +orchestration into a single `phpup test` runner exercised identically +locally (`make ci-cell`) and in CI (`ci.yml::pipeline`). The +unification spec **allocated** the following pieces for drop-in compat: + +- `phpup internal test-cell` iterates fixtures and runs `probe.sh`. +- Fixtures stay in `test/compat/fixtures.yaml`. +- "Goldens in `test/compat/testdata/`." +- "Diffs against goldens." + +Everything except the last bullet was wired up in PR 3 of the unification +rollout (July 2026 bundle). The "diffs against goldens" step — the +actual compat gate — was never implemented. Today: + +- `test-cell` runs `probe.sh` per fixture and parses the JSON, but + compares it only against **internal invariants** (our own shape/presence + checks); nothing in the code path ever loads a v2-baseline golden. +- `phpup compat-diff` exists as a subcommand (from the PR-6 consolidation) + with a tested Go API (`internal/compatdiff.diffProbes`), but it is not + called from anywhere in the pipeline. +- `docs/compat-matrix.md` carries the pinned v2 SHA + (`accd6127cb78bee3e8082180cb391013d204ef9f`), the pinned PPA snapshot + date (`2026-04-11`), and a populated deviations allowlist inside + `` markers. + +The previously-removed `.github/workflows/compat-harness.yml` (deleted in +PR #60 of the unification rollout) ran **both** actions in every cell +and diffed their probes live. That was the right idea with the wrong +delivery shape: it coupled PR-time CI to live apt + PPA state, ran 4×N +jobs per PR, and induced cross-action PATH/PHPRC collisions. This design +keeps the original goal (a PR-time compat gate against a pinned v2 +baseline) while eliminating the live-v2-install anti-pattern from every +PR. + +## Goal + +On every PR, detect any change that makes `buildrush/setup-php` behave +differently from `shivammathur/setup-php@v2` on a canonical set of +drop-in scenarios — without running v2 at PR time. + +## Non-goals + +- Running `shivammathur/setup-php` on PRs. The goldens are captured + offline and committed. +- Matching every quirk of v2 in every axis (PHP version × arch × OS × + extension set). The canonical cell is one environment; broader sweeps + are out of scope (see §10). +- A new binary or standalone workflow for compat. Compat lives inside + `phpup internal test-cell`, run identically locally and in CI by the + existing `make ci-cell` / `ci.yml::pipeline` path. + +## Architecture + +### Framing + +This design is **completion of unfinished work in the 2026-04-23 +local-CI-unification spec**, not a new subsystem. Every building block +exists except the wire-up from `test-cell` to `phpup compat-diff` and the +committed JSON files it reads. + +``` +┌──────────────────────────────────────────────────────────────────────┐ +│ PR-time flow (existing) │ +│ │ +│ ci.yml::pipeline matrix (2 OS × 2 arch × 5 PHP = 20 cells) │ +│ │ │ +│ └─▶ for each cell: make ci-cell OS=.. ARCH=.. PHP=.. │ +│ │ │ +│ └─▶ phpup test │ +│ │ │ +│ └─▶ docker run bare-ubuntu │ +│ │ │ +│ └─▶ phpup internal test-cell │ +│ │ │ +│ ├─ phpup install │ +│ ├─ probe.sh │ +│ ├─ assertInvariants │ +│ └─ NEW: compatDiff(...) │ +│ │ +└──────────────────────────────────────────────────────────────────────┘ +``` + +The compat-diff step is a no-op on cells where the +`(os, arch, php)` tuple ≠ `(noble, x86_64, 8.4)` or where the fixture +doesn't carry `compat: true`. Exactly one cell (noble/amd64/8.4) × eight +fixtures performs comparisons; the other nineteen cells skip compat and +exercise internal invariants only. + +### Canonical cell + +Goldens are captured on one environment: **Ubuntu 24.04 (noble), x86_64, +PHP 8.4**. This matches v2's canonical target (`ubuntu-latest` = +`ubuntu-24.04` on amd64) and keeps the golden count minimal. + +### Golden fixtures (eight) + +Axis-representative subset of `test/compat/fixtures.yaml`: + +| Fixture | Axis exercised | +| --- | --- | +| `bare` | empty extensions, no ini-values, coverage: none | +| `exclusion` | `:opcache` removal token | +| `single-ext` | one shared extension (`redis`) | +| `multi-ext` | two extensions (`redis, xdebug`) | +| `none-reset` | `none, redis` — `none` hoisted, ext re-added | +| `coverage-pcov` | `coverage: pcov` auto-disable-xdebug + install | +| `ini-and-coverage` | `ini-values` merged with `coverage: xdebug` | +| `ini-file-development` | `ini-file: development` base | + +All eight already exist and already run in the noble/amd64/8.4 pipeline +cell. No fixture additions are required; a new `compat: true` flag is +added to these eight entries to opt them into the gate. The flag is +absent (false) on all other fixtures. + +## Components + +| Component | Location | Status | +| --- | --- | --- | +| Committed goldens | `test/compat/testdata/golden/v2/.json` | NEW — 8 files | +| Compat-diff invocation | `internal/testsuite/testcell.go:runFixture` after `assertFixtureInvariants` | MODIFY | +| Golden-presence gate | Inside `runFixture`; skip unless canonical cell AND fixture has `compat: true` AND golden file exists | NEW logic | +| Deviation artifact writer | `test-cell` writes `./deviations/.json` on compat-diff exit 1 | NEW | +| `compat-report` job | `.github/workflows/ci.yml` | NEW job, `if: failure()` | +| Golden-refresh workflow | `.github/workflows/compat-golden-refresh.yml` | NEW workflow | +| Makefile capture target | `make compat-refresh-goldens` | NEW target | +| Capture subcommand | `phpup internal golden-capture` | NEW under `internal/testsuite/` | +| Fixture `compat` field | `test/compat/fixtures.yaml` on the 8 elected entries | NEW yaml field | + +No new Go packages; capture reuses `internal/testsuite/`. The in-cell +compat-diff call invokes the `compatdiff` package **directly** (Go API, +not a shell-out) for speed and error clarity. The existing internal +helper `diffProbes` is unexported; the implementation plan will either +export it, or add a narrow public wrapper, to expose it to +`testsuite`. The existing `phpup compat-diff` subcommand stays as the +CLI surface for offline debugging and does not change. + +## Data flow + +### PR-time (in-cell) + +``` +runFixture(f): + probeJSON = probe.sh(f) + assertInvariants(probeJSON) # existing + if (cellOS, cellArch, cellPHP) == ("noble","x86_64","8.4") + and f.Compat + and exists("test/compat/testdata/golden/v2/"+f.Name+".json"): + ours = probeJSON + theirs = read("test/compat/testdata/golden/v2/"+f.Name+".json") + al = loadAllowlist("docs/compat-matrix.md") + devs = compatdiff.(ours, theirs, al, f.Name) + if devs non-empty: + appendDeviationsArtifact("./deviations/noble-x86_64-8.4.json", f.Name, devs) + fixture.Fail(devs) +``` + +### CI-time (Shape Z consolidation) + +``` +pipeline matrix: + noble/x86_64/8.4 cell: + - on compat fail: uploads deviations-noble-x86_64-8.4.json + - on compat OK: uploads nothing +compat-report (if: always() && needs.pipeline.result != 'skipped'): + download deviations-* artifacts (may be zero) + find existing sticky comment by + if deviations found: + render markdown body (§7 template) + post-or-update sticky comment + else if existing sticky comment present: + delete it (prior red PR is now green — clear stale state) + else: + no-op +``` + +The `compat-report` job is pull-request-scoped (gated on +`github.event_name == 'pull_request'`); it doesn't run on push-to-main +because there is no PR to comment on. Its gate is +`always() && needs.pipeline.result != 'skipped'` so it runs whether +`pipeline` passed or failed, deleting stale comments when deviations +clear between pushes. + +### Refresh-time (weekly + on-demand) + +``` +compat-golden-refresh.yml (on schedule + workflow_dispatch): + sanity: + assert matrix.PinnedSHA == workflow.pinnedSHA + matrix over the 8 compat fixtures, runs-on: ubuntu-24.04: + - uses: shivammathur/setup-php@ + with: fixture.inputs + - bash test/compat/probe.sh → captured-.json + - upload artifact + aggregate: + download artifacts into test/compat/testdata/golden/v2/ + if git diff --quiet: exit 0 (record "no drift") + else: + create branch `chore/compat-golden-refresh-` + open PR via peter-evans/create-pull-request + body = "Per-fixture key-level deltas:\n ..." +``` + +## Refresh workflow specifics + +**File:** `.github/workflows/compat-golden-refresh.yml` + +**Triggers:** +- `schedule: cron: '0 12 * * MON'` — weekly Monday 12:00 UTC. +- `workflow_dispatch:` — on-demand refresh. This is the + escape-hatch the PR-comment template points at. + +**Pinning discipline:** The workflow uses the v2 SHA literally listed in +`docs/compat-matrix.md` under `## Pinning`. A pre-run sanity step reads +the markdown-pinned SHA and refuses to run if it doesn't match the +`env.PINNED_V2_SHA` value embedded in the workflow file (belt + +braces). Bumping the pinned SHA is a deliberate manual edit to the +matrix document plus the workflow env value — not a thing the refresh +workflow does on its own. + +**Refresh PR body:** generated by an aggregation step. Lists for each +fixture whose golden JSON changed: the fixture name, the diff of +top-level JSON paths (`extensions`, each `ini.*` key, `env_delta`, +`path_additions`), and whether the allowlist already tolerates those +paths. If the allowlist already covers the diff (i.e., the drift would +not have failed PR-time compat-diff), the PR body flags that explicitly +so a reviewer can decide whether to merge the refresh or leave it — +both outcomes are safe. + +**Isolation:** the refresh workflow runs in its own GitHub-hosted +ephemeral runner, uses `shivammathur/setup-php@` which is the only +place in the repo that installs v2. No artifact from the refresh job +ever feeds into PR-time CI directly; the only handoff is the committed +JSON files on the drift PR's branch. + +## PR comment on compat-diff failure + +A sticky comment, created-or-updated by +`peter-evans/create-or-update-comment` keyed on the header +``. Body template: + +```markdown +## ⚠️ Compat-diff detected deviations from shivammathur/setup-php@v2 + + + +This PR's `phpup install` behavior diverges from the pinned v2 baseline +in ways the compat-matrix allowlist doesn't cover. + +### Deviations + +**Fixture `multi-ext` (noble/amd64/8.4)** +- `ini.xdebug.mode`: ours=`coverage`, theirs=`develop,coverage` +- `extensions`: ours=[redis, xdebug, ...], theirs=[xdebug, ...] + + + +### What to do + +1. **If this is a regression in your change** — fix it, or add an entry + to the deviations allowlist in + [`docs/compat-matrix.md`](../blob/main/docs/compat-matrix.md) with a + `reason:` explaining why it's intentional. The YAML block is + delimited by `` / + ``. + +2. **If this looks like upstream v2 drift** (e.g., the PPA shipped a + new package between golden refreshes) — ask a maintainer to + force-refresh the goldens: + + gh workflow run compat-golden-refresh.yml + + Merge the resulting drift PR (`chore: refresh v2 compat goldens`), + then rebase this PR onto `main` to pick up the new goldens. + +Goldens pinned to v2 SHA `accd6127…` + PPA snapshot `2026-04-11`. Last +refreshed: . +``` + +The comment is deterministic: re-running a failing PR re-posts the +same content (sticky-key match). A PR that moves from red→green on the +compat gate has its comment **deleted** by the `compat-report` job (it +runs `if: always()` on every pipeline completion, not only on +failure; see §5 data flow), so the PR no longer carries stale +deviation text. + +## Failure policy + +**Hard fail with an on-demand refresh escape hatch** (Shape 2 of the +brainstorm). + +- Compat-diff returns exit code 1 ⇒ fixture fails ⇒ cell fails ⇒ + `pipeline` job fails ⇒ PR is red. The PR can only merge by + either (a) removing the regression from the code, or (b) adding an + allowlist entry to `docs/compat-matrix.md` in the same PR with a + `reason:` (reviewable in the diff). +- Exit code 2 (malformed input or missing golden) ⇒ treat as a bug in + the test machinery, surface with a clear error, still fail the cell. + This prevents a silent skip if a golden is somehow deleted or + corrupted. +- The PR comment explicitly surfaces the on-demand refresh path so a + developer whose PR is red because of upstream v2 drift (between + weekly scheduled refreshes) can recover without maintainer intuition: + run the workflow, merge the drift PR, rebase. + +## Testing + +- **Unit tests, `internal/testsuite`:** + - Golden-presence gate: canonical cell + non-canonical cell both run + fixtures; only the canonical cell invokes compat-diff. + - Compat-diff wiring: `runFixture` with stubbed probe + stubbed + golden + the real allowlist parser → assert exit codes and + deviation-artifact contents. + - Deviation-artifact writer: fail twice across two fixtures, one + artifact file collects both. +- **Unit tests, `internal/testsuite/golden_capture.go`:** + - Uses the existing `stub-php.sh` to simulate `shivammathur/setup-php` + output. Verifies probe shape matches what the PR-time path produces. +- **End-to-end smoke (added to `make check`):** + - `make ci-cell OS=noble ARCH=x86_64 PHP=8.4` must show all 8 compat + fixtures passing (`compat=OK`). If goldens are missing the cell + must fail fast with an actionable error pointing at + `make compat-refresh-goldens`. + - `make ci-cell OS=jammy ARCH=aarch64 PHP=8.1` must show the fixtures + passing with `compat=SKIP (non-canonical cell)` — compat is + invisible to non-canonical cells. +- **Manual verification (pre-merge):** + - Push a branch with an intentional deviation (e.g., add a random + ini-value default), observe the `compat-report` job posts a + correctly-formatted sticky comment. + - Push a follow-up commit removing the deviation, observe the same + `compat-report` job **deletes** the sticky comment (no stale + "deviations detected" text left behind on a now-green PR). + - Dispatch `compat-golden-refresh` workflow manually, observe the + drift PR opens with reasonable body (or exits cleanly on no drift). + +## Critical files + +**New:** +- `.github/workflows/compat-golden-refresh.yml` +- `test/compat/testdata/golden/v2/bare.json` +- `test/compat/testdata/golden/v2/exclusion.json` +- `test/compat/testdata/golden/v2/single-ext.json` +- `test/compat/testdata/golden/v2/multi-ext.json` +- `test/compat/testdata/golden/v2/none-reset.json` +- `test/compat/testdata/golden/v2/coverage-pcov.json` +- `test/compat/testdata/golden/v2/ini-and-coverage.json` +- `test/compat/testdata/golden/v2/ini-file-development.json` +- `internal/testsuite/golden_capture.go` + `_test.go` +- `Makefile` target `compat-refresh-goldens` + +**Modified:** +- `.github/workflows/ci.yml` — add `compat-report` job after `pipeline` +- `internal/testsuite/testcell.go` — compat-diff call in `runFixture`, + deviation-artifact writer, per-fixture outcome now carries compat + status +- `internal/testsuite/fixtures.go` — `Compat bool` field on `Fixture` +- `test/compat/fixtures.yaml` — `compat: true` on the 8 elected entries +- `cmd/phpup/main.go` — register `phpup internal golden-capture` under + the existing `internal` dispatcher + +**No deletions.** + +## Verification + +- **Per-fixture:** the eight committed goldens are byte-identical to + the output of running `shivammathur/setup-php@` on + `ubuntu-24.04` × amd64 × PHP 8.4 with the fixture's inputs, piped + through `test/compat/probe.sh`. Reproducible by running + `make compat-refresh-goldens` and observing no git diff. +- **Per-PR (positive):** a no-op PR (e.g., README typo) runs the full + pipeline with the compat-diff step succeeding on all 8 fixtures. +- **Per-PR (negative):** an intentional regression (e.g., bump a + default ini value) produces a red PR with the sticky comment + listing the divergent keys. +- **Refresh workflow (positive):** a workflow_dispatch with no v2/PPA + drift completes with `git diff --quiet` and records "no drift" in the + job summary. +- **Refresh workflow (negative):** a workflow_dispatch after a + deliberate SHA bump in `docs/compat-matrix.md` + the workflow env + produces a drift PR with an accurate body. + +## Open questions / future work + +- **Broader sweep.** The 8-fixture / 1-cell PR gate explicitly does not + cover arm64, jammy, PHP 8.1–8.3/8.5, or the extension-heavy + `multi-ext-top10`/`multi-ext-hard4` fixtures. If arm64-only + regressions become a pattern, expand the canonical cell set (likely + adding `noble/arm64/8.4` as a second compat cell, capturing its own + goldens with the `opcache.jit_buffer_size: 128M` divergence naturally + encoded). Mechanism extends without redesign. +- **Allowlist UX.** If compat-diff false-positives become a PR-time + annoyance (expected: rare, since the allowlist is already populated + from the previous harness era), consider a `phpup internal compat- + diff --propose-allowlist-entry` helper that prints a YAML snippet a + developer can paste into the matrix. +- **Golden compression.** Eight JSON files × O(2-5KB) each = small + enough to ignore for now. If the fixture count grows 10× the + committed JSONs can migrate to a single consolidated + `golden/v2.json` map without changing the compat-diff API. From c51ccd8a598bd5aa7f84afc4c103f1a1f39cc24a Mon Sep 17 00:00:00 2001 From: Maximilian Date: Fri, 24 Apr 2026 15:55:01 +0200 Subject: [PATCH 02/18] feat(testsuite): add Fixture.Compat flag and mark 8 axis fixtures --- internal/testsuite/fixtures.go | 8 ++++++ internal/testsuite/fixtures_test.go | 39 +++++++++++++++++++++++++++++ test/compat/fixtures.yaml | 8 ++++++ 3 files changed, 55 insertions(+) diff --git a/internal/testsuite/fixtures.go b/internal/testsuite/fixtures.go index ebec2f7..ba2dda5 100644 --- a/internal/testsuite/fixtures.go +++ b/internal/testsuite/fixtures.go @@ -36,6 +36,14 @@ type Fixture struct { Arch string `yaml:"arch,omitempty"` // RunnerOS is optional; e.g. "ubuntu-22.04". Empty means any OS. RunnerOS string `yaml:"runner_os,omitempty"` + // Compat, when true, opts this fixture into the PR-time drop-in-v2 + // compat-diff gate: the runner compares the fixture's probe output + // against a committed v2 golden at + // test/compat/testdata/golden/v2/.json. The comparison runs + // only in the canonical cell (noble/x86_64/8.4); non-canonical + // cells skip compat-diff even when this is true. Absent == false. + // See docs/superpowers/specs/2026-04-24-pr-gated-v2-compat-design.md. + Compat bool `yaml:"compat,omitempty"` } // FixtureSet is the top-level document shape of test/compat/fixtures.yaml. diff --git a/internal/testsuite/fixtures_test.go b/internal/testsuite/fixtures_test.go index 843142d..cafc363 100644 --- a/internal/testsuite/fixtures_test.go +++ b/internal/testsuite/fixtures_test.go @@ -179,3 +179,42 @@ func TestFilter_EmptyResult_NoMatch(t *testing.T) { t.Fatalf("Filter returned %d fixtures for no-match PHP version; want 0", len(got)) } } + +// TestLoad_CompatField asserts that each axis-representative fixture +// listed in docs/superpowers/specs/2026-04-24-pr-gated-v2-compat-design.md +// is marked compat: true in the checked-in test/compat/fixtures.yaml. +// The compat layer keys off this field to decide whether to diff a +// fixture's probe against the committed v2 golden. +func TestLoad_CompatField(t *testing.T) { + path := filepath.Join("..", "..", "test", "compat", "fixtures.yaml") + set, err := Load(path) + if err != nil { + t.Fatalf("Load(%q): %v", path, err) + } + want := map[string]bool{ + "bare": true, + "exclusion": true, + "single-ext": true, + "multi-ext": true, + "none-reset": true, + "coverage-pcov": true, + "ini-and-coverage": true, + "ini-file-development": true, + } + got := map[string]bool{} + for _, f := range set.Fixtures { + if f.Compat { + got[f.Name] = true + } + } + for name := range want { + if !got[name] { + t.Errorf("fixture %q: want Compat=true, got false", name) + } + } + for name := range got { + if !want[name] { + t.Errorf("fixture %q: Compat=true set unexpectedly (design scope is 8 fixtures)", name) + } + } +} diff --git a/test/compat/fixtures.yaml b/test/compat/fixtures.yaml index 1300c55..fa7e097 100644 --- a/test/compat/fixtures.yaml +++ b/test/compat/fixtures.yaml @@ -10,24 +10,28 @@ fixtures: extensions: '' ini-values: '' coverage: 'none' + compat: true - name: exclusion php-version: '8.4' extensions: ':opcache' ini-values: '' coverage: 'none' + compat: true - name: coverage-pcov php-version: '8.4' extensions: '' ini-values: '' coverage: 'pcov' + compat: true - name: ini-and-coverage php-version: '8.4' extensions: '' ini-values: 'memory_limit=256M,date.timezone=UTC' coverage: 'xdebug' + compat: true - name: ini-file-development php-version: '8.4' @@ -35,24 +39,28 @@ fixtures: ini-values: '' coverage: 'none' ini-file: 'development' + compat: true - name: multi-ext php-version: '8.4' extensions: 'redis, xdebug' ini-values: '' coverage: 'none' + compat: true - name: none-reset php-version: '8.4' extensions: 'none, redis' ini-values: '' coverage: 'none' + compat: true - name: single-ext php-version: '8.4' extensions: 'redis' ini-values: '' coverage: 'none' + compat: true - name: bare-85 php-version: '8.5' From 7a3ac6da9c16e86763a9eba9795c796b25f26fdb Mon Sep 17 00:00:00 2001 From: Maximilian Date: Fri, 24 Apr 2026 16:04:22 +0200 Subject: [PATCH 03/18] feat(compatdiff): expose DiffFiles + Deviation for in-process use --- internal/compatdiff/public.go | 44 ++++++++++++++++++++++++++ internal/compatdiff/public_test.go | 51 ++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 internal/compatdiff/public.go create mode 100644 internal/compatdiff/public_test.go diff --git a/internal/compatdiff/public.go b/internal/compatdiff/public.go new file mode 100644 index 0000000..56961bf --- /dev/null +++ b/internal/compatdiff/public.go @@ -0,0 +1,44 @@ +package compatdiff + +// Deviation is a single path-level divergence between two probes after +// allowlist processing. Exported counterpart of the internal diffEntry +// type; used by internal/testsuite.runFixture to record structured +// failure data for the PR-time deviation artifact. +type Deviation struct { + Path string + Ours string + Theirs string +} + +// DiffFiles loads two probe JSON files and an allowlist markdown file, +// runs the same comparison as the `phpup compat-diff` CLI, and returns +// any deviations that are neither identical nor allowlist-tolerated. +// +// Errors: +// - reading or parsing either probe -> non-nil error +// - reading or parsing the allowlist -> non-nil error +// - invalid allowlist entries (bad kind/empty path) -> non-nil error +// +// A nil error with an empty returned slice means "no deviations". +// A nil error with a non-empty slice means "deviations found" — the +// caller decides how to surface them. +func DiffFiles(oursPath, theirsPath, allowlistPath, fixture string) ([]Deviation, error) { + ours, err := readProbe(oursPath) + if err != nil { + return nil, err + } + theirs, err := readProbe(theirsPath) + if err != nil { + return nil, err + } + al, err := loadAllowlist(allowlistPath) + if err != nil { + return nil, err + } + entries := diffProbes(&ours, &theirs, al, fixture) + devs := make([]Deviation, len(entries)) + for i, e := range entries { + devs[i] = Deviation(e) + } + return devs, nil +} diff --git a/internal/compatdiff/public_test.go b/internal/compatdiff/public_test.go new file mode 100644 index 0000000..343b02f --- /dev/null +++ b/internal/compatdiff/public_test.go @@ -0,0 +1,51 @@ +package compatdiff + +import ( + "path/filepath" + "testing" +) + +// TestDiffFiles_NoDeviations verifies DiffFiles returns an empty slice +// and nil error when ours == theirs byte-for-byte and the allowlist +// contains no matching rules. +func TestDiffFiles_NoDeviations(t *testing.T) { + bare := filepath.Join("testdata", "probe-bare.json") + empty := filepath.Join("testdata", "compat-matrix-empty.md") + devs, err := DiffFiles(bare, bare, empty, "bare") + if err != nil { + t.Fatalf("DiffFiles: %v", err) + } + if len(devs) != 0 { + t.Fatalf("DiffFiles: want 0 deviations, got %d: %+v", len(devs), devs) + } +} + +// TestDiffFiles_ReportsDeviations checks that divergent probes produce +// a non-empty []Deviation whose fields match the internal diffEntry shape. +func TestDiffFiles_ReportsDeviations(t *testing.T) { + ours := filepath.Join("testdata", "probe-bare.json") + theirs := filepath.Join("testdata", "probe-bare-ini-shift.json") + empty := filepath.Join("testdata", "compat-matrix-empty.md") + devs, err := DiffFiles(ours, theirs, empty, "bare") + if err != nil { + t.Fatalf("DiffFiles: %v", err) + } + if len(devs) == 0 { + t.Fatalf("DiffFiles: want at least 1 deviation, got 0") + } + for _, d := range devs { + if d.Path == "" { + t.Errorf("Deviation.Path empty: %+v", d) + } + } +} + +// TestDiffFiles_InvalidProbeSurfacesError confirms a broken probe path +// (missing file) returns a non-nil error (not a silent empty diff). +func TestDiffFiles_InvalidProbeSurfacesError(t *testing.T) { + empty := filepath.Join("testdata", "compat-matrix-empty.md") + _, err := DiffFiles("/nonexistent/ours.json", "/nonexistent/theirs.json", empty, "bare") + if err == nil { + t.Fatalf("DiffFiles: want error for missing probe files, got nil") + } +} From 14d5a113e3919849cbf0d474d9afaf6d5db544da Mon Sep 17 00:00:00 2001 From: Maximilian Date: Fri, 24 Apr 2026 16:12:40 +0200 Subject: [PATCH 04/18] feat(testsuite): mount compat-matrix.md + goldens dir into test-cell --- internal/testsuite/runner.go | 31 +++++++++-- internal/testsuite/runner_test.go | 87 +++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 3 deletions(-) diff --git a/internal/testsuite/runner.go b/internal/testsuite/runner.go index eaeafc4..ca1a84e 100644 --- a/internal/testsuite/runner.go +++ b/internal/testsuite/runner.go @@ -310,10 +310,13 @@ func runCell(ctx context.Context, opts *testOpts, set *FixtureSet, cellOS, cellA // remote URI, returns an error because mounting a remote registry inside // the test container isn't wired up yet (PR 4 scope). // -// Three mounts are produced: +// Mounts produced (in order): // - oci-layout directory → /registry (read-only) -// - phpup self-binary → /usr/local/bin/phpup (read-only) -// - repo's test/compat → /test-compat (read-only) +// - phpup self-binary → /usr/local/bin/phpup (read-only) +// - repo's test/compat → /test-compat (read-only) +// - lockfile override → /tmp/bundles-override.lock (read-only) +// - docs/compat-matrix.md → /compat-matrix.md (read-only; REQUIRED, errors if missing) +// - goldens dir → /golden (read-only; OPTIONAL, silently skipped if dir absent) // // Env is minimal: PHPUP_REGISTRY=oci-layout:/registry so the inner // phpup invocation inherits the right registry without needing to pass @@ -360,6 +363,28 @@ func buildCellMounts(opts *testOpts, _, _, _ string) ([]build.Mount, map[string] "PHPUP_REGISTRY": "oci-layout:/registry", "PHPUP_LOCKFILE": "/tmp/bundles-override.lock", } + + // Compat-diff inputs: matrix markdown (for allowlist parsing) and the + // goldens directory (for byte-for-byte comparison against v2). Matrix + // is a repo-integrity requirement; goldens dir is optional — missing + // dir means a fresh checkout that hasn't run the refresh workflow + // yet. The fixture-level compat-diff gate in runFixture (added in a + // later PR of feat/pr-gated-v2-compat) will handle per-fixture-missing + // cases with an actionable error. + compatMatrix := filepath.Join(opts.AbsRepo, "docs", "compat-matrix.md") + if _, err := os.Stat(compatMatrix); err != nil { + return nil, nil, fmt.Errorf("docs/compat-matrix.md missing under repo: %w", err) + } + mounts = append(mounts, + build.Mount{Host: compatMatrix, Container: "/compat-matrix.md", ReadOnly: true}, + ) + goldenDir := filepath.Join(opts.AbsRepo, "test", "compat", "testdata", "golden") + if _, err := os.Stat(goldenDir); err == nil { + mounts = append(mounts, + build.Mount{Host: goldenDir, Container: "/golden", ReadOnly: true}, + ) + } + return mounts, env, nil } diff --git a/internal/testsuite/runner_test.go b/internal/testsuite/runner_test.go index 49c34cb..3f4d705 100644 --- a/internal/testsuite/runner_test.go +++ b/internal/testsuite/runner_test.go @@ -15,6 +15,7 @@ import ( // writeTestsuiteFixture creates a minimal repo layout under dir with: // - test/compat/fixtures.yaml (a small synthetic FixtureSet) // - test/compat/probe.sh (empty but exists) +// - docs/compat-matrix.md (compat matrix stub, required by buildCellMounts) // - out/oci-layout/ (empty dir to pass stat check) // - a fake "phpup" binary file (content doesn't matter — only presence is checked) // @@ -42,6 +43,7 @@ func writeTestsuiteFixture(t *testing.T, dir string) string { coverage: none `, 0o644) mustWrite("test/compat/probe.sh", "#!/bin/bash\n", 0o755) + mustWrite("docs/compat-matrix.md", "# compat matrix stub\n", 0o644) if err := os.MkdirAll(filepath.Join(dir, "out", "oci-layout"), 0o750); err != nil { t.Fatal(err) } @@ -217,3 +219,88 @@ func TestCellSummary_FormatsOK(t *testing.T) { } } } + +// TestBuildCellMounts_MountsCompatMatrix asserts that the compat-matrix +// markdown doc and the goldens directory (when it exists) are mounted +// read-only into the test-cell container so compatdiff.DiffFiles can +// read them via their container-side paths. The matrix file is a hard +// requirement (error if missing); the goldens dir is optional (absent +// = degraded compat-skip mode in runFixture). +func TestBuildCellMounts_MountsCompatMatrix(t *testing.T) { + dir := t.TempDir() + fakeBinary := writeTestsuiteFixture(t, dir) + + // writeTestsuiteFixture already wrote docs/compat-matrix.md. + matrixPath := filepath.Join(dir, "docs", "compat-matrix.md") + goldenDir := filepath.Join(dir, "test", "compat", "testdata", "golden") + if err := os.MkdirAll(goldenDir, 0o755); err != nil { + t.Fatalf("mkdir golden: %v", err) + } + + opts := testOptsWith(t, dir, fakeBinary, []string{"noble"}, []string{"x86_64"}, []string{"8.4"}) + mounts, _, err := buildCellMounts(opts, "noble", "x86_64", "8.4") + if err != nil { + t.Fatalf("buildCellMounts: %v", err) + } + var sawMatrix, sawGoldens bool + for _, m := range mounts { + if m.Host == matrixPath && m.Container == "/compat-matrix.md" && m.ReadOnly { + sawMatrix = true + } + if m.Host == goldenDir && m.Container == "/golden" && m.ReadOnly { + sawGoldens = true + } + } + if !sawMatrix { + t.Errorf("buildCellMounts did not mount docs/compat-matrix.md -> /compat-matrix.md (ro); mounts=%+v", mounts) + } + if !sawGoldens { + t.Errorf("buildCellMounts did not mount test/compat/testdata/golden -> /golden (ro); mounts=%+v", mounts) + } +} + +// TestBuildCellMounts_MissingCompatMatrix asserts that buildCellMounts +// returns an error (not silently skips) when docs/compat-matrix.md is +// absent from the repo. This is a repo-integrity signal: any checkout +// in which that file is missing should fail loudly. +func TestBuildCellMounts_MissingCompatMatrix(t *testing.T) { + dir := t.TempDir() + fakeBinary := writeTestsuiteFixture(t, dir) + // Remove docs/compat-matrix.md that writeTestsuiteFixture created. + matrixPath := filepath.Join(dir, "docs", "compat-matrix.md") + if err := os.Remove(matrixPath); err != nil { + t.Fatalf("remove compat-matrix.md: %v", err) + } + + opts := testOptsWith(t, dir, fakeBinary, []string{"noble"}, []string{"x86_64"}, []string{"8.4"}) + _, _, err := buildCellMounts(opts, "noble", "x86_64", "8.4") + if err == nil { + t.Fatalf("buildCellMounts: want error for missing compat-matrix.md, got nil") + } + if !strings.Contains(err.Error(), "compat-matrix.md") { + t.Errorf("error message should mention compat-matrix.md; got %v", err) + } +} + +// TestBuildCellMounts_GoldensDirOptional asserts that an absent +// goldens directory is NOT an error — it's the degraded mode that +// a fresh checkout (pre-refresh-workflow) sits in. Compat-diff +// skips later in runFixture when the golden file is missing. +func TestBuildCellMounts_GoldensDirOptional(t *testing.T) { + dir := t.TempDir() + fakeBinary := writeTestsuiteFixture(t, dir) + + // writeTestsuiteFixture already wrote docs/compat-matrix.md. + // Deliberately do NOT create test/compat/testdata/golden. + + opts := testOptsWith(t, dir, fakeBinary, []string{"noble"}, []string{"x86_64"}, []string{"8.4"}) + mounts, _, err := buildCellMounts(opts, "noble", "x86_64", "8.4") + if err != nil { + t.Fatalf("buildCellMounts (no goldens dir): %v", err) + } + for _, m := range mounts { + if m.Container == "/golden" { + t.Errorf("no /golden mount expected when goldens dir is absent; got %+v", m) + } + } +} From 053a610d818cec30a1a8b8d7e16af36a6e06bee7 Mon Sep 17 00:00:00 2001 From: Maximilian Date: Fri, 24 Apr 2026 16:23:33 +0200 Subject: [PATCH 05/18] feat(testsuite): per-cell deviation artifact writer --- internal/testsuite/deviations.go | 76 +++++++++++++++++++++ internal/testsuite/deviations_test.go | 95 +++++++++++++++++++++++++++ 2 files changed, 171 insertions(+) create mode 100644 internal/testsuite/deviations.go create mode 100644 internal/testsuite/deviations_test.go diff --git a/internal/testsuite/deviations.go b/internal/testsuite/deviations.go new file mode 100644 index 0000000..7d6782e --- /dev/null +++ b/internal/testsuite/deviations.go @@ -0,0 +1,76 @@ +package testsuite + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + + "github.com/buildrush/setup-php/internal/compatdiff" +) + +// DeviationsFile is the on-disk shape of the per-cell deviation artifact +// uploaded by ci.yml::pipeline when a fixture's compat-diff fails. +// One file per cell (noble-x86_64-8.4 is the only compat cell today), +// aggregating all failing fixtures from that cell. +type DeviationsFile struct { + Cell string `json:"cell"` + Fixtures []FixtureDeviations `json:"fixtures"` +} + +// FixtureDeviations collects the non-empty deviations from one fixture's +// compat-diff invocation. +type FixtureDeviations struct { + Name string `json:"name"` + Deviations []compatdiff.Deviation `json:"deviations"` +} + +// AppendDeviations merges (fixture, deviations) into the JSON file at +// path. If the file doesn't exist, it is created with cell as the top- +// level Cell field. If it exists with a different Cell, an error is +// returned — the writer refuses to cross-contaminate per-cell artifacts. +// +// Callers pass the cell tag in the form "noble-x86_64-8.4" (os-arch-php) +// so the CI `compat-report` job can render a clear per-cell heading. +// +// Not safe for concurrent callers on the same path: the helper performs +// read-modify-write without locking and expects the serial fixture loop +// in testcell.go (runFixture is called sequentially from RunTestCell) to +// be the only caller. If fixture execution is ever parallelized, add a +// per-path mutex or switch to an append-only output shape. +func AppendDeviations(path, cell, fixture string, devs []compatdiff.Deviation) error { + if len(devs) == 0 { + return fmt.Errorf("AppendDeviations: refuse to record fixture %q with no deviations (artifact is for failing fixtures only)", fixture) + } + var doc DeviationsFile + data, err := os.ReadFile(filepath.Clean(path)) + switch { + case err == nil: + if err := json.Unmarshal(data, &doc); err != nil { + return fmt.Errorf("AppendDeviations: parse existing %s: %w", path, err) + } + if doc.Cell != "" && doc.Cell != cell { + return fmt.Errorf("AppendDeviations: cell mismatch: file=%q call=%q", doc.Cell, cell) + } + case errors.Is(err, os.ErrNotExist): + // Fresh file — fall through. + default: + return fmt.Errorf("AppendDeviations: read existing %s: %w", path, err) + } + if doc.Cell == "" { + doc.Cell = cell + } + doc.Fixtures = append(doc.Fixtures, FixtureDeviations{Name: fixture, Deviations: devs}) + out, err := json.MarshalIndent(doc, "", " ") + if err != nil { + return fmt.Errorf("AppendDeviations: marshal: %w", err) + } + if err := os.MkdirAll(filepath.Dir(path), 0o750); err != nil { + return fmt.Errorf("AppendDeviations: mkdir: %w", err) + } + if err := os.WriteFile(path, out, 0o644); err != nil { //nolint:gosec // G306: artifact is non-sensitive CI output uploaded by upload-artifact; 0644 matches prior art (writeLayoutLockfileOverride). + return fmt.Errorf("AppendDeviations: write: %w", err) + } + return nil +} diff --git a/internal/testsuite/deviations_test.go b/internal/testsuite/deviations_test.go new file mode 100644 index 0000000..d004b0f --- /dev/null +++ b/internal/testsuite/deviations_test.go @@ -0,0 +1,95 @@ +package testsuite + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/buildrush/setup-php/internal/compatdiff" +) + +// TestAppendDeviations_CreatesFileOnFirstCall asserts that writing to a +// non-existent path creates the file with a well-formed JSON document +// containing the single fixture's entry. +func TestAppendDeviations_CreatesFileOnFirstCall(t *testing.T) { + path := filepath.Join(t.TempDir(), "deviations.json") + devs := []compatdiff.Deviation{{Path: "ini.memory_limit", Ours: "256M", Theirs: "-1"}} + if err := AppendDeviations(path, "noble-x86_64-8.4", "bare", devs); err != nil { + t.Fatalf("AppendDeviations: %v", err) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + var parsed DeviationsFile + if err := json.Unmarshal(data, &parsed); err != nil { + t.Fatalf("Unmarshal: %v\nraw: %s", err, data) + } + if parsed.Cell != "noble-x86_64-8.4" { + t.Errorf("Cell: got %q, want %q", parsed.Cell, "noble-x86_64-8.4") + } + if len(parsed.Fixtures) != 1 || parsed.Fixtures[0].Name != "bare" { + t.Errorf("Fixtures: got %+v, want 1 entry for 'bare'", parsed.Fixtures) + } + if len(parsed.Fixtures[0].Deviations) != 1 || parsed.Fixtures[0].Deviations[0].Path != "ini.memory_limit" { + t.Errorf("Deviations: got %+v, want 1 entry for ini.memory_limit", parsed.Fixtures[0].Deviations) + } +} + +// TestAppendDeviations_AppendsToExistingFile asserts that a second call +// to AppendDeviations merges into the same JSON file without clobbering +// the first entry. +func TestAppendDeviations_AppendsToExistingFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "deviations.json") + if err := AppendDeviations(path, "noble-x86_64-8.4", "bare", + []compatdiff.Deviation{{Path: "ini.memory_limit", Ours: "256M", Theirs: "-1"}}); err != nil { + t.Fatalf("first AppendDeviations: %v", err) + } + if err := AppendDeviations(path, "noble-x86_64-8.4", "multi-ext", + []compatdiff.Deviation{{Path: "extensions", Ours: "[a,b]", Theirs: "[a]"}}); err != nil { + t.Fatalf("second AppendDeviations: %v", err) + } + data, _ := os.ReadFile(path) + var parsed DeviationsFile + if err := json.Unmarshal(data, &parsed); err != nil { + t.Fatalf("Unmarshal: %v\nraw: %s", err, data) + } + if len(parsed.Fixtures) != 2 { + t.Fatalf("Fixtures: got %d, want 2", len(parsed.Fixtures)) + } + names := map[string]bool{parsed.Fixtures[0].Name: true, parsed.Fixtures[1].Name: true} + if !names["bare"] || !names["multi-ext"] { + t.Errorf("expected both 'bare' and 'multi-ext' in %+v", parsed.Fixtures) + } +} + +// TestAppendDeviations_RejectsCellMismatch protects against a caller +// mixing two different cells' outputs into the same artifact file — +// that would break the per-cell artifact naming scheme in CI. +func TestAppendDeviations_RejectsCellMismatch(t *testing.T) { + path := filepath.Join(t.TempDir(), "deviations.json") + _ = AppendDeviations(path, "noble-x86_64-8.4", "bare", + []compatdiff.Deviation{{Path: "p", Ours: "a", Theirs: "b"}}) + err := AppendDeviations(path, "noble-aarch64-8.4", "multi-ext", + []compatdiff.Deviation{{Path: "p", Ours: "a", Theirs: "b"}}) + if err == nil { + t.Fatalf("AppendDeviations: expected cell-mismatch error, got nil") + } +} + +// TestAppendDeviations_RejectsEmptyDeviations pins the contract that the +// artifact records failing fixtures only. A caller that passes an empty +// (or nil) deviations slice has a bug — reject loudly rather than +// silently writing `"deviations": null`. +func TestAppendDeviations_RejectsEmptyDeviations(t *testing.T) { + path := filepath.Join(t.TempDir(), "deviations.json") + err := AppendDeviations(path, "noble-x86_64-8.4", "bare", nil) + if err == nil { + t.Errorf("AppendDeviations(nil devs): want error, got nil") + } + err = AppendDeviations(path, "noble-x86_64-8.4", "bare", []compatdiff.Deviation{}) + if err == nil { + t.Errorf("AppendDeviations(empty devs): want error, got nil") + } +} From 65d27ceba09f98e12020ec5c9229dceb4fec4f3f Mon Sep 17 00:00:00 2001 From: Maximilian Date: Fri, 24 Apr 2026 16:39:09 +0200 Subject: [PATCH 06/18] feat(testsuite): run compat-diff per fixture in canonical cell Extend testCellOpts with CompatMatrix/GoldenDir/DeviationsPath flags, wire them into parseTestCellFlags, add the shouldRunCompat gate and compat-diff step inside runFixture (step 6, after invariants), add the RW deviations mount in buildCellMounts, and thread all three new flags from runCell into the inner test-cell command. --- internal/testsuite/runner.go | 22 +- internal/testsuite/runner_test.go | 10 +- internal/testsuite/testcell.go | 80 +++++- internal/testsuite/testcell_compat_test.go | 285 +++++++++++++++++++++ 4 files changed, 386 insertions(+), 11 deletions(-) create mode 100644 internal/testsuite/testcell_compat_test.go diff --git a/internal/testsuite/runner.go b/internal/testsuite/runner.go index ca1a84e..699f301 100644 --- a/internal/testsuite/runner.go +++ b/internal/testsuite/runner.go @@ -284,12 +284,17 @@ func runCell(ctx context.Context, opts *testOpts, set *FixtureSet, cellOS, cellA // preinstalled. Wrap the test-cell invocation with the same apt // preamble that build-ext uses so probe.sh's `php -v` has its // shared libs available (libreadline, libpq, libssl, libxml2, etc.). - cellCmd := strings.Join([]string{"/usr/local/bin/phpup", "internal", "test-cell", + cellCmdParts := []string{ + "/usr/local/bin/phpup", "internal", "test-cell", "--os", cellOS, "--arch", cellArch, "--php", cellPHP, "--fixtures", "/test-compat/fixtures.yaml", "--probe", "/test-compat/probe.sh", "--registry", "oci-layout:/registry", - }, " ") + "--compat-matrix", "/compat-matrix.md", + "--golden-dir", "/golden", + "--deviations", "/deviations/" + cellOS + "-" + cellArch + "-" + cellPHP + ".json", + } + cellCmd := strings.Join(cellCmdParts, " ") runOpts := &build.DockerRunOpts{ Image: image, Platform: platform, @@ -317,6 +322,7 @@ func runCell(ctx context.Context, opts *testOpts, set *FixtureSet, cellOS, cellA // - lockfile override → /tmp/bundles-override.lock (read-only) // - docs/compat-matrix.md → /compat-matrix.md (read-only; REQUIRED, errors if missing) // - goldens dir → /golden (read-only; OPTIONAL, silently skipped if dir absent) +// - deviations dir → /deviations (read-write; CI compat-report consumes) // // Env is minimal: PHPUP_REGISTRY=oci-layout:/registry so the inner // phpup invocation inherits the right registry without needing to pass @@ -385,6 +391,18 @@ func buildCellMounts(opts *testOpts, _, _, _ string) ([]build.Mount, map[string] ) } + // Deviations output directory: RW mount so runFixture can write per-cell + // artifact JSONs that ci.yml::compat-report consumes via upload-artifact. + // Directory is created here (not lazily inside the container) so the + // bind mount has a valid host path before `docker run`. + deviationsDir := filepath.Join(opts.AbsRepo, ".cache", "phpup-test", "deviations") + if err := os.MkdirAll(deviationsDir, 0o750); err != nil { + return nil, nil, fmt.Errorf("mkdir deviations: %w", err) + } + mounts = append(mounts, + build.Mount{Host: deviationsDir, Container: "/deviations", ReadOnly: false}, + ) + return mounts, env, nil } diff --git a/internal/testsuite/runner_test.go b/internal/testsuite/runner_test.go index 3f4d705..9490d19 100644 --- a/internal/testsuite/runner_test.go +++ b/internal/testsuite/runner_test.go @@ -283,9 +283,13 @@ func TestBuildCellMounts_MissingCompatMatrix(t *testing.T) { } // TestBuildCellMounts_GoldensDirOptional asserts that an absent -// goldens directory is NOT an error — it's the degraded mode that -// a fresh checkout (pre-refresh-workflow) sits in. Compat-diff -// skips later in runFixture when the golden file is missing. +// goldens directory is NOT an error from buildCellMounts — it's the +// degraded mode that a fresh checkout (pre-refresh-workflow) sits in. +// Note: on a canonical-cell run, opted-in fixtures whose per-fixture +// golden file is missing are a HARD error at the runFixture level +// (step 6), not a silent skip. This test only covers the mount- +// construction layer; the per-fixture error is covered in +// testcell_compat_test.go. func TestBuildCellMounts_GoldensDirOptional(t *testing.T) { dir := t.TempDir() fakeBinary := writeTestsuiteFixture(t, dir) diff --git a/internal/testsuite/testcell.go b/internal/testsuite/testcell.go index f8d86b7..a80c46a 100644 --- a/internal/testsuite/testcell.go +++ b/internal/testsuite/testcell.go @@ -14,6 +14,8 @@ import ( "sort" "strings" "sync" + + "github.com/buildrush/setup-php/internal/compatdiff" ) // InstallFunc runs `phpup install` for a single fixture with the given env @@ -190,11 +192,20 @@ func composeProbeEnv(envFile, pathFile string) ([]string, error) { } // testCellOpts is the parsed flag set for `phpup internal test-cell`. +// The Compat* paths are container-side absolute paths populated by the +// outer runner (runCell in runner.go) when the compat gate should be +// active. Non-canonical cells and the standalone `phpup install` path +// leave them empty — runFixture's shouldRunCompat gate short-circuits +// on any empty value, so the compat-diff step becomes a no-op without +// requiring the CLI user to know about it. type testCellOpts struct { - OS, Arch, PHP string - FixturesPath string - ProbePath string - RegistryURI string + OS, Arch, PHP string + FixturesPath string + ProbePath string + RegistryURI string + CompatMatrix string // path to compat-matrix.md; "" = compat off + GoldenDir string // directory containing .json goldens; "" = compat off + DeviationsPath string // path to append deviation artifacts to; "" = compat off } // parseTestCellFlags parses the flag tail for `phpup internal test-cell`. @@ -208,6 +219,9 @@ func parseTestCellFlags(args []string) (*testCellOpts, error) { fixturesFlag := fs.String("fixtures", "/test-compat/fixtures.yaml", "Path to fixtures YAML") probeFlag := fs.String("probe", "/test-compat/probe.sh", "Path to probe.sh") registryFlag := fs.String("registry", "", "Registry URI for phpup install (propagated via PHPUP_REGISTRY env)") + compatMatrixFlag := fs.String("compat-matrix", "", "Path to compat-matrix.md; empty disables compat-diff") + goldenDirFlag := fs.String("golden-dir", "", "Directory containing v2 golden probe JSONs; empty disables compat-diff") + deviationsFlag := fs.String("deviations", "", "Path to append deviation artifact JSON; empty disables artifact writes") if err := fs.Parse(args); err != nil { return nil, err } @@ -216,8 +230,12 @@ func parseTestCellFlags(args []string) (*testCellOpts, error) { } return &testCellOpts{ OS: *osFlag, Arch: *archFlag, PHP: *phpFlag, - FixturesPath: *fixturesFlag, ProbePath: *probeFlag, - RegistryURI: *registryFlag, + FixturesPath: *fixturesFlag, + ProbePath: *probeFlag, + RegistryURI: *registryFlag, + CompatMatrix: *compatMatrixFlag, + GoldenDir: *goldenDirFlag, + DeviationsPath: *deviationsFlag, }, nil } @@ -386,9 +404,59 @@ func runFixture(ctx context.Context, opts *testCellOpts, f *Fixture) fixtureOutc out.Err = err return out } + + // 6. Compat-diff gate (canonical cell + fixture.Compat + golden exists). + if shouldRunCompat(opts, f) { + goldenPath := filepath.Join(opts.GoldenDir, f.Name+".json") + if _, err := os.Stat(goldenPath); err != nil { + // Missing golden on a fixture that opted in is a hard error: it + // means the refresh workflow has never captured this fixture, or + // a golden was deleted. Better to fail loudly than silently skip. + out.Err = fmt.Errorf("compat: golden missing for fixture %q at %s: run `gh workflow run compat-golden-refresh.yml` to capture it", f.Name, goldenPath) + return out + } + devs, err := compatdiff.DiffFiles(probeOut, goldenPath, opts.CompatMatrix, f.Name) + if err != nil { + out.Err = fmt.Errorf("compat: diff failed for fixture %q: %w", f.Name, err) + return out + } + if len(devs) > 0 { + if opts.DeviationsPath != "" { + cell := opts.OS + "-" + opts.Arch + "-" + opts.PHP + if werr := AppendDeviations(opts.DeviationsPath, cell, f.Name, devs); werr != nil { + // Artifact write failure is diagnostic, not compat-diff + // failure; log but don't overwrite the deviation error. + fmt.Fprintf(os.Stderr, "compat: append deviations artifact: %v\n", werr) + } + } + out.Err = fmt.Errorf("compat: fixture %q has %d unexplained deviation(s) vs v2 golden", f.Name, len(devs)) + return out + } + } return out } +// shouldRunCompat returns true iff the current cell is the canonical +// compat cell (noble/x86_64/8.4), the fixture opted in via Compat: true, +// and the outer runner supplied both compat paths. Non-empty paths +// indicate the outer runner intends compat to run in this cell; the +// on-disk presence of the per-fixture golden file is re-verified +// inside step 6 of runFixture (the `os.Stat(goldenPath)` check). +func shouldRunCompat(opts *testCellOpts, f *Fixture) bool { + if !f.Compat { + return false + } + // Defense-in-depth: the CI-driven path (runner.go:runCell) always + // passes both flags with constant non-empty values, so this branch + // is only reachable by test-direct callers that build testCellOpts + // by hand. Guard kept so a future bug in flag threading fails + // closed (compat off) rather than open (nil-deref in DiffFiles). + if opts.CompatMatrix == "" || opts.GoldenDir == "" { + return false + } + return opts.OS == "noble" && opts.Arch == "x86_64" && opts.PHP == "8.4" +} + // writeEnvSnapshot captures os.Environ() into path, one KEY=value per line, // sorted alphabetically. Used by probe.sh to diff against the post-install // env for the env_delta computation. diff --git a/internal/testsuite/testcell_compat_test.go b/internal/testsuite/testcell_compat_test.go new file mode 100644 index 0000000..5cd8a5b --- /dev/null +++ b/internal/testsuite/testcell_compat_test.go @@ -0,0 +1,285 @@ +package testsuite + +import ( + "context" + "encoding/json" + "io" + "os" + "path/filepath" + "strings" + "testing" +) + +// TestRunFixture_CompatSkippedOnNonCanonicalCell asserts that a fixture +// with Compat: true still passes on non-canonical cells (e.g., 8.1, +// arm64) because compat-diff is gated to noble/x86_64/8.4. The in-cell +// invariants still run; only the compat step is skipped. +func TestRunFixture_CompatSkippedOnNonCanonicalCell(t *testing.T) { + restore := SetInstaller(stubInstallerOK) + defer restore() + + workDir := t.TempDir() + opts := &testCellOpts{ + OS: "jammy", Arch: "aarch64", PHP: "8.1", + FixturesPath: filepath.Join(workDir, "fixtures.yaml"), + ProbePath: filepath.Join(workDir, "probe.sh"), + CompatMatrix: filepath.Join(workDir, "compat-matrix.md"), + GoldenDir: filepath.Join(workDir, "golden"), + DeviationsPath: filepath.Join(workDir, "deviations", "out.json"), + } + writeStubProbe(t, opts.ProbePath, `{"php_version":"8.1.0","sapi":"cli","zts":false,"extensions":[],"ini":{},"env_delta":[],"path_additions":[]}`) + f := &Fixture{Name: "bare", PHPVersion: "8.1", Compat: true} + + out := runFixture(context.Background(), opts, f) + if out.Err != nil { + t.Fatalf("runFixture err (want nil on non-canonical cell): %v", out.Err) + } + if _, err := os.Stat(opts.DeviationsPath); !os.IsNotExist(err) { + t.Errorf("deviations file should NOT exist on non-canonical cell; got err=%v", err) + } +} + +// TestRunFixture_CompatPassesOnCanonicalCellWithMatchingGolden asserts +// the compat-diff path runs when (os, arch, php) == canonical AND +// fixture.Compat && golden exists, and the probe matches. +func TestRunFixture_CompatPassesOnCanonicalCellWithMatchingGolden(t *testing.T) { + restore := SetInstaller(stubInstallerOK) + defer restore() + + workDir := t.TempDir() + opts := &testCellOpts{ + OS: "noble", Arch: "x86_64", PHP: "8.4", + FixturesPath: filepath.Join(workDir, "fixtures.yaml"), + ProbePath: filepath.Join(workDir, "probe.sh"), + CompatMatrix: filepath.Join(workDir, "compat-matrix.md"), + GoldenDir: filepath.Join(workDir, "golden"), + DeviationsPath: filepath.Join(workDir, "deviations", "out.json"), + } + probeJSON := `{"php_version":"8.4.0","sapi":"cli","zts":false,"extensions":[],"ini":{},"env_delta":[],"path_additions":[]}` + writeStubProbe(t, opts.ProbePath, probeJSON) + if err := os.MkdirAll(opts.GoldenDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(opts.GoldenDir, "bare.json"), []byte(probeJSON), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(opts.CompatMatrix, []byte(emptyAllowlistMD()), 0o644); err != nil { + t.Fatal(err) + } + f := &Fixture{Name: "bare", PHPVersion: "8.4", Compat: true} + + out := runFixture(context.Background(), opts, f) + if out.Err != nil { + t.Fatalf("runFixture err: %v", out.Err) + } + if _, err := os.Stat(opts.DeviationsPath); !os.IsNotExist(err) { + t.Errorf("deviations file should NOT exist on a passing compat run; got err=%v", err) + } +} + +// TestRunFixture_CompatFailsAndWritesDeviationsArtifact asserts that a +// probe that diverges from the golden fails the fixture AND appends to +// the deviations artifact. +func TestRunFixture_CompatFailsAndWritesDeviationsArtifact(t *testing.T) { + restore := SetInstaller(stubInstallerOK) + defer restore() + + workDir := t.TempDir() + opts := &testCellOpts{ + OS: "noble", Arch: "x86_64", PHP: "8.4", + FixturesPath: filepath.Join(workDir, "fixtures.yaml"), + ProbePath: filepath.Join(workDir, "probe.sh"), + CompatMatrix: filepath.Join(workDir, "compat-matrix.md"), + GoldenDir: filepath.Join(workDir, "golden"), + DeviationsPath: filepath.Join(workDir, "deviations", "out.json"), + } + oursProbe := `{"php_version":"8.4.0","sapi":"cli","zts":false,"extensions":[],"ini":{"memory_limit":"256M"},"env_delta":[],"path_additions":[]}` + theirsGolden := `{"php_version":"8.4.0","sapi":"cli","zts":false,"extensions":[],"ini":{"memory_limit":"-1"},"env_delta":[],"path_additions":[]}` + writeStubProbe(t, opts.ProbePath, oursProbe) + if err := os.MkdirAll(opts.GoldenDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(opts.GoldenDir, "bare.json"), []byte(theirsGolden), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(opts.CompatMatrix, []byte(emptyAllowlistMD()), 0o644); err != nil { + t.Fatal(err) + } + f := &Fixture{Name: "bare", PHPVersion: "8.4", Compat: true} + + out := runFixture(context.Background(), opts, f) + if out.Err == nil { + t.Fatalf("runFixture: want err for compat deviation, got nil") + } + if !strings.Contains(out.Err.Error(), "compat") { + t.Errorf("runFixture err should mention compat; got: %v", out.Err) + } + data, err := os.ReadFile(opts.DeviationsPath) + if err != nil { + t.Fatalf("deviations artifact not written: %v", err) + } + var df DeviationsFile + if err := json.Unmarshal(data, &df); err != nil { + t.Fatalf("parse deviations artifact: %v\nraw: %s", err, data) + } + if df.Cell != "noble-x86_64-8.4" { + t.Errorf("artifact cell: got %q, want noble-x86_64-8.4", df.Cell) + } + if len(df.Fixtures) != 1 || df.Fixtures[0].Name != "bare" { + t.Errorf("artifact fixtures: got %+v, want single 'bare' entry", df.Fixtures) + } +} + +// TestRunFixture_CompatDevationsPathEmpty asserts that when compat +// deviates but the caller passed an empty DeviationsPath, the fixture +// STILL fails with the compat-deviation error — the artifact-write +// step is skipped silently, not elevated into a separate failure. +// Covers the non-transitive gate: shouldRunCompat returns true even +// when DeviationsPath is empty; the artifact write is guarded +// separately inside step 6 of runFixture. +func TestRunFixture_CompatDevationsPathEmpty(t *testing.T) { + restore := SetInstaller(stubInstallerOK) + defer restore() + + workDir := t.TempDir() + opts := &testCellOpts{ + OS: "noble", Arch: "x86_64", PHP: "8.4", + FixturesPath: filepath.Join(workDir, "fixtures.yaml"), + ProbePath: filepath.Join(workDir, "probe.sh"), + CompatMatrix: filepath.Join(workDir, "compat-matrix.md"), + GoldenDir: filepath.Join(workDir, "golden"), + // DeviationsPath deliberately left "" — artifact write must skip + // without elevating to a failure mode. + } + oursProbe := `{"php_version":"8.4.0","sapi":"cli","zts":false,"extensions":[],"ini":{"memory_limit":"256M"},"env_delta":[],"path_additions":[]}` + theirsGolden := `{"php_version":"8.4.0","sapi":"cli","zts":false,"extensions":[],"ini":{"memory_limit":"-1"},"env_delta":[],"path_additions":[]}` + writeStubProbe(t, opts.ProbePath, oursProbe) + if err := os.MkdirAll(opts.GoldenDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(opts.GoldenDir, "bare.json"), []byte(theirsGolden), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(opts.CompatMatrix, []byte(emptyAllowlistMD()), 0o644); err != nil { + t.Fatal(err) + } + f := &Fixture{Name: "bare", PHPVersion: "8.4", Compat: true} + + out := runFixture(context.Background(), opts, f) + if out.Err == nil { + t.Fatalf("runFixture: want compat-deviation error, got nil") + } + if !strings.Contains(out.Err.Error(), "compat") { + t.Errorf("err should mention compat; got: %v", out.Err) + } +} + +// TestRunFixture_CompatWriterErrorIsDiagnostic asserts that when +// AppendDeviations fails (unwritable path), the fixture still fails +// with the compat-deviation error — the writer error is surfaced on +// stderr but does NOT overwrite the fixture's out.Err. CI's +// compat-report job relies on the deviation error being surfaced, not +// a file-write error. +func TestRunFixture_CompatWriterErrorIsDiagnostic(t *testing.T) { + restore := SetInstaller(stubInstallerOK) + defer restore() + + workDir := t.TempDir() + // Point DeviationsPath at a read-only parent so AppendDeviations' + // MkdirAll fails. Creating the parent as 0o500 (read+execute, no + // write) achieves this without needing superuser. + readOnlyParent := filepath.Join(workDir, "ro-parent") + if err := os.MkdirAll(readOnlyParent, 0o500); err != nil { + t.Fatal(err) + } + // Child dir inside the read-only parent; MkdirAll will fail when + // trying to create it. + deviationsPath := filepath.Join(readOnlyParent, "child", "deviations.json") + + opts := &testCellOpts{ + OS: "noble", Arch: "x86_64", PHP: "8.4", + FixturesPath: filepath.Join(workDir, "fixtures.yaml"), + ProbePath: filepath.Join(workDir, "probe.sh"), + CompatMatrix: filepath.Join(workDir, "compat-matrix.md"), + GoldenDir: filepath.Join(workDir, "golden"), + DeviationsPath: deviationsPath, + } + oursProbe := `{"php_version":"8.4.0","sapi":"cli","zts":false,"extensions":[],"ini":{"memory_limit":"256M"},"env_delta":[],"path_additions":[]}` + theirsGolden := `{"php_version":"8.4.0","sapi":"cli","zts":false,"extensions":[],"ini":{"memory_limit":"-1"},"env_delta":[],"path_additions":[]}` + writeStubProbe(t, opts.ProbePath, oursProbe) + if err := os.MkdirAll(opts.GoldenDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(opts.GoldenDir, "bare.json"), []byte(theirsGolden), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(opts.CompatMatrix, []byte(emptyAllowlistMD()), 0o644); err != nil { + t.Fatal(err) + } + f := &Fixture{Name: "bare", PHPVersion: "8.4", Compat: true} + + out := runFixture(context.Background(), opts, f) + if out.Err == nil { + t.Fatalf("runFixture: want compat-deviation error, got nil") + } + if !strings.Contains(out.Err.Error(), "compat") || !strings.Contains(out.Err.Error(), "unexplained deviation") { + t.Errorf("err should still be the compat-deviation error even when writer fails; got: %v", out.Err) + } + // The writer failure should NOT have replaced the compat error. We + // don't assert on stderr content here because capturing it would + // require more plumbing than is worth for this contract; the + // assertion that out.Err is the compat-deviation error (not a + // write-error with "mkdir" or "write" in its message) suffices. + if strings.Contains(out.Err.Error(), "mkdir") || strings.Contains(out.Err.Error(), "write") { + t.Errorf("out.Err should not carry the AppendDeviations error; got: %v", out.Err) + } +} + +// stubInstallerOK is a no-op InstallFunc for tests; it writes the GHA- +// runner emulation files so downstream composeProbeEnv doesn't fail. +func stubInstallerOK(_ context.Context, env map[string]string, _, _ io.Writer) error { + // runFixture pre-creates GITHUB_ENV / GITHUB_PATH files and passes their + // paths via env. A real installer writes to them; this stub does not. + // composeProbeEnv tolerates empty files (parseGitHubEnv returns empty + // overlay on read error or empty body). + _ = env + return nil +} + +// writeStubProbe writes a bash script that emits the given JSON to its +// first argument. Mirrors the shape of test/compat/probe.sh just enough +// for runFixture's parsing to succeed. probe.sh is invoked by runFixture +// with four args: out, env-before, path-before, ini-keys. This stub only +// reads the first and writes the literal JSON to it. +// +// The heredoc delimiter is single-quoted (<<'JSON') so bash does not +// perform variable expansion, command substitution, or backslash +// handling on jsonBody — tests can safely embed JSON containing $, `, +// or \ without corrupting the written output. +func writeStubProbe(t *testing.T, path, jsonBody string) { + t.Helper() + script := `#!/usr/bin/env bash +set -euo pipefail +out="$1" +cat > "$out" <<'JSON' +` + jsonBody + ` +JSON +` + if err := os.WriteFile(path, []byte(script), 0o755); err != nil { + t.Fatal(err) + } +} + +// emptyAllowlistMD returns a minimal compat-matrix.md body with the +// required delimiters and an empty deviations list. Just enough for +// internal/compatdiff.loadAllowlist to parse without error. +func emptyAllowlistMD() string { + return `# Stub + + +` + "```yaml" + ` +deviations: [] +` + "```" + ` + +` +} From d6aec32f86703a010610aae084c94a7e713ec248 Mon Sep 17 00:00:00 2001 From: Maximilian Date: Fri, 24 Apr 2026 16:54:59 +0200 Subject: [PATCH 07/18] feat(testsuite): add phpup internal golden-capture subcommand --- internal/testsuite/goldencapture.go | 129 +++++++++++++++++++++++ internal/testsuite/goldencapture_test.go | 128 ++++++++++++++++++++++ internal/testsuite/internalmain.go | 2 + 3 files changed, 259 insertions(+) create mode 100644 internal/testsuite/goldencapture.go create mode 100644 internal/testsuite/goldencapture_test.go diff --git a/internal/testsuite/goldencapture.go b/internal/testsuite/goldencapture.go new file mode 100644 index 0000000..64480c1 --- /dev/null +++ b/internal/testsuite/goldencapture.go @@ -0,0 +1,129 @@ +package testsuite + +import ( + "errors" + "flag" + "fmt" + "os" + "os/exec" + "path/filepath" +) + +// GoldenCaptureOpts is the parsed flag set for `phpup internal +// golden-capture`. Callers may construct one directly (tests) or via +// parseGoldenCaptureFlags (CLI). +type GoldenCaptureOpts struct { + FixturesPath string + ProbePath string + OutDir string +} + +// RunGoldenCapture iterates every Compat: true fixture in the loaded +// fixtures file, runs probe.sh (assuming v2 has already been set up +// by the outer workflow), and writes the probe output JSON to +// /.json. Caller is responsible for sequencing +// the v2 setup step before invoking this; this function does not +// install or configure PHP. +// +// Running this on a developer laptop without first installing v2 +// captures that laptop's PHP — the resulting JSON is not a valid v2 +// golden. See docs/superpowers/specs/2026-04-24-pr-gated-v2-compat-design.md +// and the Makefile compat-refresh-goldens target for the canonical +// invocation path (weekly refresh workflow + pinned v2 install). +func RunGoldenCapture(opts GoldenCaptureOpts) error { + set, err := Load(opts.FixturesPath) + if err != nil { + return fmt.Errorf("golden-capture: %w", err) + } + if err := os.MkdirAll(opts.OutDir, 0o750); err != nil { + return fmt.Errorf("golden-capture: mkdir out-dir: %w", err) + } + var failed []string + for i := range set.Fixtures { + f := &set.Fixtures[i] + if !f.Compat { + continue + } + outPath := filepath.Join(opts.OutDir, f.Name+".json") + if err := captureOne(opts.ProbePath, f, outPath); err != nil { + fmt.Fprintf(os.Stderr, "golden-capture: fixture=%s FAIL: %v\n", f.Name, err) + failed = append(failed, f.Name) + continue + } + fmt.Printf("golden-capture: fixture=%s wrote %s\n", f.Name, outPath) + } + if len(failed) > 0 { + return fmt.Errorf("golden-capture: %d fixture(s) failed: %v", len(failed), failed) + } + return nil +} + +// captureOne is the per-fixture probe invocation. It writes tmpfiles +// for probe.sh's env-before / path-before / ini-keys inputs, then +// execs bash probe.sh with the given outPath. +func captureOne(probePath string, f *Fixture, outPath string) (err error) { + dir, mktempErr := os.MkdirTemp("", "phpup-golden-capture-"+f.Name+"-*") + if mktempErr != nil { + return fmt.Errorf("mktemp: %w", mktempErr) + } + // Clean up the per-fixture tempdir on success; leave it behind on + // error so a maintainer can inspect env-before / path-before / + // ini-keys.txt when debugging a failed probe. + defer func() { + if err == nil { + _ = os.RemoveAll(dir) + } + }() + envBefore := filepath.Join(dir, "env-before") + pathBefore := filepath.Join(dir, "path-before") + iniKeys := filepath.Join(dir, "ini-keys.txt") + if err := writeEnvSnapshot(envBefore); err != nil { + return fmt.Errorf("env-before: %w", err) + } + if err := os.WriteFile(pathBefore, []byte(os.Getenv("PATH")), 0o600); err != nil { + return fmt.Errorf("path-before: %w", err) + } + if err := writeIniKeysFromFixture(iniKeys, f); err != nil { + return fmt.Errorf("ini-keys: %w", err) + } + //nolint:gosec // G204: probePath is --probe flag value (trusted operator input); + // tmp-file args are filepath.Join-derived. No user CLI flows into argv. + cmd := exec.Command("bash", probePath, outPath, envBefore, pathBefore, iniKeys) + out, runErr := cmd.CombinedOutput() + if runErr != nil { + return fmt.Errorf("probe.sh: %w (is PHP on PATH? this subcommand assumes the caller has installed v2 first, e.g. `shivammathur/setup-php@` in the refresh workflow); output tail:\n%s", runErr, tailBytes(out, 20)) + } + return nil +} + +// parseGoldenCaptureFlags parses the flag tail for +// `phpup internal golden-capture`. All flags have sensible defaults +// for invocation from the CI refresh workflow (where the working +// directory is the repo root). +func parseGoldenCaptureFlags(args []string) (*GoldenCaptureOpts, error) { + fs := flag.NewFlagSet("phpup internal golden-capture", flag.ContinueOnError) + fixtures := fs.String("fixtures", "test/compat/fixtures.yaml", "Path to fixtures YAML") + probe := fs.String("probe", "test/compat/probe.sh", "Path to probe.sh") + outDir := fs.String("out-dir", "test/compat/testdata/golden/v2", "Where to write .json") + if err := fs.Parse(args); err != nil { + return nil, err + } + if *fixtures == "" || *probe == "" || *outDir == "" { + return nil, errors.New("golden-capture: --fixtures, --probe, --out-dir are required") + } + return &GoldenCaptureOpts{ + FixturesPath: *fixtures, + ProbePath: *probe, + OutDir: *outDir, + }, nil +} + +// MainGoldenCapture is the entry point invoked by `phpup internal +// golden-capture`. It parses flags, then calls RunGoldenCapture. +func MainGoldenCapture(args []string) error { + opts, err := parseGoldenCaptureFlags(args) + if err != nil { + return err + } + return RunGoldenCapture(*opts) +} diff --git a/internal/testsuite/goldencapture_test.go b/internal/testsuite/goldencapture_test.go new file mode 100644 index 0000000..32ea151 --- /dev/null +++ b/internal/testsuite/goldencapture_test.go @@ -0,0 +1,128 @@ +package testsuite + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" +) + +// TestRunGoldenCapture_WritesAllEligibleFixtures verifies the capture +// command reads test/compat/fixtures.yaml, filters to entries with +// Compat: true, invokes probe.sh (stubbed), and writes one JSON per +// fixture to --out-dir. +func TestRunGoldenCapture_WritesAllEligibleFixtures(t *testing.T) { + workDir := t.TempDir() + + // Minimal fixtures file: 2 compat fixtures + 1 non-compat (skipped). + fixturesPath := filepath.Join(workDir, "fixtures.yaml") + if err := os.WriteFile(fixturesPath, []byte(`fixtures: + - name: bare + php-version: "8.4" + extensions: "" + ini-values: "" + coverage: "none" + compat: true + - name: multi-ext + php-version: "8.4" + extensions: "redis, xdebug" + ini-values: "" + coverage: "none" + compat: true + - name: other + php-version: "8.4" + extensions: "" + ini-values: "" + coverage: "none" +`), 0o644); err != nil { + t.Fatal(err) + } + + // Stub probe.sh: emits an always-ok JSON (probe contents are irrelevant + // to this test; we're verifying orchestration, not probe fidelity). + probePath := filepath.Join(workDir, "probe.sh") + if err := os.WriteFile(probePath, []byte(`#!/usr/bin/env bash +set -euo pipefail +out="$1" +cat > "$out" <<'JSON' +{"php_version":"8.4.0","sapi":"cli","zts":false,"extensions":[],"ini":{},"env_delta":[],"path_additions":[]} +JSON +`), 0o755); err != nil { + t.Fatal(err) + } + + outDir := filepath.Join(workDir, "golden") + err := RunGoldenCapture(GoldenCaptureOpts{ + FixturesPath: fixturesPath, + ProbePath: probePath, + OutDir: outDir, + }) + if err != nil { + t.Fatalf("RunGoldenCapture: %v", err) + } + + // Check presence of the two compat goldens and absence of the third. + for _, name := range []string{"bare", "multi-ext"} { + path := filepath.Join(outDir, name+".json") + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("expected %s; got err: %v", path, err) + } + var doc map[string]any + if err := json.Unmarshal(data, &doc); err != nil { + t.Errorf("%s is not valid JSON: %v", path, err) + } + } + if _, err := os.Stat(filepath.Join(outDir, "other.json")); !os.IsNotExist(err) { + t.Errorf("non-compat fixture 'other' should not have a golden; err=%v", err) + } +} + +// TestRunGoldenCapture_NoCompatFixturesIsNoOp pins the contract that +// a fixtures file containing zero Compat: true entries returns nil +// (no work, no error). Guards against a future refactor that would +// error on "no eligible fixtures found" — the refresh workflow must +// tolerate the empty case. +func TestRunGoldenCapture_NoCompatFixturesIsNoOp(t *testing.T) { + workDir := t.TempDir() + + fixturesPath := filepath.Join(workDir, "fixtures.yaml") + if err := os.WriteFile(fixturesPath, []byte(`fixtures: + - name: a + php-version: "8.4" + extensions: "" + ini-values: "" + coverage: "none" + - name: b + php-version: "8.4" + extensions: "" + ini-values: "" + coverage: "none" +`), 0o644); err != nil { + t.Fatal(err) + } + + probePath := filepath.Join(workDir, "probe.sh") + if err := os.WriteFile(probePath, []byte(`#!/usr/bin/env bash +exit 0 +`), 0o755); err != nil { + t.Fatal(err) + } + + outDir := filepath.Join(workDir, "golden") + err := RunGoldenCapture(GoldenCaptureOpts{ + FixturesPath: fixturesPath, + ProbePath: probePath, + OutDir: outDir, + }) + if err != nil { + t.Errorf("RunGoldenCapture on all-non-compat fixtures: want nil, got %v", err) + } + entries, err := os.ReadDir(outDir) + if err != nil { + t.Fatalf("ReadDir(%s): %v", outDir, err) + } + if len(entries) != 0 { + t.Errorf("out-dir should be empty when no fixtures are eligible; got %d entries", len(entries)) + } +} diff --git a/internal/testsuite/internalmain.go b/internal/testsuite/internalmain.go index ecf82f8..df74fe9 100644 --- a/internal/testsuite/internalmain.go +++ b/internal/testsuite/internalmain.go @@ -17,6 +17,8 @@ func InternalMain(args []string) error { return errors.New("phpup internal: usage: phpup internal [flags]") } switch args[0] { + case "golden-capture": + return MainGoldenCapture(args[1:]) case "test-cell": return RunTestCell(context.Background(), args[1:]) default: From 36ee94c24cc8224abc81abc5ff9a9eb7e9447d99 Mon Sep 17 00:00:00 2001 From: Maximilian Date: Fri, 24 Apr 2026 17:03:51 +0200 Subject: [PATCH 08/18] build(make): add compat-refresh-goldens target --- Makefile | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index ca1c0a1..d9751d7 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ .PHONY: check fmt fmt-check vet lint tidy tidy-check test test-node build clean \ build-linux-amd64 build-linux-arm64 bundle-php bundle-ext gc-bundles-dry-run \ - ci-cell ci + ci-cell ci compat-refresh-goldens # Path to the native phpup binary used by bundle-php / bundle-ext. Overridable # so CI / power users can point at a pre-built binary. @@ -179,6 +179,18 @@ ci: $(PHPUP_BIN) done; \ done +# compat-refresh-goldens: run `phpup internal golden-capture` against the +# current PHP environment. Intended for use INSIDE the +# compat-golden-refresh.yml workflow after `shivammathur/setup-php@` +# has populated the runner. Running it on a developer laptop without +# having first installed v2 will produce a golden that reflects the +# laptop's PHP, not v2's — do not commit that output. +compat-refresh-goldens: $(PHPUP_BIN) + $(PHPUP_BIN) internal golden-capture \ + --fixtures test/compat/fixtures.yaml \ + --probe test/compat/probe.sh \ + --out-dir test/compat/testdata/golden/v2 + # Clean build artifacts clean: rm -rf bin/ dist/ From 01f95c1484bb54b52cbdc623e96818e1638ba4d0 Mon Sep 17 00:00:00 2001 From: Maximilian Date: Fri, 24 Apr 2026 17:09:10 +0200 Subject: [PATCH 09/18] feat(ci): weekly + on-demand v2 compat golden refresh workflow --- .github/workflows/compat-golden-refresh.yml | 166 ++++++++++++++++++ .../2026-04-24-pr-gated-v2-compat-design.md | 4 +- 2 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/compat-golden-refresh.yml diff --git a/.github/workflows/compat-golden-refresh.yml b/.github/workflows/compat-golden-refresh.yml new file mode 100644 index 0000000..f73ff1c --- /dev/null +++ b/.github/workflows/compat-golden-refresh.yml @@ -0,0 +1,166 @@ +name: compat-golden-refresh + +# Re-captures v2 compat probes weekly and opens a drift PR if the bytes +# changed. Also exposed via workflow_dispatch so maintainers can +# force-refresh on demand (e.g. to unblock a feature PR that went red +# on compat because v2 shipped something new between scheduled runs). + +on: + schedule: + - cron: "0 12 * * MON" + workflow_dispatch: + +permissions: + contents: write + pull-requests: write + +env: + # Pinned v2 SHA. MUST match `## Pinning` in docs/compat-matrix.md. + # The sanity-check job reads the markdown and refuses to run if the + # two values disagree. + PINNED_V2_SHA: accd6127cb78bee3e8082180cb391013d204ef9f + +jobs: + sanity: + runs-on: ubuntu-24.04 + timeout-minutes: 5 + outputs: + last_refreshed: ${{ steps.stamp.outputs.date }} + steps: + - uses: actions/checkout@v6 + - name: Verify PINNED_V2_SHA matches docs/compat-matrix.md + run: | + # Extract the SHA from the "Commit SHA (dereferenced)" row of the + # compat-matrix.md pinning table. Using awk with backtick as the + # field separator keeps this self-consistent even when the env's + # PINNED_V2_SHA changes — we read what's in the doc, then + # compare. Any drift between doc and env fails the workflow + # fast with an informative message. + doc_sha="$(awk -F'`' '/Commit SHA \(dereferenced\)/ {print $2; exit}' docs/compat-matrix.md)" + if [ "$doc_sha" != "$PINNED_V2_SHA" ]; then + echo "::error::docs/compat-matrix.md pins '$doc_sha' but workflow env is '$PINNED_V2_SHA' — update one to match the other" + exit 1 + fi + - id: stamp + run: echo "date=$(date -u +%Y-%m-%d)" >> "$GITHUB_OUTPUT" + + capture: + needs: sanity + runs-on: ubuntu-24.04 + timeout-minutes: 15 + strategy: + fail-fast: false + matrix: + fixture: + - { name: bare, php: "8.4", extensions: "", ini_values: "", coverage: "none", ini_file: "" } + - { name: exclusion, php: "8.4", extensions: ":opcache", ini_values: "", coverage: "none", ini_file: "" } + - { name: single-ext, php: "8.4", extensions: "redis", ini_values: "", coverage: "none", ini_file: "" } + - { name: multi-ext, php: "8.4", extensions: "redis, xdebug", ini_values: "", coverage: "none", ini_file: "" } + - { name: none-reset, php: "8.4", extensions: "none, redis", ini_values: "", coverage: "none", ini_file: "" } + - { name: coverage-pcov, php: "8.4", extensions: "", ini_values: "", coverage: "pcov", ini_file: "" } + - { name: ini-and-coverage, php: "8.4", extensions: "", ini_values: "memory_limit=256M,date.timezone=UTC", coverage: "xdebug", ini_file: "" } + - { name: ini-file-development, php: "8.4", extensions: "", ini_values: "", coverage: "none", ini_file: "development" } + steps: + - uses: actions/checkout@v6 + - name: Snapshot env + PATH before setup-php + run: | + env > /tmp/env-before + printf "%s" "$PATH" > /tmp/path-before + - name: Setup PHP (pinned v2) + uses: shivammathur/setup-php@accd6127cb78bee3e8082180cb391013d204ef9f + with: + php-version: ${{ matrix.fixture.php }} + extensions: ${{ matrix.fixture.extensions }} + ini-values: ${{ matrix.fixture.ini_values }} + coverage: ${{ matrix.fixture.coverage }} + ini-file: ${{ matrix.fixture.ini_file }} + - name: Probe + run: | + chmod +x test/compat/probe.sh + bash test/compat/probe.sh \ + /tmp/golden-${{ matrix.fixture.name }}.json \ + /tmp/env-before \ + /tmp/path-before \ + test/compat/ini-keys.txt + - uses: actions/upload-artifact@v7 + with: + name: golden-${{ matrix.fixture.name }} + path: /tmp/golden-${{ matrix.fixture.name }}.json + retention-days: 3 + + aggregate: + needs: [sanity, capture] + runs-on: ubuntu-24.04 + timeout-minutes: 10 + steps: + - uses: actions/checkout@v6 + - uses: actions/download-artifact@v8 + with: + pattern: golden-* + path: /tmp/golden-raw + merge-multiple: true + - name: Stage captured goldens into the repo tree + run: | + mkdir -p test/compat/testdata/golden/v2 + # Each artifact name is golden-; the file inside is + # named golden-.json. Normalize to .json. + for f in /tmp/golden-raw/golden-*.json; do + name="$(basename "$f" .json)" + dest_name="${name#golden-}" + cp "$f" "test/compat/testdata/golden/v2/${dest_name}.json" + done + ls -la test/compat/testdata/golden/v2/ + - name: Detect drift + id: drift + run: | + if git diff --quiet test/compat/testdata/golden/v2/; then + echo "drift=false" >> "$GITHUB_OUTPUT" + echo "No v2 drift detected." >> "$GITHUB_STEP_SUMMARY" + else + echo "drift=true" >> "$GITHUB_OUTPUT" + { + echo "### Per-fixture golden diffs"; + git diff --stat test/compat/testdata/golden/v2/ + } >> "$GITHUB_STEP_SUMMARY" + fi + - name: Open drift PR + if: steps.drift.outputs.drift == 'true' + uses: peter-evans/create-pull-request@v7 + with: + token: ${{ secrets.GITHUB_TOKEN }} + commit-message: "chore: refresh v2 compat goldens" + branch: chore/compat-golden-refresh + delete-branch: true + # github.ref_name expands to `main` on the scheduled-cron path + # and to the dispatch-target branch on `workflow_dispatch` — so + # the PR targets the correct base in both flows. In particular + # this is what makes the Task 10 initial-capture flow work: + # dispatching against feat/pr-gated-v2-compat opens a PR + # against that branch (not main), so the captured goldens + # land on the feature branch before it merges. + base: ${{ github.ref_name }} + title: "chore: refresh v2 compat goldens" + body: | + Weekly (or on-demand) refresh of the pinned v2 compat probes. + + **Pinned v2 SHA:** `${{ env.PINNED_V2_SHA }}` + **Captured at:** `${{ needs.sanity.outputs.last_refreshed }}` UTC + + ### Drift summary + + Review `git diff` on `test/compat/testdata/golden/v2/` to see + which fixture(s) shifted and which JSON paths moved. + + ### How to decide + + - **Allowlist already tolerates the drift** — safe to merge; + PR-time compat-diff would not have flagged it. Merging just + keeps goldens close to live v2. + - **Allowlist does NOT tolerate the drift** — merging this PR + makes the new v2 behavior the PR-time baseline. Open a + separate PR to either (a) match the new v2 behavior in our + action or (b) extend the allowlist in `docs/compat-matrix.md` + with a `reason:`. + + Opened by `.github/workflows/compat-golden-refresh.yml`. + labels: compat-refresh, chore diff --git a/docs/superpowers/specs/2026-04-24-pr-gated-v2-compat-design.md b/docs/superpowers/specs/2026-04-24-pr-gated-v2-compat-design.md index 4be084f..6548fee 100644 --- a/docs/superpowers/specs/2026-04-24-pr-gated-v2-compat-design.md +++ b/docs/superpowers/specs/2026-04-24-pr-gated-v2-compat-design.md @@ -208,8 +208,8 @@ compat-golden-refresh.yml (on schedule + workflow_dispatch): download artifacts into test/compat/testdata/golden/v2/ if git diff --quiet: exit 0 (record "no drift") else: - create branch `chore/compat-golden-refresh-` - open PR via peter-evans/create-pull-request + force-push to single rolling branch `chore/compat-golden-refresh` + open PR via peter-evans/create-pull-request (delete-branch: true) body = "Per-fixture key-level deltas:\n ..." ``` From 66fbef1798c6eed4730610bba90f4a5c2828dc2d Mon Sep 17 00:00:00 2001 From: Maximilian Date: Fri, 24 Apr 2026 17:20:38 +0200 Subject: [PATCH 10/18] feat(ci): compat-report job posts sticky PR comment on compat deviations --- .github/workflows/ci.yml | 103 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 956029e..46716d3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -113,6 +113,14 @@ jobs: # "do the normal cache-probe". FORCE: ${{ inputs.force && '1' || '' }} run: make ci-cell OS=${{ matrix.os }} ARCH=${{ matrix.arch }} PHP=${{ matrix.php }} FORCE="$FORCE" + - name: Upload deviation artifacts (compat fail only) + if: failure() && matrix.os == 'noble' && matrix.arch == 'x86_64' && matrix.php == '8.4' + uses: actions/upload-artifact@v7 + with: + name: compat-deviations-${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.php }} + path: .cache/phpup-test/deviations/${{ matrix.os }}-${{ matrix.arch }}-${{ matrix.php }}.json + if-no-files-found: ignore + retention-days: 7 - name: Upload oci-layout artifact (main only) if: github.ref == 'refs/heads/main' uses: actions/upload-artifact@v7 @@ -188,6 +196,101 @@ jobs: || { echo "::error::memory_limit=$memlim, want 256M"; exit 1; } echo "smoke-action: action surface OK" + compat-report: + name: Compat report (PR comment) + needs: pipeline + if: always() && github.event_name == 'pull_request' && needs.pipeline.result != 'skipped' + runs-on: ubuntu-24.04 + permissions: + pull-requests: write + steps: + - uses: actions/checkout@v6 + - uses: actions/download-artifact@v8 + with: + pattern: compat-deviations-* + path: /tmp/compat-deviations + merge-multiple: true + - name: Render comment body + id: render + env: + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + set -euo pipefail + mkdir -p /tmp/compat-deviations + body_file=/tmp/compat-report-body.md + found=0 + for f in /tmp/compat-deviations/*.json; do + [ -f "$f" ] || continue + found=1 + break + done + if [ $found -eq 1 ]; then + { + echo "## ⚠️ Compat-diff detected deviations from shivammathur/setup-php@v2" + echo "" + echo "" + echo "" + echo "This PR's \`phpup install\` behavior diverges from the pinned v2 baseline in ways the compat-matrix allowlist doesn't cover." + echo "" + echo "### Deviations" + echo "" + for f in /tmp/compat-deviations/*.json; do + cell=$(jq -r '.cell' "$f") + jq -r --arg cell "$cell" '.fixtures[] | "**Fixture \`\(.name)\` (\($cell))**\n" + (.deviations[:20] | map("- \`\(.Path)\`: ours=\`\(.Ours)\`, theirs=\`\(.Theirs)\`") | join("\n")) + "\n"' "$f" + done + echo "" + echo "### What to do" + echo "" + echo "1. **If this is a regression in your change** — fix it, or add an entry to the deviations allowlist in [\`docs/compat-matrix.md\`](../blob/main/docs/compat-matrix.md) with a \`reason:\` explaining why it's intentional. The YAML block is delimited by \`\` / \`\`." + echo "" + echo "2. **If this looks like upstream v2 drift** (e.g., the PPA shipped a new package between golden refreshes) — ask a maintainer to force-refresh the goldens:" + echo "" + echo " \`\`\`" + echo " gh workflow run compat-golden-refresh.yml" + echo " \`\`\`" + echo "" + echo " Merge the resulting drift PR (\`chore: refresh v2 compat goldens\`), then rebase this PR onto \`main\`." + } > "$body_file" + echo "status=deviations" >> "$GITHUB_OUTPUT" + else + # No deviations this run. + { + echo "✅ Compat-diff cleared — this PR's \`phpup install\` now matches the pinned v2 baseline on all 8 axis fixtures." + echo "" + echo "" + } > "$body_file" + echo "status=cleared" >> "$GITHUB_OUTPUT" + fi + { + echo "body_path=$body_file" + } >> "$GITHUB_OUTPUT" + + - name: Find existing sticky comment + id: find + uses: peter-evans/find-comment@v3 + with: + issue-number: ${{ github.event.pull_request.number }} + comment-author: github-actions[bot] + body-includes: + + - name: Post/update deviations comment + if: steps.render.outputs.status == 'deviations' + uses: peter-evans/create-or-update-comment@v4 + with: + issue-number: ${{ github.event.pull_request.number }} + comment-id: ${{ steps.find.outputs.comment-id }} + body-path: ${{ steps.render.outputs.body_path }} + edit-mode: replace + + - name: Update cleared comment (only if sticky exists) + if: steps.render.outputs.status == 'cleared' && steps.find.outputs.comment-id != '' + uses: peter-evans/create-or-update-comment@v4 + with: + issue-number: ${{ github.event.pull_request.number }} + comment-id: ${{ steps.find.outputs.comment-id }} + body-path: ${{ steps.render.outputs.body_path }} + edit-mode: replace + publish: name: Publish to GHCR needs: [pipeline, smoke-action] From b22d47d7749d11a74327c09e1df3907e110eb921 Mon Sep 17 00:00:00 2001 From: Maximilian Date: Fri, 24 Apr 2026 17:29:35 +0200 Subject: [PATCH 11/18] fix(ci): add pull_request trigger to compat-golden-refresh for bootstrap GitHub's workflow_dispatch requires the workflow file to already exist on the default branch before it can be dispatched from any ref. That's a chicken-and-egg for the initial capture of the feature branch's goldens: we can't dispatch against feat/pr-gated-v2-compat because the file isn't on main yet. Adding a pull_request trigger scoped to this file's own path sidesteps the restriction (pull_request workflows run from the PR's head, not from main). It also keeps this safety net permanently: any future edit to the capture pipeline re-exercises it on its own PR before landing on main. --- .github/workflows/compat-golden-refresh.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/compat-golden-refresh.yml b/.github/workflows/compat-golden-refresh.yml index f73ff1c..dcca8cd 100644 --- a/.github/workflows/compat-golden-refresh.yml +++ b/.github/workflows/compat-golden-refresh.yml @@ -4,11 +4,23 @@ name: compat-golden-refresh # changed. Also exposed via workflow_dispatch so maintainers can # force-refresh on demand (e.g. to unblock a feature PR that went red # on compat because v2 shipped something new between scheduled runs). +# +# The `pull_request` trigger (scoped to this file's own path) serves two +# purposes: (1) it bootstraps the initial golden capture during the +# feature PR that first introduces this workflow — workflow_dispatch +# requires the workflow file to already live on the default branch, and +# the pull_request trigger sidesteps that chicken-and-egg; (2) any +# subsequent edit to this workflow re-runs it, so we never merge a +# broken capture pipeline. Scoped to just this file so unrelated PRs +# don't redundantly re-capture. on: schedule: - cron: "0 12 * * MON" workflow_dispatch: + pull_request: + paths: + - .github/workflows/compat-golden-refresh.yml permissions: contents: write From 18dafcdcb684915340d61124ff337a7a5f77bccd Mon Sep 17 00:00:00 2001 From: Maximilian Date: Fri, 24 Apr 2026 17:33:02 +0200 Subject: [PATCH 12/18] fix(ci): detect untracked goldens in drift detection git diff --quiet ignores untracked files, so on the initial capture (where every JSON is new to the tree) it returned clean and the drift-PR step was silently skipped. Switch to git status --porcelain, which surfaces adds, modifications, and deletions uniformly. Observed on the PR #77 bootstrap run (#24897651795): 8 goldens were staged into the tree but drift=false fired because none had been tracked previously. --- .github/workflows/compat-golden-refresh.yml | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/.github/workflows/compat-golden-refresh.yml b/.github/workflows/compat-golden-refresh.yml index dcca8cd..5bdf37c 100644 --- a/.github/workflows/compat-golden-refresh.yml +++ b/.github/workflows/compat-golden-refresh.yml @@ -125,14 +125,23 @@ jobs: - name: Detect drift id: drift run: | - if git diff --quiet test/compat/testdata/golden/v2/; then + # Use `git status --porcelain` rather than `git diff --quiet` + # because the latter ignores untracked files — on the initial + # capture (goldens dir previously empty) every JSON is a new + # file, not a modification to a tracked one. The porcelain + # output is non-empty iff anything under the path differs + # from HEAD, covering adds, modifications, and deletions. + if [ -z "$(git status --porcelain test/compat/testdata/golden/v2/)" ]; then echo "drift=false" >> "$GITHUB_OUTPUT" echo "No v2 drift detected." >> "$GITHUB_STEP_SUMMARY" else echo "drift=true" >> "$GITHUB_OUTPUT" { - echo "### Per-fixture golden diffs"; - git diff --stat test/compat/testdata/golden/v2/ + echo "### Per-fixture golden changes" + git status --porcelain test/compat/testdata/golden/v2/ + echo + echo "### Per-fixture diff stats (modifications only)" + git diff --stat test/compat/testdata/golden/v2/ || true } >> "$GITHUB_STEP_SUMMARY" fi - name: Open drift PR From 1ba5e79d49c009955160f8f1776fa98b327f6c66 Mon Sep 17 00:00:00 2001 From: Maximilian Date: Fri, 24 Apr 2026 17:35:56 +0200 Subject: [PATCH 13/18] fix(ci): use head_ref as PR base on pull_request events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On pull_request triggers github.ref_name is the synthetic /merge ref — not a real branch — so peter-evans/create-pull-request fails with "couldn't find remote ref" when it tries to rebase onto it. github.head_ref holds the PR's source branch, which is what we actually want. Observed on PR #77 bootstrap run (#24897788494) after the drift- detection fix: 8 goldens committed cleanly, then the PR-open step errored on git fetch 77/merge. --- .github/workflows/compat-golden-refresh.yml | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/.github/workflows/compat-golden-refresh.yml b/.github/workflows/compat-golden-refresh.yml index 5bdf37c..07e1452 100644 --- a/.github/workflows/compat-golden-refresh.yml +++ b/.github/workflows/compat-golden-refresh.yml @@ -152,14 +152,16 @@ jobs: commit-message: "chore: refresh v2 compat goldens" branch: chore/compat-golden-refresh delete-branch: true - # github.ref_name expands to `main` on the scheduled-cron path - # and to the dispatch-target branch on `workflow_dispatch` — so - # the PR targets the correct base in both flows. In particular - # this is what makes the Task 10 initial-capture flow work: - # dispatching against feat/pr-gated-v2-compat opens a PR - # against that branch (not main), so the captured goldens - # land on the feature branch before it merges. - base: ${{ github.ref_name }} + # Base branch picker by event type: + # - schedule / workflow_dispatch: github.ref_name is the + # branch the workflow is running against (`main` for + # scheduled runs; maintainer-chosen ref for dispatch). + # - pull_request: github.ref_name is the synthetic + # `/merge` ref (not a real branch) — we need + # github.head_ref, which is the PR's source branch. + # This ternary keeps a single rolling drift PR targeting + # whatever branch the workflow fired from. + base: ${{ github.event_name == 'pull_request' && github.head_ref || github.ref_name }} title: "chore: refresh v2 compat goldens" body: | Weekly (or on-demand) refresh of the pinned v2 compat probes. From 935e739a398d96afc9bd73b8de7ee91a851266fc Mon Sep 17 00:00:00 2001 From: phramz <2319696+phramz@users.noreply.github.com> Date: Fri, 24 Apr 2026 15:36:41 +0000 Subject: [PATCH 14/18] chore: refresh v2 compat goldens --- test/compat/testdata/golden/v2/bare.json | 9 +++++++++ test/compat/testdata/golden/v2/coverage-pcov.json | 9 +++++++++ test/compat/testdata/golden/v2/exclusion.json | 9 +++++++++ test/compat/testdata/golden/v2/ini-and-coverage.json | 9 +++++++++ test/compat/testdata/golden/v2/ini-file-development.json | 9 +++++++++ test/compat/testdata/golden/v2/multi-ext.json | 9 +++++++++ test/compat/testdata/golden/v2/none-reset.json | 9 +++++++++ test/compat/testdata/golden/v2/single-ext.json | 9 +++++++++ 8 files changed, 72 insertions(+) create mode 100644 test/compat/testdata/golden/v2/bare.json create mode 100644 test/compat/testdata/golden/v2/coverage-pcov.json create mode 100644 test/compat/testdata/golden/v2/exclusion.json create mode 100644 test/compat/testdata/golden/v2/ini-and-coverage.json create mode 100644 test/compat/testdata/golden/v2/ini-file-development.json create mode 100644 test/compat/testdata/golden/v2/multi-ext.json create mode 100644 test/compat/testdata/golden/v2/none-reset.json create mode 100644 test/compat/testdata/golden/v2/single-ext.json diff --git a/test/compat/testdata/golden/v2/bare.json b/test/compat/testdata/golden/v2/bare.json new file mode 100644 index 0000000..71c4609 --- /dev/null +++ b/test/compat/testdata/golden/v2/bare.json @@ -0,0 +1,9 @@ +{ + "php_version": "8.4.20", + "sapi": "cli", + "zts": false, + "extensions": ["amqp","apcu","ast","bcmath","bz2","calendar","core","ctype","curl","date","dba","dom","ds","enchant","exif","ffi","fileinfo","filter","ftp","gd","gettext","gmp","hash","iconv","igbinary","imagick","imap","intl","json","ldap","libxml","mbstring","memcache","memcached","mongodb","msgpack","mysqli","mysqlnd","odbc","openssl","pcntl","pcre","pdo","pdo_dblib","pdo_firebird","pdo_mysql","pdo_odbc","pdo_pgsql","pdo_sqlite","pdo_sqlsrv","pgsql","phar","posix","random","readline","redis","reflection","session","shmop","simplexml","snmp","soap","sockets","sodium","spl","sqlite3","sqlsrv","standard","sysvmsg","sysvsem","sysvshm","tidy","tokenizer","xml","xmlreader","xmlwriter","xsl","yaml","zend opcache","zend opcache","zip","zlib","zmq"], + "ini": {"date.timezone":"UTC","disable_functions":"","display_errors":"","error_reporting":"22527","expose_php":"1","log_errors":"1","max_execution_time":"0","max_input_time":"-1","memory_limit":"-1","opcache.enable":"1","opcache.enable_cli":"0","opcache.jit":"1235","opcache.jit_buffer_size":"256M","opcache.memory_consumption":"128","opcache.revalidate_freq":"2","opcache.validate_timestamps":"1","post_max_size":"8M","session.gc_maxlifetime":"1440","session.save_handler":"files","short_open_tag":"","upload_max_filesize":"2M","xdebug.mode":"","xdebug.start_with_request":""}, + "env_delta": ["COMPOSER_NO_AUDIT","COMPOSER_NO_INTERACTION","COMPOSER_PROCESS_TIMEOUT"], + "path_additions": ["/home/runner/.composer/vendor/bin","/opt/hostedtoolcache/setup-php/tools"] +} diff --git a/test/compat/testdata/golden/v2/coverage-pcov.json b/test/compat/testdata/golden/v2/coverage-pcov.json new file mode 100644 index 0000000..9d5b177 --- /dev/null +++ b/test/compat/testdata/golden/v2/coverage-pcov.json @@ -0,0 +1,9 @@ +{ + "php_version": "8.4.20", + "sapi": "cli", + "zts": false, + "extensions": ["amqp","apcu","ast","bcmath","bz2","calendar","core","ctype","curl","date","dba","dom","ds","enchant","exif","ffi","fileinfo","filter","ftp","gd","gettext","gmp","hash","iconv","igbinary","imagick","imap","intl","json","ldap","libxml","mbstring","memcache","memcached","mongodb","msgpack","mysqli","mysqlnd","odbc","openssl","pcntl","pcov","pcre","pdo","pdo_dblib","pdo_firebird","pdo_mysql","pdo_odbc","pdo_pgsql","pdo_sqlite","pdo_sqlsrv","pgsql","phar","posix","random","readline","redis","reflection","session","shmop","simplexml","snmp","soap","sockets","sodium","spl","sqlite3","sqlsrv","standard","sysvmsg","sysvsem","sysvshm","tidy","tokenizer","xml","xmlreader","xmlwriter","xsl","yaml","zend opcache","zend opcache","zip","zlib","zmq"], + "ini": {"date.timezone":"UTC","disable_functions":"","display_errors":"","error_reporting":"22527","expose_php":"1","log_errors":"1","max_execution_time":"0","max_input_time":"-1","memory_limit":"-1","opcache.enable":"1","opcache.enable_cli":"0","opcache.jit":"1235","opcache.jit_buffer_size":"256M","opcache.memory_consumption":"128","opcache.revalidate_freq":"2","opcache.validate_timestamps":"1","post_max_size":"8M","session.gc_maxlifetime":"1440","session.save_handler":"files","short_open_tag":"","upload_max_filesize":"2M","xdebug.mode":"","xdebug.start_with_request":""}, + "env_delta": ["COMPOSER_NO_AUDIT","COMPOSER_NO_INTERACTION","COMPOSER_PROCESS_TIMEOUT"], + "path_additions": ["/home/runner/.composer/vendor/bin","/opt/hostedtoolcache/setup-php/tools"] +} diff --git a/test/compat/testdata/golden/v2/exclusion.json b/test/compat/testdata/golden/v2/exclusion.json new file mode 100644 index 0000000..beb49ed --- /dev/null +++ b/test/compat/testdata/golden/v2/exclusion.json @@ -0,0 +1,9 @@ +{ + "php_version": "8.4.20", + "sapi": "cli", + "zts": false, + "extensions": ["amqp","apcu","ast","bcmath","bz2","calendar","core","ctype","curl","date","dba","dom","ds","enchant","exif","ffi","fileinfo","filter","ftp","gd","gettext","gmp","hash","iconv","igbinary","imagick","imap","intl","json","ldap","libxml","mbstring","memcache","memcached","mongodb","msgpack","mysqli","mysqlnd","odbc","openssl","pcntl","pcre","pdo","pdo_dblib","pdo_firebird","pdo_mysql","pdo_odbc","pdo_pgsql","pdo_sqlite","pdo_sqlsrv","pgsql","phar","posix","random","readline","redis","reflection","session","shmop","simplexml","snmp","soap","sockets","sodium","spl","sqlite3","sqlsrv","standard","sysvmsg","sysvsem","sysvshm","tidy","tokenizer","xml","xmlreader","xmlwriter","xsl","yaml","zip","zlib","zmq"], + "ini": {"date.timezone":"UTC","disable_functions":"","display_errors":"","error_reporting":"22527","expose_php":"1","log_errors":"1","max_execution_time":"0","max_input_time":"-1","memory_limit":"-1","opcache.enable":"","opcache.enable_cli":"","opcache.jit":"","opcache.jit_buffer_size":"","opcache.memory_consumption":"","opcache.revalidate_freq":"","opcache.validate_timestamps":"","post_max_size":"8M","session.gc_maxlifetime":"1440","session.save_handler":"files","short_open_tag":"","upload_max_filesize":"2M","xdebug.mode":"","xdebug.start_with_request":""}, + "env_delta": ["COMPOSER_NO_AUDIT","COMPOSER_NO_INTERACTION","COMPOSER_PROCESS_TIMEOUT"], + "path_additions": ["/home/runner/.composer/vendor/bin","/opt/hostedtoolcache/setup-php/tools"] +} diff --git a/test/compat/testdata/golden/v2/ini-and-coverage.json b/test/compat/testdata/golden/v2/ini-and-coverage.json new file mode 100644 index 0000000..e23d3da --- /dev/null +++ b/test/compat/testdata/golden/v2/ini-and-coverage.json @@ -0,0 +1,9 @@ +{ + "php_version": "8.4.20", + "sapi": "cli", + "zts": false, + "extensions": ["amqp","apcu","ast","bcmath","bz2","calendar","core","ctype","curl","date","dba","dom","ds","enchant","exif","ffi","fileinfo","filter","ftp","gd","gettext","gmp","hash","iconv","igbinary","imagick","imap","intl","json","ldap","libxml","mbstring","memcache","memcached","mongodb","msgpack","mysqli","mysqlnd","odbc","openssl","pcntl","pcre","pdo","pdo_dblib","pdo_firebird","pdo_mysql","pdo_odbc","pdo_pgsql","pdo_sqlite","pdo_sqlsrv","pgsql","phar","posix","random","readline","redis","reflection","session","shmop","simplexml","snmp","soap","sockets","sodium","spl","sqlite3","sqlsrv","standard","sysvmsg","sysvsem","sysvshm","tidy","tokenizer","xdebug","xdebug","xml","xmlreader","xmlwriter","xsl","yaml","zend opcache","zend opcache","zip","zlib","zmq"], + "ini": {"date.timezone":"UTC","disable_functions":"","display_errors":"","error_reporting":"22527","expose_php":"1","log_errors":"1","max_execution_time":"0","max_input_time":"-1","memory_limit":"256M","opcache.enable":"1","opcache.enable_cli":"0","opcache.jit":"1235","opcache.jit_buffer_size":"256M","opcache.memory_consumption":"128","opcache.revalidate_freq":"2","opcache.validate_timestamps":"1","post_max_size":"8M","session.gc_maxlifetime":"1440","session.save_handler":"files","short_open_tag":"","upload_max_filesize":"2M","xdebug.mode":"coverage","xdebug.start_with_request":"default"}, + "env_delta": ["COMPOSER_NO_AUDIT","COMPOSER_NO_INTERACTION","COMPOSER_PROCESS_TIMEOUT"], + "path_additions": ["/home/runner/.composer/vendor/bin","/opt/hostedtoolcache/setup-php/tools"] +} diff --git a/test/compat/testdata/golden/v2/ini-file-development.json b/test/compat/testdata/golden/v2/ini-file-development.json new file mode 100644 index 0000000..c184144 --- /dev/null +++ b/test/compat/testdata/golden/v2/ini-file-development.json @@ -0,0 +1,9 @@ +{ + "php_version": "8.4.20", + "sapi": "cli", + "zts": false, + "extensions": ["amqp","apcu","ast","bcmath","bz2","calendar","core","ctype","curl","date","dba","dom","ds","enchant","exif","ffi","fileinfo","filter","ftp","gd","gettext","gmp","hash","iconv","igbinary","imagick","imap","intl","json","ldap","libxml","mbstring","memcache","memcached","mongodb","msgpack","mysqli","mysqlnd","odbc","openssl","pcntl","pcre","pdo","pdo_dblib","pdo_firebird","pdo_mysql","pdo_odbc","pdo_pgsql","pdo_sqlite","pdo_sqlsrv","pgsql","phar","posix","random","readline","redis","reflection","session","shmop","simplexml","snmp","soap","sockets","sodium","spl","sqlite3","sqlsrv","standard","sysvmsg","sysvsem","sysvshm","tidy","tokenizer","xml","xmlreader","xmlwriter","xsl","yaml","zend opcache","zend opcache","zip","zlib","zmq"], + "ini": {"date.timezone":"UTC","disable_functions":"pcntl_fork,pcntl_waitpid,pcntl_waitid,pcntl_wait,pcntl_signal,pcntl_signal_get_handler,pcntl_signal_dispatch,pcntl_sigprocmask,pcntl_sigwaitinfo,pcntl_sigtimedwait,pcntl_wifexited,pcntl_wifstopped,pcntl_wifcontinued,pcntl_wifsignaled,pcntl_wexitstatus,pcntl_wtermsig,pcntl_wstopsig,pcntl_exec,pcntl_alarm,pcntl_get_last_error,pcntl_getpriority,pcntl_setpriority,pcntl_strerror,pcntl_async_signals,pcntl_unshare,pcntl_rfork,pcntl_forkx,pcntl_setns,pcntl_getcpuaffinity,pcntl_setcpuaffinity,pcntl_getcpu,pcntl_getqos_class,pcntl_setqos_class,","display_errors":"1","error_reporting":"30719","expose_php":"1","log_errors":"1","max_execution_time":"0","max_input_time":"-1","memory_limit":"-1","opcache.enable":"1","opcache.enable_cli":"0","opcache.jit":"1235","opcache.jit_buffer_size":"256M","opcache.memory_consumption":"128","opcache.revalidate_freq":"2","opcache.validate_timestamps":"1","post_max_size":"8M","session.gc_maxlifetime":"1440","session.save_handler":"files","short_open_tag":"","upload_max_filesize":"2M","xdebug.mode":"","xdebug.start_with_request":""}, + "env_delta": ["COMPOSER_NO_AUDIT","COMPOSER_NO_INTERACTION","COMPOSER_PROCESS_TIMEOUT"], + "path_additions": ["/home/runner/.composer/vendor/bin","/opt/hostedtoolcache/setup-php/tools"] +} diff --git a/test/compat/testdata/golden/v2/multi-ext.json b/test/compat/testdata/golden/v2/multi-ext.json new file mode 100644 index 0000000..71c4609 --- /dev/null +++ b/test/compat/testdata/golden/v2/multi-ext.json @@ -0,0 +1,9 @@ +{ + "php_version": "8.4.20", + "sapi": "cli", + "zts": false, + "extensions": ["amqp","apcu","ast","bcmath","bz2","calendar","core","ctype","curl","date","dba","dom","ds","enchant","exif","ffi","fileinfo","filter","ftp","gd","gettext","gmp","hash","iconv","igbinary","imagick","imap","intl","json","ldap","libxml","mbstring","memcache","memcached","mongodb","msgpack","mysqli","mysqlnd","odbc","openssl","pcntl","pcre","pdo","pdo_dblib","pdo_firebird","pdo_mysql","pdo_odbc","pdo_pgsql","pdo_sqlite","pdo_sqlsrv","pgsql","phar","posix","random","readline","redis","reflection","session","shmop","simplexml","snmp","soap","sockets","sodium","spl","sqlite3","sqlsrv","standard","sysvmsg","sysvsem","sysvshm","tidy","tokenizer","xml","xmlreader","xmlwriter","xsl","yaml","zend opcache","zend opcache","zip","zlib","zmq"], + "ini": {"date.timezone":"UTC","disable_functions":"","display_errors":"","error_reporting":"22527","expose_php":"1","log_errors":"1","max_execution_time":"0","max_input_time":"-1","memory_limit":"-1","opcache.enable":"1","opcache.enable_cli":"0","opcache.jit":"1235","opcache.jit_buffer_size":"256M","opcache.memory_consumption":"128","opcache.revalidate_freq":"2","opcache.validate_timestamps":"1","post_max_size":"8M","session.gc_maxlifetime":"1440","session.save_handler":"files","short_open_tag":"","upload_max_filesize":"2M","xdebug.mode":"","xdebug.start_with_request":""}, + "env_delta": ["COMPOSER_NO_AUDIT","COMPOSER_NO_INTERACTION","COMPOSER_PROCESS_TIMEOUT"], + "path_additions": ["/home/runner/.composer/vendor/bin","/opt/hostedtoolcache/setup-php/tools"] +} diff --git a/test/compat/testdata/golden/v2/none-reset.json b/test/compat/testdata/golden/v2/none-reset.json new file mode 100644 index 0000000..e06c761 --- /dev/null +++ b/test/compat/testdata/golden/v2/none-reset.json @@ -0,0 +1,9 @@ +{ + "php_version": "8.4.20", + "sapi": "cli", + "zts": false, + "extensions": ["apcu","core","date","filter","hash","igbinary","json","libxml","msgpack","openssl","pcntl","pcre","phar","random","redis","reflection","session","sodium","spl","standard","zlib"], + "ini": {"date.timezone":"UTC","disable_functions":"","display_errors":"","error_reporting":"22527","expose_php":"1","log_errors":"1","max_execution_time":"0","max_input_time":"-1","memory_limit":"-1","opcache.enable":"","opcache.enable_cli":"","opcache.jit":"","opcache.jit_buffer_size":"","opcache.memory_consumption":"","opcache.revalidate_freq":"","opcache.validate_timestamps":"","post_max_size":"8M","session.gc_maxlifetime":"1440","session.save_handler":"files","short_open_tag":"","upload_max_filesize":"2M","xdebug.mode":"","xdebug.start_with_request":""}, + "env_delta": ["COMPOSER_NO_AUDIT","COMPOSER_NO_INTERACTION","COMPOSER_PROCESS_TIMEOUT"], + "path_additions": ["/home/runner/.composer/vendor/bin","/opt/hostedtoolcache/setup-php/tools"] +} diff --git a/test/compat/testdata/golden/v2/single-ext.json b/test/compat/testdata/golden/v2/single-ext.json new file mode 100644 index 0000000..71c4609 --- /dev/null +++ b/test/compat/testdata/golden/v2/single-ext.json @@ -0,0 +1,9 @@ +{ + "php_version": "8.4.20", + "sapi": "cli", + "zts": false, + "extensions": ["amqp","apcu","ast","bcmath","bz2","calendar","core","ctype","curl","date","dba","dom","ds","enchant","exif","ffi","fileinfo","filter","ftp","gd","gettext","gmp","hash","iconv","igbinary","imagick","imap","intl","json","ldap","libxml","mbstring","memcache","memcached","mongodb","msgpack","mysqli","mysqlnd","odbc","openssl","pcntl","pcre","pdo","pdo_dblib","pdo_firebird","pdo_mysql","pdo_odbc","pdo_pgsql","pdo_sqlite","pdo_sqlsrv","pgsql","phar","posix","random","readline","redis","reflection","session","shmop","simplexml","snmp","soap","sockets","sodium","spl","sqlite3","sqlsrv","standard","sysvmsg","sysvsem","sysvshm","tidy","tokenizer","xml","xmlreader","xmlwriter","xsl","yaml","zend opcache","zend opcache","zip","zlib","zmq"], + "ini": {"date.timezone":"UTC","disable_functions":"","display_errors":"","error_reporting":"22527","expose_php":"1","log_errors":"1","max_execution_time":"0","max_input_time":"-1","memory_limit":"-1","opcache.enable":"1","opcache.enable_cli":"0","opcache.jit":"1235","opcache.jit_buffer_size":"256M","opcache.memory_consumption":"128","opcache.revalidate_freq":"2","opcache.validate_timestamps":"1","post_max_size":"8M","session.gc_maxlifetime":"1440","session.save_handler":"files","short_open_tag":"","upload_max_filesize":"2M","xdebug.mode":"","xdebug.start_with_request":""}, + "env_delta": ["COMPOSER_NO_AUDIT","COMPOSER_NO_INTERACTION","COMPOSER_PROCESS_TIMEOUT"], + "path_additions": ["/home/runner/.composer/vendor/bin","/opt/hostedtoolcache/setup-php/tools"] +} From d5d903dc3fbdaef4d523772468d9be2f69d7cdf3 Mon Sep 17 00:00:00 2001 From: Maximilian Date: Fri, 24 Apr 2026 17:39:51 +0200 Subject: [PATCH 15/18] chore: exclude v2 goldens from prettier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The JSONs under test/compat/testdata/golden/v2/ are captured from shivammathur/setup-php@ via test/compat/probe.sh. probe.sh emits compact single-line arrays; prettier wants one element per line. Every weekly refresh would otherwise force a follow-up reformat commit and cause lint churn on both the feature PR and every drift PR. Goldens are frozen captured data, not source — same rationale as gitignoring generated artifacts. --- .prettierignore | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .prettierignore diff --git a/.prettierignore b/.prettierignore new file mode 100644 index 0000000..a39e948 --- /dev/null +++ b/.prettierignore @@ -0,0 +1,6 @@ +# Captured v2 compat probes — generated by +# .github/workflows/compat-golden-refresh.yml via test/compat/probe.sh. +# Prettier's JSON style (one array element per line) would conflict +# with probe.sh's compact output, forcing every refresh PR to also +# reformat. These are frozen data, not source. +test/compat/testdata/golden/v2/ From f00a065e702694d1b84cae77d416b765c7deff5a Mon Sep 17 00:00:00 2001 From: Maximilian Date: Fri, 24 Apr 2026 17:45:00 +0200 Subject: [PATCH 16/18] fix(testsuite): mount golden/v2 subdir directly as /golden buildCellMounts was mounting test/compat/testdata/golden/ as /golden, so captured files at .../golden/v2/bare.json landed at /golden/v2/ bare.json inside the container. But runFixture looks for /golden/ .json (no v2/ subpath), causing every opted-in fixture to fail with "golden missing". Mount the v2 subdirectory directly so the container path matches what the consumer expects. Preserves the version namespace on the host side (room for future golden/v3/ if the upstream pin ever bumps in a non-backward-compatible way). Observed on PR #77 ci run #24898111265: all 8 compat fixtures failed with "golden missing for fixture X at /golden/X.json: run \`gh workflow run compat-golden-refresh.yml\`" despite goldens being present on the branch. --- internal/testsuite/runner.go | 9 ++++++++- internal/testsuite/runner_test.go | 4 ++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/internal/testsuite/runner.go b/internal/testsuite/runner.go index 699f301..febd32e 100644 --- a/internal/testsuite/runner.go +++ b/internal/testsuite/runner.go @@ -384,7 +384,14 @@ func buildCellMounts(opts *testOpts, _, _, _ string) ([]build.Mount, map[string] mounts = append(mounts, build.Mount{Host: compatMatrix, Container: "/compat-matrix.md", ReadOnly: true}, ) - goldenDir := filepath.Join(opts.AbsRepo, "test", "compat", "testdata", "golden") + // Goldens live under a version-namespaced subdirectory + // (test/compat/testdata/golden/v2/.json) so future alternate + // sources can coexist (e.g. golden/v3/ when a new upstream baseline + // is pinned). The container mount flattens that namespace to /golden + // so runFixture can read /golden/.json without knowing the + // version subpath convention. See internal/testsuite/testcell.go + // shouldRunCompat for the consumer path. + goldenDir := filepath.Join(opts.AbsRepo, "test", "compat", "testdata", "golden", "v2") if _, err := os.Stat(goldenDir); err == nil { mounts = append(mounts, build.Mount{Host: goldenDir, Container: "/golden", ReadOnly: true}, diff --git a/internal/testsuite/runner_test.go b/internal/testsuite/runner_test.go index 9490d19..9d72fb8 100644 --- a/internal/testsuite/runner_test.go +++ b/internal/testsuite/runner_test.go @@ -232,7 +232,7 @@ func TestBuildCellMounts_MountsCompatMatrix(t *testing.T) { // writeTestsuiteFixture already wrote docs/compat-matrix.md. matrixPath := filepath.Join(dir, "docs", "compat-matrix.md") - goldenDir := filepath.Join(dir, "test", "compat", "testdata", "golden") + goldenDir := filepath.Join(dir, "test", "compat", "testdata", "golden", "v2") if err := os.MkdirAll(goldenDir, 0o755); err != nil { t.Fatalf("mkdir golden: %v", err) } @@ -255,7 +255,7 @@ func TestBuildCellMounts_MountsCompatMatrix(t *testing.T) { t.Errorf("buildCellMounts did not mount docs/compat-matrix.md -> /compat-matrix.md (ro); mounts=%+v", mounts) } if !sawGoldens { - t.Errorf("buildCellMounts did not mount test/compat/testdata/golden -> /golden (ro); mounts=%+v", mounts) + t.Errorf("buildCellMounts did not mount test/compat/testdata/golden/v2 -> /golden (ro); mounts=%+v", mounts) } } From 98e152f6b9aa4076b74513c38a24f164049ffe92 Mon Sep 17 00:00:00 2001 From: Maximilian Date: Fri, 24 Apr 2026 17:49:39 +0200 Subject: [PATCH 17/18] fix(ci): replace jq backtick escapes with string concatenation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit jq's JSON-string parser rejects \` (backtick preceded by backslash) as an invalid escape sequence — only the standard JSON escapes (", \, /, b, f, n, r, t, u) are valid. Using literal backticks inside concatenated string segments produces the same markdown output without tripping the lexer. Observed on PR #77 ci run #24898366328: compat-report's "Render comment body" step exited with "jq: 8 compile errors" and the 8 per-fixture deviations never reached the sticky comment. --- .github/workflows/ci.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 46716d3..c549f55 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -236,7 +236,10 @@ jobs: echo "" for f in /tmp/compat-deviations/*.json; do cell=$(jq -r '.cell' "$f") - jq -r --arg cell "$cell" '.fixtures[] | "**Fixture \`\(.name)\` (\($cell))**\n" + (.deviations[:20] | map("- \`\(.Path)\`: ours=\`\(.Ours)\`, theirs=\`\(.Theirs)\`") | join("\n")) + "\n"' "$f" + # jq's JSON-string escape rules reject `\``; build the + # markdown with string concatenation and literal + # backticks instead. + jq -r --arg cell "$cell" '.fixtures[] | "**Fixture `" + .name + "` (" + $cell + ")**\n" + (.deviations[:20] | map("- `" + .Path + "`: ours=`" + .Ours + "`, theirs=`" + .Theirs + "`") | join("\n")) + "\n"' "$f" done echo "" echo "### What to do" From af7ecc516b012c269944d7764092db8c479ce59b Mon Sep 17 00:00:00 2001 From: Maximilian Date: Fri, 24 Apr 2026 18:01:42 +0200 Subject: [PATCH 18/18] fix(ci,docs): serialize refresh workflow + align spec with cleared marker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From the final holistic review on #77: 1. Add a concurrency group to compat-golden-refresh.yml so a Monday cron firing while a PR-trigger run is in flight doesn't race on the shared chore/compat-golden-refresh branch. cancel-in-progress is false because the workflow opens a PR — cancelling mid-run could leave it half-pushed. 2. Update the spec to match what the compat-report job actually does: on a red→green transition it REWRITES the sticky comment body to the "✅ Compat-diff cleared" marker (preserving audit trail) rather than DELETING the comment. The implementation chose the replace path deliberately; the spec text is now consistent. --- .github/workflows/compat-golden-refresh.yml | 12 ++++++++++ .../2026-04-24-pr-gated-v2-compat-design.md | 24 ++++++++++++------- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/.github/workflows/compat-golden-refresh.yml b/.github/workflows/compat-golden-refresh.yml index 07e1452..de35a7e 100644 --- a/.github/workflows/compat-golden-refresh.yml +++ b/.github/workflows/compat-golden-refresh.yml @@ -26,6 +26,18 @@ permissions: contents: write pull-requests: write +# Serialize refresh runs on the shared `chore/compat-golden-refresh` +# branch. Without this, a Monday cron firing while a PR-trigger run +# from a workflow-file edit is still in-flight would race on +# peter-evans/create-pull-request — both fighting over the same +# branch would either force-push each other's commits or open +# duplicate PRs. cancel-in-progress=false lets the earlier run +# finish cleanly before the newer one starts, which is the correct +# posture for a side-effecting (PR-opening) workflow. +concurrency: + group: compat-golden-refresh + cancel-in-progress: false + env: # Pinned v2 SHA. MUST match `## Pinning` in docs/compat-matrix.md. # The sanity-check job reads the markdown and refuses to run if the diff --git a/docs/superpowers/specs/2026-04-24-pr-gated-v2-compat-design.md b/docs/superpowers/specs/2026-04-24-pr-gated-v2-compat-design.md index 6548fee..3c2a9ee 100644 --- a/docs/superpowers/specs/2026-04-24-pr-gated-v2-compat-design.md +++ b/docs/superpowers/specs/2026-04-24-pr-gated-v2-compat-design.md @@ -181,7 +181,10 @@ compat-report (if: always() && needs.pipeline.result != 'skipped'): if deviations found: render markdown body (§7 template) + post-or-update sticky comment else if existing sticky comment present: - delete it (prior red PR is now green — clear stale state) + rewrite its body to "✅ Compat-diff cleared" marker (prior + red PR is now green — clear state by replacing the body, not + by deleting the comment, so the PR's discussion retains an + audit trail of when the gate flipped) else: no-op ``` @@ -190,8 +193,8 @@ The `compat-report` job is pull-request-scoped (gated on `github.event_name == 'pull_request'`); it doesn't run on push-to-main because there is no PR to comment on. Its gate is `always() && needs.pipeline.result != 'skipped'` so it runs whether -`pipeline` passed or failed, deleting stale comments when deviations -clear between pushes. +`pipeline` passed or failed, rewriting stale deviation comments to +the cleared marker when deviations clear between pushes. ### Refresh-time (weekly + on-demand) @@ -291,10 +294,11 @@ refreshed: . The comment is deterministic: re-running a failing PR re-posts the same content (sticky-key match). A PR that moves from red→green on the -compat gate has its comment **deleted** by the `compat-report` job (it -runs `if: always()` on every pipeline completion, not only on -failure; see §5 data flow), so the PR no longer carries stale -deviation text. +compat gate has its comment **replaced** with a "✅ Compat-diff +cleared" body by the `compat-report` job (it runs `if: always()` on +every pipeline completion, not only on failure; see §5 data flow), +so the PR no longer carries stale deviation text while retaining +an audit trail of the comment lifecycle. ## Failure policy @@ -341,8 +345,10 @@ brainstorm). ini-value default), observe the `compat-report` job posts a correctly-formatted sticky comment. - Push a follow-up commit removing the deviation, observe the same - `compat-report` job **deletes** the sticky comment (no stale - "deviations detected" text left behind on a now-green PR). + `compat-report` job **rewrites** the sticky comment body to the + "✅ Compat-diff cleared" marker (no stale "deviations detected" + text left behind on a now-green PR; the comment history itself + is preserved for audit). - Dispatch `compat-golden-refresh` workflow manually, observe the drift PR opens with reasonable body (or exits cleanly on no drift).