Overhaul the test harness: committed-once install, load modes, existing-mode CI#44
Merged
jnasbyupgrade merged 3 commits intoJul 21, 2026
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:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
jnasbyupgrade
force-pushed
the
extract-test-harness
branch
from
July 21, 2026 20:36
dd1fd05 to
608a9ba
Compare
…ng-mode CI Replace the per-test extension install with a single committed-once installer and rewire the pg_upgrade / extension-update CI legs to exercise real migrated objects. test/install/load.sql (run once, committed, before the suite via pgxntool's test/install feature) is now the sole place that installs the extension and the test roles + grants. It supports three modes selected by the cat_tools.test_load_mode GUC that the Makefile exports through PGOPTIONS: fresh (CREATE EXTENSION at the current version), update (CREATE EXTENSION at TEST_UPDATE_FROM=0.2.2 then ALTER EXTENSION UPDATE to current), and existing (assert an already-installed current extension without touching it, for pg_upgraded / externally-updated databases). `make test-update` is a convenience wrapper for update mode. test/deps.sql no longer creates the roles; it only (re)sets the session-local psql variables the suite references. bin/test_existing carries the shared existing-mode journey used by CI (plant-guard, update, prepare-old, run-suite, update-scenario, update-check): install an old version, plant a dependency guard that blocks a non-CASCADE DROP EXTENSION, pg_upgrade / update, then run the suite via --use-existing against the real objects. ci.yml's pg-upgrade-test and extension-update-test call it; extension-update-test widens to PG12-18 for the 0.2.2->current suite while PG10 keeps the pre-0.2.2 legacy-script checks. The build-sanity check moves from test/sql/zzz_build.sql to pgxntool's native test/build/build.sql, retiring the old test/.build TEST_LOAD_SOURCE scaffolding (load_new.sql / load_upgrade.sql). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A database that reaches 0.2.x through the 0.2.0--0.2.2 update script carries an unguarded cat_tools.trigger__parse (missing the empty-args guard the fresh install has), which errors with "cannot determine type of empty array" when the existing-mode suite parses a trigger on newer PostgreSQL (surfaced on the 10->18 leg; PG11 masks it). That is a latent fresh-vs-update divergence in the shipped 0.2.x scripts, fixed by the 0.2.3->0.3.0 update, so the full 0.2.0 legacy journey belongs on the 0.3.0 branch. Fresh 0.2.2 has the guarded trigger__parse and is pg_upgrade-safe, and the 0.2.0-only install + 0.2.2->0.2.3 rebuild stay covered by extension-update-test on PG10. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reword the pg-upgrade-test old_pg=10 matrix comment: the leg SHOULD start from 0.2.0 (oldest-version install -> update to 0.2.3 -> survive binary pg_upgrade is the whole point). It is temporarily pinned to a fresh 0.2.2 because a 0.2.0-origin database cannot yet survive the journey -- the shipped 0.2.x update scripts leave cat_tools.trigger__parse with a hardcoded `EXECUTE PROCEDURE` (PG11+ needs `EXECUTE (?:PROCEDURE|FUNCTION)`) and no empty-args guard, breaking on newer PG. Restore 0.2.0 once the 0.2.2->0.2.3 repair covers trigger__parse. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jnasbyupgrade
force-pushed
the
extract-test-harness
branch
from
July 21, 2026 20:45
608a9ba to
c8263bd
Compare
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.
Replace the per-test extension install with a single committed-once installer and rewire the pg_upgrade / extension-update CI legs to exercise real migrated objects instead of a throwaway fresh install. Everything here is valid on the current master (0.2.3); it touches no extension SQL or versioned/
0.2.3files.The test-role improvement that originally rode along on this branch (single-source role names + intentionally quoting-requiring names + the
:"use_role"quoting fixes) has been split out into its own PR: #45 . This PR keeps master's current role names/quoting unchanged and is fully independent of that one.What changed
test/install/load.sql(+test/install/.gitignore): the sole, committed-once installer, run before the suite by pgxntool's nativetest/installfeature. It creates the extension and the test roles + grants once, committed, so every (rolled-back) test reads them without reinstalling. Three modes via thecat_tools.test_load_modeGUC that the Makefile exports throughPGOPTIONS:fresh(default):CREATE EXTENSION cat_toolsat the current version.update:CREATE EXTENSIONatTEST_UPDATE_FROM(default 0.2.2) thenALTER EXTENSION UPDATEto current -- same suite, same expected output, so an updated DB must behave identically to a fresh one.existing: assert an already-installed current extension without touching it (for pg_upgraded / externally-updated databases), paired with--use-existing.test/deps.sql: no longer creates the roles or runsCREATE EXTENSION; it only (re)sets the session-local psql variables the suite references, since load.sql now owns the committed install. Role names remain master's currentcat_tools_testing__use_role/__no_use_role.Makefile:TEST_LOAD_SOURCE(fresh/update/existing) wiring,PGXNTOOL_ENABLE_TEST_INSTALL=yes, GUC export, and atest-updateconvenience target. pgxntool is already 2.1.0 on master, so no vendor bump.bin/test_existing: the shared existing-mode journey used by CI --plant-guard,update,prepare-old,run-suite,update-scenario,update-check. It installs an old version, plants a dependency guard that blocks a non-CASCADEDROP EXTENSION, pg_upgrades / updates, then runs the suite via--use-existingagainst the real objects (a plainmake testwould silently test a fresh install)..github/workflows/ci.yml:pg-upgrade-testandextension-update-testnow callbin/test_existing.extension-update-testwidens to PG12-18 for the 0.2.2->current suite; PG10 keeps the pre-0.2.2 legacy-script checks. Thetestmatrix and the pg_upgrade older-PG coverage are unchanged.test/sql/zzz_build.sqlto pgxntool's nativetest/build/build.sql, retiring the oldtest/.buildTEST_LOAD_SOURCEscaffolding (load_new.sql/load_upgrade.sql).Verified locally
Fresh
make testandmake test-update(0.2.2->0.2.3) green on PG12 and PG17;bin/test_existing update-scenario(install 0.2.2 + plant guard + update + run suite via--use-existing) green, with the dependency guard blocking non-CASCADE drops as expected. No extension SQL or versioned files are touched.🤖 Generated with Claude Code