Add binary pg_upgrade CI job (existing-mode suite + dependency guard) - #6
Closed
jnasbyupgrade wants to merge 1 commit into
Closed
Add binary pg_upgrade CI job (existing-mode suite + dependency guard)#6jnasbyupgrade wants to merge 1 commit into
jnasbyupgrade wants to merge 1 commit into
Conversation
…-mode suite Adds a pg-upgrade-test job (3 legs: 12->13, 16->17, 12->17) that installs extension_drop on an old cluster, plants a dependency guard, performs a real binary pg_upgrade, and runs the suite in TEST_LOAD_SOURCE=existing mode against the migrated database, with a dynamic version assertion and a docs-only cost gate for the new heavy job. Modeled on Postgres-Extensions/cat_tools's bin/test_existing and .github/scripts/pg_upgrade_cluster, substantially simplified: extension_drop has only ever shipped one real version, so no bridge leg or update-scenario machinery is needed. Verified end to end locally (PG12 -> PG17) before committing, which also surfaced a real conflict between a persistent dependency guard and test/sql/schema.sql's unconditional non-cascade drop, now handled by excluding schema.sql/zzz_build.sql from the existing-mode REGRESS list (both are already proven by the fresh-install `test` job). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Aug 4, 2026
Owner
Author
|
Migrated to a same-repo stacked PR on the main repo so GitHub can recognize the dependency chain natively (cross-fork stacked PRs aren't supported): Postgres-Extensions#20. Closing this fork-internal PR in favor of that one. |
jnasbyupgrade
added a commit
that referenced
this pull request
Aug 4, 2026
…, quoting-requiring schema test Builds the U&U (update & upgrade) test infrastructure that doesn't require a real second extension_drop version or pg_upgrade CI to already exist: - PGXNTOOL_ENABLE_TEST_INSTALL = yes, with test/install/load.sql as the committed-once installer for the extension (no test roles exist for this extension, so unlike cat_tools there's nothing role-related to add). - TEST_LOAD_SOURCE (fresh/update/existing) GUC/make-var switch, parse-time validated, exported unconditionally, read in load.sql without missing_ok. `existing` mode is fully exercised locally (verified against a real, already-installed database, including the failure path when the extension is genuinely absent). `update` mode is wired up and structurally verified end-to-end, but extension_drop has no real prior released version to update FROM yet -- the Makefile refuses to run it without TEST_UPDATE_FROM set explicitly, and no CI leg exercises it in this repo today. - Dependency guard (test/sql/dependency_guard.sql): a view depending on extension_drop__commands' row type blocks a non-CASCADE DROP EXTENSION; proven by actually attempting the drop and asserting failure, not assumed. - test/sql/schema.sql's custom-schema test names renamed to mixed case (requires identifier quoting), reusing its existing coverage rather than adding a new schema-testing dimension. - ci.yml: run `make test && make verify-results` instead of pg-build-test, so a real regression actually fails the build (pgxntool's .IGNORE: installcheck otherwise reports green regardless of test results, per RELEASE.md's existing note about PRs #6/#7). Moving the extension's own installation into test/install/load.sql required adapting every test file that used to install it per-test in a rolled-back transaction (test/deps.sql, test/sql/simple.sql, test/sql/schema.sql, test/sql/zzz_build.sql) to work against the new committed-once install instead, since an extension name is a database-wide singleton. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
jnasbyupgrade
added a commit
that referenced
this pull request
Aug 4, 2026
…redate this branch CI on this branch showed the switch to `make test && make verify-results` surfacing real pgTAP failures on PostgreSQL 9.3/9.6 (cat_tools/extension_drop never actually install there). Checked PR Postgres-Extensions#10's own baseline CI (Postgres-Extensions#10, run 30665031257): PG 9.3 and 9.6 already report "3 of 3 tests failed" in the raw job log there too, just silently reported as a passing check because pg-build-test's underlying `make test` hits pgxntool's `.IGNORE: installcheck` the same way. So this isn't a regression from this PR's own changes -- it's the exact masking problem RELEASE.md already documents, just now applying to a different, older part of the PG matrix than the PRs (#6/#7) it originally cites. Reverting the ci.yml step back to pg-build-test here keeps this PR scoped to test/install infrastructure; fixing cat_tools's install path on pre-PG10 belongs to whoever owns that dependency setup (PR Postgres-Extensions#10 or a follow-up), not this PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack position
Third link in a stack, all in
jnasbyupgrade/extension_tools:fix-cat-tools-install(base, upstream draft PR #10)→
test-install-foundation(fork-internal draft PR #4)→
pg-upgrade-ci(this PR, based ontest-install-foundation).None of these branches exist yet on
Postgres-Extensions/extension_tools, so this PR's own CI may not be meaningfully triggerable/mergeable until the earlier links land somewhere real — flagging this per the task instructions rather than guessing at why a run behaves oddly.What this builds
Following
~/advanced-extension-testing.md(reference implementation:Postgres-Extensions/cat_tools's currentmaster, specifically itspg-upgrade-testjob inci.ymlandbin/test_existing) and~/test-fixes.mditems 1/2/3/5/6:bin/test_existing— a committed, parameterized script (plant-guard,prepare,run-suitesubcommands) factoring the install→guard→pg_upgrade→assert→run-suite flow out of inline YAML..github/scripts/pg_upgrade_cluster— the binary-pg_upgrade cluster mechanics (recreate-old,upgrade), generic CI plumbing with nothing extension-specific in it.pg-upgrade-testjob inci.yml— 3 legs:12→13,16→17(adjacent-major legs near the floor/ceiling) and12→17(full span). Each leg installs extension_drop fresh on the old cluster, plants + proves a dependency guard, binarypg_upgrades straight to the new major, then runs the suite inTEST_LOAD_SOURCE=existingmode against the real migrated database.assert_version()derives expected version frommake -s print-PGXNVERSION, with empty-value guards on both sides.changesjob — cheap docs-only gate (fail-safe-firstGITHUB_OUTPUTwrite), gating only the new heavypg-upgrade-testjob (the pre-existingtestjob is already cheap, so it stays ungated).pg-upgrade-testneeds[changes, test]; workflow-levelconcurrencygroup withcancel-in-progress: true.all-checks-passedneeds list updated to[changes, test, pg-upgrade-test].What I took verbatim / adapted / skipped from cat_tools
.github/scripts/pg_upgrade_cluster: taken essentially verbatim — it's pure cluster mechanics (pg_ctlcluster/pg_createcluster/pg_upgrade against the pgxn-tools "test" cluster convention), nothing cat_tools-specific in it.bin/test_existing: adapted and substantially smaller. extension_drop has only ever shipped one real version (1.0.0 — seeHISTORY.asc/RELEASE.md; the only PGXN listing, 0.1.x from 2017, predates the current SQL entirely), so:pg_upgrade-unsafe old version exists to bridge from (per the doc's own guidance not to build this preemptively).TEST_LOAD_SOURCE=updatemode is wired up but unexercised for the same reason).old_pg/new_pgpairs viastrategy.matrix.include) and the dependency-guard technique itself (plant a view with a hardpg_dependedge on a stable extension member, prove a non-CASCADE drop is blocked) are the same pattern, unchanged.Something NOT in cat_tools's model — found by actually running this locally
cat_tools's control file pins
schema = 'cat_tools'(non-relocatable), so it has no test that freely retargets the install schema. extension_drop'stest/sql/schema.sql(added by the foundation PR) does — it proves the schema-targeting/quoting pipeline by unconditionally doingDROP EXTENSION extension_drop;(non-CASCADE) partway through. That collides for real with a persistent dependency guard: the very first statement inschema.sqlfailed with the guard's own2BP01error when I ran the full suite against a real pg_upgraded database locally.test/sql/zzz_build.sqlsurvives (it does a CASCADE drop) but adds adrop cascades to view ...NOTICE not present in the checked-in expected output. Fix:bin/test_existing'srun_suitecomputes an existing-modeREGRESSlist that excludesschemaandzzz_build(both already proven by the regular freshtestjob on every PostgreSQL major) — derived fromtest/sql/*.sql's actual contents, not a hardcoded list, so a future new test file is automatically included unless deliberately excluded. Flagging this loudly per the "ultimate goal" section: this is a genuinely new pattern (schema-flexible extension + persistent dependency guard) that cat_tools's own model doesn't need to solve.Deliberately skipped
testjob's PG list (9.3–17, extension_drop's own claimed floor) andpg-upgrade-test's floor (12) are genuinely different lists for a real reason, not accidental duplication:make installunconditionally buildscat_toolsfromPostgres-Extensions/cat_tools'smaster(PGXN's published cat_tools is a stale 2017 release extension_drop can't use — see the Makefile's owncat_toolstarget comment), and that current cat_tools requires PostgreSQL ≥ 12 for a fresh install (its ownMETA.json). Unifying two lists that are supposed to differ would be actively wrong here, so I left them separate and documented why inci.yml's top-of-file comment.pg-upgrade-stepwise(§6c-bis, every-major climb) — not built. extension_drop has no catalog-touching views/functions (per §7's risk profile), so the incremental value over the 3 existing legs looked low relative to the cost (installing every PG major, N sequentialpg_upgrades). Can be added later if that risk profile changes.all-checks-passedneeds-list self-check (cat_tools has a Python step verifying itsneeds:list matches the actual job set) — skipped to keep this PR's diff focused; I manually confirmed[changes, test, pg-upgrade-test]matches the actual 3 non-aggregator jobs.Local verification
This container has PostgreSQL 12 (port 5412) and 17 (port 5417) running — exactly the floor/ceiling this job uses. I did not use the persistent
mainclusters; instead created throwawaytest-named clusters (matching the pgxn-toolspg-startconventionpg_upgrade_clusterexpects) on a separate port, ran the actual committed scripts against them, and tore them down afterward:make install PG_CONFIG=.../12/bin/pg_config(installs extension_drop + git-built cat_tools 0.3.0 on PG12)bin/test_existing prepare extdrop_upg3— creates the DB,CREATE EXTENSION ... CASCADE, plants + proves the dependency guard (confirmed non-CASCADE drop blocked with2BP01).make install PG_CONFIG=.../17/bin/pg_config(extension_drop/cat_tools are pure SQL, so this just needs to be present in the new cluster's sharedir).INITDB_OPTS="--data-checksums --auth trust" .github/scripts/pg_upgrade_cluster upgrade 12 17— real binarypg_upgrade, completed clean.bin/test_existing run-suite extdrop_upg3— asserted version (1.0.0both sides), re-proved the guard survived pg_upgrade, ranmake test/make verify-resultsin existing mode (dependency_guard+simple,schema/zzz_buildexcluded per above) — all passed, exit code 0. Re-confirmed the guard was still present afterward.This process caught a real bug before it ever reached CI: my first draft passed
REGRESS=$regressthrough an unquoted flat string, which word-split the multi-valueREGRESS="dependency_guard simple"into a bogussimplemake target (No rule to make target 'simple'). Fixed by switching to a bash array (existing_args=(...),make test "${existing_args[@]}") — see the comment left at that fix site inbin/test_existing.No environment quirks unrelated to this change were encountered (the
12 → 13/16 → 17legs weren't locally re-verified since only PG12/17 binaries are installed in this dev container, but they exercise the identical script paths already proven end-to-end on12 → 17).CI status
Pushed to
jnasby/pg-upgrade-ci; watching the Actions run now and will report/fix any failures.