Uh oh!
There was an error while loading. Please reload this page.
Cross extension-update-test/pg-upgrade-test with TEST_SCHEMA - #32
Cross extension-update-test/pg-upgrade-test with TEST_SCHEMA#32jnasbyupgrade wants to merge 6 commits into
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
8647cf9 to
dffacb8Compare6b45ca9 to
995dcedCompare995dced to
01e6d51Compare01e6d51 to
d26f9b7Compare1d942ca to
1694173Compare1694173 to
08f22cfCompare08f22cf to
7207912Compare7207912 to
d76ac58Compared76ac58 to
a24874fComparea24874f to
4d0f14cCompare4d0f14c to
f96fb78Compare293db9f to
ee0f795Compareee0f795 to
8a8f05fCompare8a8f05f to
412c933Compare412c933 to
c159d3dComparef3da5c0 to
6e3cca1Compare…rce PG list Independent of the U&U testing work itself, but best done now that multiple CI jobs exist and before the next phase adds the most expensive one (a real pg_upgrade job): - `changes` job: computes the actual per-push diff and skips test/ extension-update-test/pg-tle-test entirely on doc-only pushes, always triggering itself (no workflow-level paths-ignore, which would leave all-checks-passed stuck Pending on doc-only pushes in branch protection). - Derives the supported-PostgreSQL-major list from ONE set of constants (NEWEST/FLOOR) in that same job, consumed by both the `test` and `extension-update-test` matrices via fromJSON - they can't silently drift onto different lists, and a new major is a one-line change. - `all-checks-passed`: single stable required-status-check name, with a self-check that its own needs list can't silently omit a newly-added job. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The org-wide Actions runner queue backs up easily; a draft PR being actively iterated on doesn't need the full PG matrix or the heavy pg-tle-test job re-run on every push. Add a newest_pg scalar output (single source alongside supported_pg) and reduce the test job's matrix to just that value on a draft PR, while skipping pg-tle-test (and any later heavy job following the same needs:[changes]/if: docs_only pattern) outright. Non-draft PRs and push events (e.g. post-merge on master) are unaffected.
Adds the pg-upgrade-test CI job: install 0.9.6 on an old PostgreSQL major, plant + prove a dependency guard, binary pg_upgrade to a newer major, ALTER EXTENSION UPDATE the migrated objects, then run the suite against the real upgraded database in existing mode. bin/test_existing is much smaller than the equivalent script would have been pre-test/install: only prepare-old and run-suite are genuinely external-to-pg_regress concerns (a real pg_upgrade binary run isn't something pg_regress can invoke itself), plus a small `update` subcommand for the post-upgrade ALTER EXTENSION UPDATE step. There's no update-scenario subcommand at all - that entire scenario is just `make test-update` now (test/install/load.sql's own 'update' mode, added in phase 3), since an in-place update has no external step to drive. run_suite() gates on plain `make test`, not the old belt-and-suspenders `make test && make verify-results` - pgxntool 2.3.0 (this repo's phase 0) already made `make test` itself exit non-zero on regression failures. Not yet crossed with TEST_SCHEMA - that's the next phase, once both this job and extension-update-test can cross it together. Verified locally against PG17 (prepare-old -> update -> run-suite, without a real pg_upgrade - this container's clusters are persistent shared infra, so the actual binary pg_upgrade leg is left for CI's ephemeral containers, same reasoning as the pg-tle-test work). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… not after Reorders prepare-old -> update -> pg_upgrade -> run-suite (was prepare-old -> pg_upgrade -> update -> run-suite). The old order proved pg_upgrade could migrate 0.9.6's frozen objects, then updated afterward - not actionable, since that version already shipped. This job's whole point is proving pg_upgrade correctly migrates the objects count_nulls' CURRENT code creates, which requires updating BEFORE the binary upgrade runs. make install (into the old cluster) already happens earlier in the job, so the current version's update scripts are on disk in time for the moved step. Updates the job's step names/comments and bin/test_existing's own file-header sequence description to match the new order.
Propagates the draft-PR gating from phase3.5-ci-hygiene to the pg-upgrade-test job introduced by this branch: same needs:[changes]/ if: docs_only pattern as pg-tle-test, so it gets the same && github.event.pull_request.draft != true guard.
…/shell loops, not a matrix Redesign of the original approach (which crossed TEST_SCHEMA into both jobs' CI matrices) per the same reasoning as the `test` job's collapse: a schema name is just an input the same assertions run against, not a real environment difference. - extension-update-test: added `make test-update-schema-all` (Makefile), the same TEST_SCHEMA loop as test-schema-all but with TEST_LOAD_SOURCE=update. Job step calls it instead of crossing schema into the matrix. - pg-upgrade-test: no make-level loop is possible here (bin/test_existing's steps are shell, not `make test`), so instead prepares TWO databases - count_nulls_upgrade_none and count_nulls_upgrade_quoted, one per TEST_SCHEMA value - before the SINGLE pg_upgrade call, which migrates the whole cluster (every database in it) in one pass. This is strictly better than a doubled matrix would have been: it also halves the number of actual pg_upgrade binary invocations (the single most expensive operation in this job), not just container/checkout overhead. Verified locally against PG17: prepare-old -> update -> run-suite passes for both databases in the same cluster/session (no real pg_upgrade run, same reasoning as prior phases - this container's clusters are persistent shared infra); make test-update-schema-all passes both TEST_SCHEMA legs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
6e3cca1 to
508e17aCompare
["", Quoted]TEST_SCHEMA matrix) is being replaced by an always-randomize-the-install-schema design instead. Left open for reference only.Stacked on #31 (phase 4: real pg_upgrade support). This is the novel enhancement the whole redesign was building toward.
Why this is novel
Checked directly: nobody in the org currently tests update/upgrade crossed with schema scenarios.
cat_tools' ownextension-update-test/pg-upgrade-testmatrices are PG-version-only (no schema axis at all - checked its actualci.yml).extension_toolshas no U&U testing whatsoever. So this is genuinely new coverage, not something to copy from a reference implementation.What changed
extension-update-test: addedschema: ["", Quoted]to the matrix + aTEST_SCHEMAenv var - the job's ownmake verify-results TEST_LOAD_SOURCE=updatepicks it up automatically (Make auto-imports matching-named environment variables).pg-upgrade-test: added the same schema axis.old_pg/new_pgwere already plain matrix dimensions (not aninclude:list), so adding a third axis cross-products cleanly into 4 legs (2 old_pg values × 2 schema values). Threadedmatrix.schemathrough tobin/test_existing'sprepare-old/run-suitecalls, previously hardcoded to"".Why this is "free": phase 2's schema-invariant assertion descriptions mean crossing either job with
TEST_SCHEMAneeds zero new expected-output files - every leg of every job (fresh, update, real pg_upgrade) × (no schema, Quoted schema) passes against the exact sametest/expected/extension_tests.out(+ the one genuine alternate from phase 2).Verification
Locally against PG17:
prepare-old→update→run-suitepasses end to end withTEST_SCHEMA=Quoted(previously only verified with an untargeted schema in phase 4).