ci: wire up extension-update-test (0.1.0 -> stable) - #19
Open
jnasbyupgrade wants to merge 1 commit into
Open
Conversation
…ting Adds the committed install->guard->update->assert->run-suite script (bin/test_existing, modeled on cat_tools's bin/test_existing) plus a generic per-extension structural-diff tool (bin/structural_diff[.sql], copied near-verbatim from cat_tools -- it's already written generically off pg_depend's deptype='e' membership edge) and a new CI job that exercises the 0.1.0->stable update path end to end: install 0.1.0, plant + prove the dependency guard, ALTER EXTENSION UPDATE, structurally compare against a fresh "stable" install, then run the full suite in existing mode. No binary pg_upgrade job is added: object_reference has no view/function that SELECTs * over a system catalog in either its current or 0.1.0 install script (checked directly), so the cross-PostgreSQL-major risk that job protects against is low here. Left as noted future work rather than built preemptively -- see the ci.yml "Test strategy" comment and the PR description. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
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 |
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.
Stacked on #18 (link 3/3 of the update+upgrade testing effort -- completes
checklist item 7 of
advanced-extension-testing.md§10: CI wiring for theextension-update test, on top of #18's local
test/install/load.sqlthree-mode machinery).
What this adds
bin/test_existing-- the committed, parameterizedinstall→guard→update→assert→run-suite script (subcommands:
plant-guard,update,run-suite,update-scenario,diff-fresh),modeled on
Postgres-Extensions/cat_tools'sbin/test_existing.object_reference has only one real historical PGXN release (0.1.0) and
only one update script (a direct hop to
stable, no already-taggedintermediate landing version), so cat_tools's
prepare-old/update-check/update-check-versionsubcommands (built for testingmultiple historical origins, and for landing on an already-tagged
version where full parity is permanently unattainable) aren't built
here -- see the script's own header for the reasoning.
bin/structural_diff/bin/structural_diff.sql-- a genericper-extension structural comparison tool (function/view/table
definitions, comments, ACLs, driven off
pg_depend'sdeptype = 'e'membership edge), copied near-verbatim from cat_tools: the SQL file
is already written generically with no cat_tools-specific object names,
so it applies to object_reference as-is. Wired into
update-scenarioas
assert_matches_fresh, satisfying the doc's checklist item 12("structurally compare fresh vs. update before trusting it").
extension-update-testCI job: installs 0.1.0 (+ itssince-dropped
count_nullsdependency), plants + proves the dependencyguard,
ALTER EXTENSION UPDATEs to the current version, structurallycompares against a fresh
stableinstall, then runs the full suite inexistingmode (TEST_LOAD_SOURCE=existing CONTRIB_TESTDB=<db> EXTRA_REGRESS_OPTS=--use-existing PGXNTOOL_ENABLE_TEST_BUILD=no).Runs on a single PostgreSQL major (the newest supported), not the full
matrix -- 0.1.0's install script has no identified PG-version floor, so
crossing it against every major would multiply job count with no added
coverage.
all-checks-passed'sneeds:list updated to include the new job.ci.yml, per §6's guidance.A real per-repo adaptation from the cat_tools reference
bin/test_existing's dynamic version assertion derives the currentversion from
make -s print-EXTENSION_object_reference_VERSION, notprint-PGXNVERSION(which cat_tools's own script uses, and which theadvanced-extension-testing doc's §6d example also shows). object_reference
has already adopted the
stablepseudo-version scheme (#18):PGXNVERSION(meta.mk) is frozen at0.1.0, the last real numbered PGXNrelease used for git tagging/dist, and no longer tracks the current
build's
default_versiononcestableis in play. cat_tools'sPGXNVERSIONanddefault_versionstill happen to be the same value, sothis divergence wasn't visible there. Confirmed directly:
make -s print-PGXNVERSION→0.1.0,make -s print-EXTENSION_object_reference_VERSION→stable(the correct one toassert against).
pg_upgrade legs (§6c/§6c-bis): explicitly deferred, not omitted
No binary
pg_upgradejob is added in this PR. Checked directly: neitherobject_reference's current nor 0.1.0 install script has a view or function
that
SELECT *s over a system catalog (the doc's §7 risk -- catalogcolumns added/exposed/removed between PostgreSQL majors breaking a stored
view definition across
pg_upgrade). object_reference's own tables/viewsare ordinary user objects, so that specific risk is low here. Given that,
and that this is already the third PR in a stack, I judged the heavier
pg-upgrade-test/pg-upgrade-stepwiselegs not worth buildingpreemptively -- left as explicitly-noted future work (see the
ci.yml"Test strategy" comment) rather than silently skipped.
Verification
make lintclean.update-scenarioflow locally against a real PostgreSQL 17cluster before pushing: 0.1.0 install → guard planted + proven → update
to
stable→ structural diff clean → full suite green inexistingmode (7/7 pgTAP files passing).
RETURNvsEXITtraps undererrexit): deliberately corrupted acomment on a real object_reference table post-update to force a genuine
structural divergence, then ran
diff-freshdirectly. Confirmed (a) thescript exited non-zero (
FAIL: structural diff ..., exit code 1), and(b) the scratch
*__fresh_refreference database was still dropped bythe
EXITtrap despite the failure not being the trap-registeringsubshell's last command. Also confirmed via
trap -p EXITthat thescratch database name is baked into the trap as a literal at
registration time, not re-read from the variable when the trap fires.
(per this repo's own CLAUDE.md).
Test plan
make lintupdate-scenariodry run against a real clusterextension-update-testjob actually exercised,not just YAML-parsed)