CI: binary pg_upgrade testing, docs-only gate, single-source PG matrix

[jnasbyupgrade]

Rebase note + follow-up fixes

Rebased onto the updated advanced-testing/foundation (this stack is sync-pgxntool-2.3.0 -> add-test-build (#26) -> advanced-testing/foundation (#22) -> this PR; coordinator is handling gh stack link separately). advanced-testing/ci had a merge commit bringing in the OLD advanced-testing/foundation history directly, so a plain git rebase tried to replay that entire flattened history against the new base -- used git rebase --onto <new-foundation> <old-foundation-tip> instead, to replay only this branch's own unique commits.

Real conflicts requiring judgment (not mechanical): .github/workflows/ci.yml (this PR's changes/pg-upgrade-test/all-checks-passed structure vs. the now-simplified lint+test structure inherited from #26/the pgxntool 2.3.0 sync -- merged both), test/expected/pgtap.out (this PR's ORDER BY fix for test/helpers/create.sql's security-definer-function check reorders that output; regenerated the fresh-mode expected file from a real run once the conflict was resolved -- pgtap_1.out, the existing-mode alternate, already had the correct sorted order and needed no change), and the same test/sql/install.sql/pgtap.sql mode-gating reconciliation #22 already needed against #26's trim (nothing new here, just replayed).

Fixes applied per this round of review, all verified locally before pushing

(a) pg-upgrade-test now gates on test, plus added concurrency. Previously needs: [changes] only, so a trivially-broken PR still burned the full binary pg_upgrade matrix. Now needs: [changes, test]. Added a top-level concurrency: {group: "${{ github.workflow }}-${{ github.ref }}", cancel-in-progress: true} so a superseded push cancels an in-flight run. all-checks-passed's own needs/self-check invariant is unaffected (job set didn't change shape).

(b) bin/test_existing's run_suite() was running the full suite twice. make test followed immediately by make verify-results -- verify-results already depends on test (pgxntool's base.mk), so every pg-upgrade-test leg paid for two full pg_regress --use-existing runs against the real migrated database. Dropped the redundant make test call. Verified this is still correct post-pgxntool-2.3.0 by reproducing a REAL pg_upgrade leg end to end locally (16 -> 17 at the time, using throwaway pg_createcluster data directories on custom ports -- never touching this container's shared clusters): recreated the old cluster with data checksums, installed pgtap + test_factory, prepared it via bin/test_existing prepare-old, created the new cluster, installed pgtap + test_factory there too, ran the actual pg_upgrade binary, started the new cluster, then ran bin/test_existing run-suite against it -- all 3 tests passed with the deduplicated run_suite().

(c) actions/checkout@v4 -> @v7 (current latest major -- v5/v6 have both released since v4), all 5 occurrences (lint, changes, test, pg-upgrade-test, all-checks-passed -- one more than the original "4" since the lint job wasn't part of this branch when that count was written; it arrived via the pgxntool 2.3.0 sync / #26 stacked below). Coordination note: PR #29 (ci/bump-actions-versions, someone else's, out of scope for me to touch) also bumps this same pin elsewhere in this file for the original, not-yet-restructured job set -- whichever of these two efforts merges second will need a final grep -rn 'actions/checkout@v4' sweep to make sure nothing was missed.

(d) NEWEST bumped 17 -> 18. The advanced-extension-testing doc's reference implementation (cat_tools) is already on NEWEST=18. Rather than assume PG18 installs cleanly in the pgxn/pgxn-tools image, confirmed it via an actual CI run first (pushed the NEWEST=18 bump alone, watched the new "PostgreSQL 18" job) -- it installed postgresql-18 18.4-1.pgdg13+1 via pg-start and the fresh-install suite passed. With 18 now the newest major, also shifted pg-upgrade-test's two legs to match its own stated rationale (widest catalog distance; most likely to hit a new major's catalog change first): oldest-to-newest is now 10 -> 18 (was 10 -> 17), and the newest-boundary leg is now 17 -> 18 (was 16 -> 17).

Verification after all fixes

• make lint and make verify-results (fresh mode) pass locally.
• bash -n on both shell scripts, YAML parse check on ci.yml.
• A full local binary pg_upgrade leg (see (b) above) passes with the deduplicated script.
• Pushed to this PR and watched: changes, all test matrix legs (10-18), pg-upgrade-test's two legs (10->18, 17->18), and all-checks-passed.

---

Summary

PR 2 of the advanced update+upgrade (U&U) testing stack, on top of #22
(advanced-testing/foundation, not yet merged -- this PR targets that
branch, not master). Implements the remaining CI-structure items from the
advanced-extension-testing pattern, modeled on Postgres-Extensions/cat_tools's
actual ci.yml/bin/test_existing (read directly off its master, not just
prose about it) and scoped down deliberately per the plan agreed before
starting.

Note: advanced-testing/foundation was pushed to this repo (unchanged, same
commit as the fork's branch backing #22) purely so this PR's base could
reference it directly -- it is not new work, just a ref needed for a clean
stacked diff. Once #22 merges, this PR should be retargeted to master as a
follow-up (not done here).

What shipped

• Fixed the double-triggered-CI bug: push scoped to branches: [master]; pull_request stays unrestricted. test_factory was explicitly
  named as one of the repos still needing this.
• Job-level changes/docs-only gate instead of workflow-level
  paths-ignore -- computes the real per-push diff, fail-safe (docs_only=false)
  written as the literal first line so any early exit/error leaves the safe
  default in place. Avoids the stuck-Pending-required-check trap a
  workflow-level paths-ignore would cause.
• Single source of truth for the supported-PostgreSQL-major list
  (NEWEST=18, FLOOR=10), derived
  once in the changes job and consumed via fromJSON by both the test
  and new pg-upgrade-test matrices.
• all-checks-passed gate, self-verifying its own needs: list matches
  the actual job set. This is the check to wire up as required in branch
  protection -- I don't have permission to change that setting myself.
• New pg-upgrade-test job: binary pg_upgrade legs (10 -> 18,
  17 -> 18, updated from the original 10 -> 17/16 -> 17 once NEWEST was bumped -- see the rebase note above) -- install the current version on an old cluster, pg_upgrade
  to a newer major, run the suite against the migrated objects in existing
  mode. No bridge step: test_factory has shipped only one version (0.5.0),
  so every leg installs current directly on the old cluster -- there's no
  older, pg_upgrade-unsafe install to carry forward.
• bin/test_existing (new, test_factory-specific) and
  .github/scripts/pg_upgrade_cluster (new, generic pg_upgrade CI
  mechanics) -- the install -> pg_upgrade -> assert -> run-suite flow
  factored into committed scripts instead of inline YAML per job, per the
  doc's own guidance. bin/test_existing is much smaller than cat_tools's
  own: no bridge/multi-origin subcommands, just prepare-old + run-suite.
• test job now gates on make verify-results, not pgxn-tools'
  pg-build-test. Found while rewriting this job: pg-build-test's own
  make installcheck || status=$? never actually catches a failure, because
  pgxntool marks installcheck .IGNORE -- make itself exits 0 there
  regardless of regression.diffs. The old CI was silently green on a real
  regression. PGXNTOOL_ENABLE_VERIFY_RESULTS was already pgxntool's own
  default, but I pinned it explicitly in the Makefile (matching
  ENABLE_TEST_INSTALL's existing explicit-over-implicit convention) so a
  future pgxntool default change can't silently disable the gate.
• Dynamic version assertion: bin/test_existing's assert_version
  derives the expected version from make -s print-PGXNVERSION, never
  hardcoded, with empty-value guards on both sides ("" != "" is false, so a
  broken extraction can't silently pass).

A real bug the local pg_upgrade dry run actually found

Per the brief, I ran the full old-cluster-install -> pg_upgrade ->
new-cluster-suite-run cycle locally before trusting any of this in CI (PG12
-> PG17 and PG12 -> PG16, using throwaway initdb data directories, never
touching the container's shared clusters) -- and it surfaced a real, if
low-stakes, bug: test/helpers/create.sql's security-definer-function check
had no ORDER BY, so its row order depended on pg_proc's physical layout.
That happens to match creation order on a fresh CREATE EXTENSION but is
not preserved by pg_upgrade's dump/restore (which reconstructs it in a
different, apparently name-sorted, order) -- producing a real but harmless
text diff (every individual pgTAP assertion still said ok, just reordered)
against the fresh-install expected output. Fixed with an explicit
ORDER BY p.oid::regproc::text.

Bonus: this makes ONE set of expected-output files valid for fresh,
existing, and pg_upgraded modes alike -- no third numbered alternate file
needed (unlike the doc's more general guidance for genuinely different
axes like TEST_SCHEMA), since the divergence here was pure incidental
non-determinism, not a legitimate different-but-correct scenario. Simpler
than it first looked once actually run.

Convergence / divergence from cat_tools PR #16 (and its master since)

Took near-verbatim (generic CI mechanics, no cat_tools-specific
content): the shape of .github/scripts/pg_upgrade_cluster
(recreate-old/upgrade subcommands, INITDB_OPTS convention, pg_upgrade
log capture for both PG17+'s pg_upgrade_output.d/ and older's CWD), the
changes job's docs-only fail-safe-first-line pattern, the
all-checks-passed self-verifying needs check, and the dynamic
version-assertion empty-guard pattern.

Adapted: bin/test_existing keeps only two subcommands
(prepare-old/run-suite) instead of cat_tools's six -- no
plant-guard/update/update-scenario/update-check, because
test_factory's dependency guard is planted and proved entirely inside
test/install/load.sql's existing-mode branch (from PR #22), not by this
script, and there's no update path to exercise yet. The changes job also
drops cat_tools's "find the last commit where real code changed" reporting
machinery -- useful polish, but not part of the checklist items this PR
scoped to; noting it here as a real simplification, not an oversight, in
case a human wants it added later.

Skipped, and why (all decided before starting, confirmed still correct
after doing the work):

• extension-update-test job -- test_factory has shipped only one version
  (0.5.0), so TEST_LOAD_SOURCE=update has no real historical update script
  to exercise yet. The mechanism exists (PR Add test/install foundation for update+upgrade testing (fresh/update/existing) #22); no CI job drives it.
• Bridge/multi-origin update machinery -- cat_tools-specific technical debt
  (recovering from old pg_upgrade-unsafe releases). test_factory has no
  catalog-touching views and only one shipped version, so there's nothing to
  bridge from.
• pg-upgrade-stepwise (every-major climb) -- test_factory has no
  catalog-internals-touching views/functions (no SELECT * over a system
  catalog), so the per-major-boundary risk this protects against is low. A
  human may disagree and ask for it later; flagging explicitly rather than
  silently omitting.
• pg_tle testing -- no evidence test_factory targets pg_tle/RDS/Aurora
  deployment.
• The stable pseudo-version -- real feature work, not part of a
  testing-infrastructure PR, not requested.

Verification

• make verify-results passes cleanly (fresh mode) on both PG12 and PG17.
• A full old-cluster-install -> pg_upgrade -> new-cluster-suite-run cycle,
  using the actual committed bin/test_existing and
  .github/scripts/pg_upgrade_cluster (not just ad hoc commands), passes
  with zero raw not ok TAP lines -- run twice, PG12->PG17 and PG12->PG16,
  in scratch data directories that never touched the container's shared
  clusters.
• .github/workflows/ci.yml parses cleanly under PyYAML; both new shell
  scripts pass bash -n.

Test plan

• CI runs green on this PR (test + pg-upgrade-test + all-checks-passed)
• Once Add test/install foundation for update+upgrade testing (fresh/update/existing) #22 merges, retarget this PR to master (follow-up, not done here)
• A repo admin adds all-checks-passed as a required status check in
  branch protection

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0abc71e2-e1b9-4371-84fb-60f5a9d8a7b9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[jnasbyupgrade]

First CI run failed everywhere (all 7 PG-matrix jobs + both pg_upgrade legs). Root-caused and fixed in 812e493:

1. test job: make verify-results alone races install vs installcheck under this container's ambient parallel make -- they're independent prerequisites of the same test target, so under -j, pg_regress could start (and fail with "extension ... is not available") before install's file copy finished. Fixed by running make install as its own separate invocation first -- two separate make processes can't race with each other.

2. pg-upgrade-test job: never installed pgtap system-wide on either cluster. This passed in my own local dry-run only by accident -- my dev container already had pgtap installed for the PG majors I happened to test with, from earlier unrelated testing. Caught it for real by deliberately clearing /usr/share/postgresql/16/extension/ (a major that container had never touched) and re-running the full recreate-old -> prepare-old -> pg_upgrade -> run-suite cycle end to end: it reproduced the exact same "extension pgtap is not available" failure CI hit, then passed once pgxn install pgtap --sudo --pg_config ... was added for both the old cluster (before prepare-old) and the new cluster (before its own make install -- pg_upgrade itself needs pgtap available on the new cluster, not just the post-upgrade suite run).

Both fixes verified locally end-to-end (real pg_ctlcluster/pg_createcluster/pg_upgrade, old=12/new=16 -- a pair this container had never exercised before) before pushing, and confirmed green on the actual PR checks afterward.

[jnasbyupgrade]

Superseded by #35 -- recreated as an upstream-branch PR so it can be part of a formal GitHub stack (gh stack link), which refuses PRs headed from a fork. Same content/commits, same CI results.