Repository navigation
Pick the test/install load mode server-side, and turn on ON_ERROR_STOP - #63
jnasbyupgrade merged 3 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: Team 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 |
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
1 similar comment
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
9734de4 to
40ec4ea
Compare
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
|
Claude finished @jnasbyupgrade's task in 4m 22s —— View job Claude's reviewTasks
SummaryThis commit ( Finding: stale "see X" pointers left behind by the
|
40ec4ea to
37f8e18
Compare
|
Claude finished @jnasbyupgrade's task in 22m 11s —— View job Claude's reviewTasks
SummaryRan a code review of this PR (
Both findings went through an independent verification pass before posting. Separately fact-checked that removing the |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| END LOOP; | ||
| END | ||
| $$; | ||
| \i test/helpers/test_user.sql |
There was a problem hiding this comment.
Correctness / test-fidelity regression: Adding \i test/helpers/test_user.sql here makes bin/test_existing's prepare-old install count_nulls as the Test user for count_nulls role instead of as the connecting (super)user.
For the pg-tle-test CI job's existing-mode leg — which, per bin/test_existing's own header comment, reuses prepare-old -> update -> run-suite with no real binary pg_upgrade in between — this means the extension's owner going into run-suite is now the test user itself. That silently makes test_user.sql's "extension already installed and owned by somebody else" branch (IF c_extension_owner IS NOT NULL AND c_extension_owner <> p_test_user THEN RETURN current_user) unreachable via this path: c_extension_owner at prepare-old time is NULL (nothing installed yet) so the switch-to-test-user proceeds and installs as that role; by run-suite time c_extension_owner already equals p_test_user, so the "stay put" branch that's supposed to model a real pg_upgrade's ownership loss never fires for the pg-tle-test job. No test fails — the existing-mode leg just silently stops exercising the ownership-mismatch scenario it's documented to model.
(For the real pg-upgrade-test job this is masked because a genuine binary pg_upgrade resets ownership to whoever performs the restore regardless of what prepare-old set it to — so the gap is specific to the pg_tle path.)
Verified: before this diff create_test_schema.sql did no role-switching at all (ran as the connecting role), and bin/test_existing's prepare-old/CI invocations pass no PGUSER/role override, so this is a genuine behavior change, not a false alarm.
There was a problem hiding this comment.
Claude here — this one's intended, not a regression, and applies to both PRs. prepare-old installing as the test user rather than superuser is the exact property this work exists to prove (count_nulls needs no superuser rights to install); the pg_upgrade leg still works because the owner short-circuit, now scoped to existing mode, leaves that later session running as the connecting role rather than switching it.
| SELECT pg_temp.count_nulls_load( | ||
| current_setting('count_nulls.test_load_mode') | ||
| , current_setting('count_nulls.test_existing_deploy') | ||
| ) AS count_nulls_loaded | ||
| \gset |
There was a problem hiding this comment.
Minor robustness regression: count_nulls.test_existing_deploy is now read unconditionally for every mode (as a plain argument to pg_temp.count_nulls_load), whereas before this refactor it was only read from inside the existing-mode branch (\if :count_nulls_existing_mode).
current_setting() is called here without missing_ok, so if this file is ever driven directly (e.g. psql -c "SET count_nulls.test_load_mode='fresh'" -f test/install/load.sql, bypassing this repo's Makefile) with only the load-mode GUC set, it now raises unrecognized configuration parameter "count_nulls.test_existing_deploy" even in fresh/update mode, where that GUC is semantically irrelevant and previously was never touched.
This is masked in the normal workflow because the Makefile unconditionally exports count_nulls.test_existing_deploy via PGOPTIONS (default 'filesystem') regardless of TEST_LOAD_SOURCE, so make test/make test-update never hits it — but it does broaden the "fail loudly on unpropagated GUC" behavior beyond what the surrounding comment describes, for any caller that invokes this file directly.
There was a problem hiding this comment.
Claude here — acknowledged, deferring; this one is #63-specific. #62 only reads count_nulls.test_existing_deploy inside its existing-mode branch, but #63's server-side mode dispatch (d1a820e) reads it unconditionally. The Makefile always exports it regardless of mode though, so every supported invocation still sets it - reading it unconditionally doesn't introduce a case where it's actually missing.
37f8e18 to
4acb9f3
Compare
|
Claude finished @jnasbyupgrade's task in 4m 36s —— View job Code review complete
SummaryI re-diffed Both previously-flagged findings remain accurate, so I'm not duplicating them, but noting current status:
I also checked No new correctness issues found in this PR's own diff. The |
4acb9f3 to
964e460
Compare
|
Claude finished @jnasbyupgrade's task in 10m 38s —— View job Code review complete
SummaryReviewed the current diff ( The refactor itself — moving Three findings posted as inline comments, all in the new
|
| RETURN current_user; | ||
| END IF; | ||
|
|
||
| IF current_setting('is_superuser') = 'on' THEN |
There was a problem hiding this comment.
Correctness: managed-cloud "superuser" roles can never bootstrap the test role.
The bootstrap block (create the role, GRANT CREATE ON DATABASE, GRANT USAGE ON SCHEMA tap) only runs IF current_setting('is_superuser') = 'on'. is_superuser reflects the actual rolsuper catalog bit of the current role — it is never on for AWS RDS/Aurora's rds_superuser, GCP Cloud SQL's cloudsqlsuperuser, or Azure Flexible Server's azure_pg_admin, none of which carry rolsuper (as this very function's own comment on c_managed_superuser_roles a few lines below explicitly notes).
Those roles are only ever consulted later, as a restriction (the test user must not be a member of one) — never as an alternate condition for allowing the bootstrap. So the very first run of this suite against a fresh managed-cloud database, connected as that platform's admin role, hits:
IF NOT EXISTS(...) THEN
RAISE EXCEPTION 'role "%" does not exist, and this session is not a superuser so it cannot be created', p_test_user;
even though the connecting role has ample privilege (it can create roles and grant CREATE ON DATABASE in practice) to do exactly that. Given the pg_tle CI job this suite ships (pg-tle-test) exists specifically because pg_tle is commonly deployed on exactly these managed platforms, this looks like a real gap rather than a merely theoretical one — worth gating the bootstrap on current_setting('is_superuser') = 'on' OR (SELECT bool_or(pg_has_role(current_user, r, 'MEMBER')) FROM unnest(c_managed_superuser_roles) r) (or similar) rather than is_superuser alone.
There was a problem hiding this comment.
|
|
||
| IF current_setting('is_superuser') = 'on' THEN | ||
| IF NOT EXISTS(SELECT 1 FROM pg_roles WHERE rolname = p_test_user) THEN | ||
| EXECUTE format('CREATE ROLE %I', p_test_user); |
There was a problem hiding this comment.
Correctness: check-then-create race on the test role.
IF NOT EXISTS(SELECT 1 FROM pg_roles WHERE rolname = p_test_user) THEN
EXECUTE format('CREATE ROLE %I', p_test_user);
END IF;
p_test_user is a fixed, non-randomized name ('Test user for count_nulls', unlike the randomized schema name), and this role is cluster-wide, not per-database. If two invocations of this suite run concurrently against the same Postgres cluster (different databases) — e.g. a developer running make test and make test-update locally at the same time, or any future CI layout that shares one instance across parallel jobs — both sessions can see "role does not exist" and both execute CREATE ROLE. The loser gets an unhandled duplicate_object error, and since this file runs under ON_ERROR_STOP, that whole install/load.sql (or deps.sql) session aborts.
Not exploitable in this repo's current CI (each job gets its own isolated pg-start cluster), but it's a real hazard for local/dev use and worth at least a comment, if not a CREATE ROLE IF NOT EXISTS / exception-swallowing guard.
There was a problem hiding this comment.
Claude here — acknowledged, not fixing now; applies to both PRs (shared code on #62). pgxntool's schedule-based install file completes before the parallel test/sql/ batch starts, so by the time any session could race on the check-then-create, the role already exists. Deferring rather than dismissing, and tracking it for follow-up since that ordering is an external invariant this file doesn't enforce itself.
There was a problem hiding this comment.
Claude here — the race is now documented in a comment at the site (a068c29). The actual hardening remains deferred and tracked separately rather than fixed here.
| IF EXISTS(SELECT 1 FROM pg_namespace WHERE nspname = 'tap') THEN | ||
| EXECUTE format('GRANT USAGE ON SCHEMA tap TO %I', p_test_user); | ||
| END IF; | ||
| END IF; |
There was a problem hiding this comment.
Efficiency (minor): the whole bootstrap block (role-existence check, GRANT CREATE ON DATABASE, tap-schema GRANT USAGE) re-runs on every per-test-file session, not just once — test/deps.sql \i's this file for every file under test/sql/ (currently just one file, but the mechanism re-executes per session regardless). Each of the GRANTs and EXISTS checks is idempotent so this is harmless, but it's a few avoidable catalog lookups/statements per session that only ever need to happen once, right after install/load.sql creates the role. Not blocking, just noting it as wasted work if this suite ever grows more test files.
There was a problem hiding this comment.
Claude here — acknowledged as real but non-blocking; applies to both PRs (shared code on #62). The bootstrap statements (role creation, grants) are idempotent, so re-running them every session is wasted work, not a correctness risk. Deferring that optimization and tracking it for follow-up.
964e460 to
0b817de
Compare
|
Claude finished @jnasbyupgrade's task in 7m 2s —— View job Code review complete
SummaryReviewed the current diff (
Both are genuine ordering/scoping regressions introduced by moving mode selection server-side, not issues with the |
| \i test/helpers/use_test_user.sql | ||
| \i test/helpers/extension_installer.sql |
There was a problem hiding this comment.
Correctness: stale-schema cleanup now runs after the switch to the test user, undoing the fix from 6529b72 ("Stale-schema cleanup moves ahead of the role switch, so it runs as the connecting role").
Here test/helpers/use_test_user.sql (the SET ROLE to the non-superuser test user) runs before test/helpers/extension_installer.sql is even loaded. count_nulls_load() — created and invoked later in this same file, i.e. after the switch — calls pg_temp.count_nulls_cleanup_test_schemas() internally (line 106), so cleanup executes as the test user, not the connecting role.
Compare with the sibling file test/helpers/create_test_schema.sql, which gets the order right: \i extension_installer.sql → SELECT pg_temp.count_nulls_cleanup_test_schemas() (still the connecting role) → \i use_test_user.sql (switch) → install.
extension_installer.sql's own header comment states the contract this file breaks: "every caller can run cleanup before switching to the test user … while installing after - a leftover schema can belong to any role, and only the connecting one is sure to be able to drop it."
Failure scenario: a prior fresh/update run crashes before its own teardown, leaving count_nulls test schema * behind owned by a superuser (or any role other than the test user) — exactly the scenario 6529b72 called out ("one created by a run predating this change belongs to a superuser"). The next run's cleanup now executes as the test user, which doesn't own that schema, so DROP SCHEMA %I CASCADE fails with a permission error instead of cleaning it up.
There was a problem hiding this comment.
Claude here — already fixed in d1a820e. Cleanup now runs before the role switch, mode-gated, so in existing mode it doesn't drop the schema the pg_upgrade run is being verified to have preserved.
| IF c_extension_owner IS NOT NULL AND c_extension_owner <> p_test_user THEN | ||
| RETURN current_user; |
There was a problem hiding this comment.
Correctness: this owner short-circuit isn't scoped to "existing" mode, so it can silently defeat the non-superuser guarantee in fresh/update mode too.
count_nulls_prepare_test_user() is called unconditionally at the top of test/install/load.sql, before the caller's count_nulls.test_load_mode is even read. If count_nulls already exists in the target database under some other owner (e.g. a leftover extension from an earlier interrupted/manual run, installed by a superuser), this check returns current_user without ever switching to the test user — regardless of whether the actual requested mode is fresh or update.
Failure scenario: a stale count_nulls extension (owned by, say, postgres) lingers in the test database when make test TEST_LOAD_SOURCE=fresh runs. count_nulls_prepare_test_user() sees c_extension_owner IS NOT NULL AND c_extension_owner <> p_test_user, returns current_user (the superuser), and the session never switches roles. count_nulls_load() then runs cleanup + fresh install entirely as the superuser, with no error — silently defeating the very property (superuser = false actually works) this test suite exists to prove.
There was a problem hiding this comment.
Claude here — fixed: count_nulls_prepare_test_user() now takes the load mode as an explicit argument, and the owner short-circuit only returns current_user when that mode is existing; otherwise it raises, naming both the owner and the test user. This applies to both PRs (the function lives on #62); the fix lands there and reaches #63 by rebase.
0b817de to
d1a820e
Compare
|
Claude finished @jnasbyupgrade's task in 3m 57s —— View job Code review complete
SummaryBranch has moved again since the last review (now Both previously-confirmed findings are now fixed:
Re-examined the one finding that looked like it might still be open — the "owner short-circuit isn't scoped to mode" point from the 20:55 review ( No new correctness issues found in No new inline comments posted; nothing new to flag. |
d1a820e to
3178cd9
Compare
|
Claude finished @jnasbyupgrade's task in 2m 21s —— View job Code review complete
SummaryBranch has moved again since the last review (now Both previously-open findings from the 20:55 review are now fixed:
Traced both fixes through all three call sites ( Not re-flagging (already covered in earlier rounds on this PR, unchanged by these two commits, and either minor or out of this PR's own diff):
Spot-checked No new correctness issues found; no inline comments needed this round. |
3178cd9 to
3b63cfb
Compare
|
Claude finished @jnasbyupgrade's task in 4m 8s —— View job Code review complete
SummaryThe branch has been rewritten again since the last review round (history restructured — commits reordered/rebased, current tip All previously-confirmed findings on this PR remain fixed in the current state:
I also re-checked Not re-flagging (deferred/accepted in earlier rounds on this PR, unchanged by the latest commits):
No new correctness issues found. No new inline comments needed this round. |
3b63cfb to
c7179df
Compare
|
Claude finished @jnasbyupgrade's task in 3m 45s —— View job Code review complete
SummaryBranch has moved since the last review round: two new commits,
All previously-confirmed findings on this PR (cleanup-order-before-role-switch, the owner short-circuit scoped to One new finding, posted inline: No other new correctness issues found. The |
| # CREATE EXTENSION count_nulls at VERSION, into a freshly, randomly | ||
| # generated schema - shared with test/install/load.sql's own fresh/update | ||
| # installs via test/helpers/create_test_schema.sql (see that file for the | ||
| # full rationale). | ||
| # full rationale). This invocation sets no count_nulls.test_load_mode GUC, | ||
| # so count_nulls_load_mode is supplied here instead: it's a genuine fresh | ||
| # install (prepare-old), never 'existing'. |
There was a problem hiding this comment.
Stale pointer, in a comment this PR's own commit (06bce4e) just edited: "shared with test/install/load.sql's own fresh/update installs via test/helpers/create_test_schema.sql (see that file for the full rationale)" is no longer true. test/install/load.sql doesn't touch create_test_schema.sql at all anymore — it \i's test/helpers/extension_installer.sql directly (test/install/load.sql:26), and that's also where the shared cleanup/install logic and its rationale actually live now. create_test_schema.sql's own header already reflects this correctly: "the work itself lives in test/helpers/extension_installer.sql."
This sentence predates this PR (it was flagged as stale in an earlier review round, when this file wasn't part of the diff yet), but 06bce4e added two new sentences immediately after it in this same comment block without correcting the one that was already wrong — so it's now shipping inaccurate documentation as part of this PR's own change, not just inheriting it unmodified.
Fix stale create_test_schema.sql pointer in bin/test_existing →
There was a problem hiding this comment.
Claude here — confirmed stale: test/install/load.sql \i's test/helpers/extension_installer.sql directly and doesn't reference create_test_schema.sql anywhere. The comment now points at test/helpers/extension_installer.sql as the shared install logic (and the rationale), and describes create_test_schema.sql as the thin wrapper that calls it, kept as a file only so this script can invoke it standalone; the two count_nulls_load_mode sentences are unchanged, since they were correct. Fixed in bd1a7b1.
test/install/load.sql's RAISE EXCEPTIONs were decorative. pg_regress resolves its expected and result paths to the same file, so the diff is self-identical no matter what, and a psql that reports an error, carries on and exits 0 leaves pg_regress nothing to notice. ON_ERROR_STOP is what makes the file fail the way its own header says it does. That couldn't just be switched on, because \if is psql 10 and CI covers back to 9.4, where psql reports it as an invalid command - and then, having skipped nothing, runs every branch. On those versions load.sql already behaved badly, silently: the existing-mode assertion fired and errored in every mode, and update mode installed 0.9.6, updated it, then re-entered create_test_schema.sql, whose stale-schema cleanup dropped that schema CASCADE - taking the just-updated extension with it - and installed current fresh instead. `make test TEST_LOAD_SOURCE=update` on 9.4/9.5/9.6 has therefore never once exercised the update path. Under ON_ERROR_STOP the same \if would have aborted those legs outright. So the mode selection moves server-side, into pg_temp.count_nulls_load(), leaving load.sql with no branching of its own - matching what test/helpers/test_user.sql already does, for the same reason. The schema creation and install it shares with bin/test_existing's prepare-old moves to test/helpers/install_extension.sql, which only defines the function; test/helpers/create_test_schema.sql is now just the call, and exists as a file purely because prepare-old invokes it as a standalone psql -f. Drops the client_min_messages suppression that used to wrap the ALTER EXTENSION UPDATE. Postgres already raises it to at least WARNING for the duration of an update script and restores the caller's setting afterwards, so it was redundant, and being unconditional it lowered the level for a caller who had set something stricter. Confirmed: update mode still emits no NOTICEs without it.
test/install/load.sql no longer goes through create_test_schema.sql; both it and this script reach the shared install logic via test/helpers/extension_installer.sql, which is also where the rationale now lives. create_test_schema.sql is just the wrapper that calls it, kept as a file so prepare-old can run it standalone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
c7179df to
bd1a7b1
Compare
|
Claude finished @jnasbyupgrade's task in 2m 1s —— View job Code review complete
SummaryReviewed the current diff (2 commits on top of All previously-confirmed findings on this PR remain fixed:
Not re-flagging (acknowledged/deferred in earlier rounds on this PR, unchanged):
One very minor, non-blocking observation not worth an inline comment: in No new correctness issues found. No inline comments needed this round. |
Note in a comment that the CREATE ROLE guard in count_nulls_prepare_test_user() is not atomic, and why that's safe today: load.sql finishes before pg_regress starts the concurrent test/sql/ sessions, so nothing races to create the role.
|
Claude finished @jnasbyupgrade's task in 1m 3s —— View job Code review complete
SummaryThe branch gained exactly one new commit since the last review round: The comment documents the check-then-create race on All previously-confirmed findings on this PR remain fixed, unaffected by this commit:
Not re-flagging (acknowledged/deferred by the author in earlier rounds, now further justified by this commit's own comment):
No new correctness issues found. No inline comments needed this round. |
e80485e Release 2.4.0 (#135) cca6885 Use the _PGXNTOOL_/_ prefix for internal test-gate variables and targets (#130) 88b3c11 Error when PGXNTOOL_ENABLE_TEST_INSTALL=yes but test/install/ is empty (#131) 065dd27 Validate PGXNTOOL_VERIFY_RESULTS_MODE (#132) 9cb2a53 Reject PGXNTOOL_PGTLE_VERSION values that aren't an exact pg_tle range name (#134) 51062d3 Fix `make test` failing when test/sql/ does not exist (#133) 5e5f002 check-test-install-error-stop.sh: accept \include and anchor patterns (#128) c25cdf4 Fix stale and inaccurate user docs (#129) f6fda71 Require HISTORY.asc issue numbers in ascending order (#125) 9b65063 Warn when two DOCS entries install to the same filename (issue #115) (#124) e8ed42d Guarantee test/install/*.sql schedule ordering (#120) 51c75ac Add post-tag-version-bump to freeze default_version after a release (#95) dffddf3 Only let an explicit ON_ERROR_STOP excuse a later turn-off in test/install (#122) 52b22fd Let `make results` seed a new test's first expected output (#121) af25072 CLAUDE.md: fix monitor-ci.sh doc mismatch, no --commit flag exists (#116) a3b85fe Rename build-results target to results-build (#123) 7a5f724 Import Postgres-Extensions/ai's shared CLAUDE.md into dev-facing docs (#117) 0a6db44 Gate the main test suite on test-build; add build-results; require ON_ERROR_STOP in test/install (#109) 59bb298 Add PGXNTOOL_ENABLE_FS_INSTALL and PGXNTOOL_ENABLE_PGXN_INSTALL (#91) bedf9b6 Add `make print-pgtle` target for combined pg_tle install files (#94) 5aafc8f ci: pin claude-code-action to a SHA in claude-code-review.yml (#112) 0d6bef9 Rename internal-only variables to the _PGXNTOOL_ shape (#93) 9fa8ae0 ci: bump GitHub Actions to latest versions (#98) 5d1d337 Fix claude-code-review.yml trust gate: check PR author, not head repo owner (#102) 2f36a12 Document `superuser`/`trusted`; split general PostgreSQL guidance into its own docs (#113) c50d8e6 claude-code-review.yml: allow the inline-comment MCP tool so review comments post individually (#105) a6d3287 Release 2.3.0 (#89) 1a7ee20 CLAUDE.md: fix pre-existing documentation-accuracy errors (#88) 35fd187 Fix DATA wildcard skipping historical full-install version scripts (Postgres-Extensions#48) (#80) f9ce2a2 Fix installcheck running before install; make test now fails on regressions (#83) 3ceb930 Rename PGTLE_VERSION to PGXNTOOL_PGTLE_VERSION (#85) 2de8429 claude-code-review.yml: add missing --comment flag (#84) 957bdf6 Restrict @claude mention trigger to a trusted account (#82) b67064b Release 2.2.0 (#77) d949d6d Fix stale README/CLAUDE.md docs; rename internal-only test shim (#76) 6d7822c ci: grant actions:write so bun cache save succeeds (#75) 2738f59 README.asc: fix stale docs, document undocumented API surface (#74) d1d3bee Add make pgxntool-version to print the embedded pgxntool version (#73) 493a0f8 Fix parallel-build SQL corruption, remote hardcoding, and add safety checks (Postgres-Extensions#61) 3b0e3de Fix make pgtle PGTLE_VERSION=X being silently ignored (Postgres-Extensions#66) 21b35ef ci: skip paired-test-PR requirement for doc-only PRs (#71) 55efd03 Rename EXTENSION_VERSION_FILES to EXTENSION__CURRENT_VERSION__FILES (#69) 9d0a856 Fix pgtle.sh rejecting non-numeric version aliases like 'stable' (#57) (Postgres-Extensions#63) 9c2f75f Docs: versioned SQL file tracking tradeoffs and update-testing pattern (Postgres-Extensions#60) c38cf2a ci: never check out fork PR head in claude-code-review pull_request_target job (#70) aab8f2b pgtle_versions.md: fix wrong uninstall_extension() version claim (Postgres-Extensions#59) e2c9174 README.asc: make tag creates a git tag, not a branch (Postgres-Extensions#58) eb84bc6 Stamp 2.1.0 9fbe6f4 Fix results ordering, control file whitespace, ENABLE_* override, debug levels (Postgres-Extensions#31) 88bb4f2 Add Claude Code GitHub Actions workflows (Postgres-Extensions#41) af5bbbb ci: pass repo owners to run-tests.yml for fork-account matching (#40) c7928af Fix repo-root guard to work inside a git worktree (Postgres-Extensions#39) b062fca ci: point reusable test workflow at @master b6cdbfd Add CI workflows and multi-session PR guard (Postgres-Extensions#33) 1ba0987 Stamp 2.0.3 1931cbe Fix pgxntool-sync remote and make it runnable without make (Postgres-Extensions#37) 8176304 Stamp 2.0.2 3e142ab Fix parse_control_file: remove comments before stripping quotes (Postgres-Extensions#27) cacc301 Stamp 2.0.1 bf1db6b Fix bash 3.2 / Linux compatibility issues (Postgres-Extensions#26) 62d0fcb Fix broken ifeq for --load-language=plpgsql on PG < 13 (Postgres-Extensions#24) 121f0b3 Stamp 2.0.0 ad3ca7e Remove .source support; add test/install, test/build, and verify-results (Postgres-Extensions#18) c010cf8 Fix bash 3.2 compatibility (Postgres-Extensions#23) abeb9d3 Remove .source file support from pg_regress integration (Postgres-Extensions#22) 08c1879 Stamp 1.1.2 6e0dad2 Fix double --dbname bug that defeated unique test database names 639756c Stamp 1.1.1 6ba3176 Fix pg_tle exception handler and empty upgrade files (Postgres-Extensions#15) 3b8cb2a Stamp 1.1.0 550a901 Remove commit.md (maintained in pgxntool-test) d73ca93 Add unique test database names to prevent conflicts (Postgres-Extensions#13) 9b344be Add update-setup-files.sh for 3-way merging after pgxntool-sync (Postgres-Extensions#12) ab7f6e2 Stamp 1.0.0 3a571ba Add pg_tle support and modernize test infrastructure (Postgres-Extensions#11) b96ea6d Add support for Claude code; build and doc improvements (Postgres-Extensions#9) e9c24de Fix pg_regress on versions > 12 (Postgres-Extensions#5) c0af00f Improvements to HISTORY.asc 6e8f2a7 Allow use of sudo when installing an extension 705f1ec Don't run clean as part of make test 370fa8e Create test/sql during setup 890053c Fix bug with DOCS 5c2173e Fix DOCS variable d8bf8ed Update HISTOROY f92d493 Add support for asciidoc dbdd978 Improve asciidoc handling 90bbfa5 Merge branch 'stable' 2f11ef0 Support 9.2 bceb84d Don't over-write DOC 1b97c8c Add pgtap as an installcheck dependency cd24b0f Reduce verbosity of test setup 48dce67 Install tap before runing deps.sql 6cb4302 Add input files as deps to installcheck c0e70ca Merge branch 'release' 899233d Merge branch 'master' into stable e8dff79 Update HISTORY f49e6e3 Fix version number handling ed515aa Remove errant . 8856f1e Decimal point doesn't work, just multiply MAJORVER by 10 35d13b3 Switch to leaving decimal point in MAJORVER 8c59619 Merge branch 'master' into stable 17ee75a Fix typo 5c3032f Add EXTENSION_SQL_FILES variable dcac898 Document the test targets e4884c2 Merge branch 'master' into stable c05f446 Clarify current version d2e6c79 Update HISTORY c3c1a08 Switch variable name for versioning aa93684 Merge branch 'release' 940f35b Merge branch 'master' into stable e6205f8 Remove merge conflict 1f5fcc5 Update HISTOROY 8b8e9b5 Remove invalid options from git subtree pull 2c1e313 Merge branch 'release' 227f558 Merge branch 'release' into stable e5f78b3 Update history e711eef Need to install upgrade scripts 102a856 Favor upstream changes in case of conflict eda5f38 Merge branch 'release' 18532a9 Consistent spelling of license 7d3f6ad History for 0.1.8 3264d28 Silence meta output a4ea40b Add expected/ as a dep 732f534 Handle alpha and beta versions 82216b6 Fix test directory handling 14e4ebc Distribution names can't contain spaces either 4aff20a Ignore meta.mk 04713aa Refactor; add PGXN/PGXNVERSION variables 02ae242 Move key to a safe location cbe8921 Change names to support new tools 3765700 Switch to generating a meta.mk file for Make dependencies on META.json 2e78459 Merge branch 'master' of github.com:decibel/pgxntool 2b8e24a History for release aac4331 Ignore ALL .asc files 2b46bda Merge pull request #3 from pnorman/patch-1 866bbe5 Use bash for setup.sh 97aaf7f Fix .gitattributes c8c4727 Merge branch 'stable' 4a57b43 Pull pgxntool from ../pgxntool stable b877657 Add note about what pgxntool is 13b0265 Test ignoring PGXNtool README b94e2f4 If output dir is needed but doesn't exist, create it. d173527 Fix directory name adc4348 Add test/sql as dep. Clean up directory creation. 0af7768 Create test output stuff if missing 798b6f4 Merge branch 'master' into stable cf42f7b Add git status output 3746eea Fix shell bug 616741a Ensure repo is empty c35e846 Add setup.sh to install instructions cf6fb04 Add make help 3b23055 Merge branch 'release' 905922e Merge branch 'master' into stable 93ff527 Leave pgTap being loaded by setup.sql c36180f Squashed commit of the following: 1dfff38 Improve warning f6464b9 Add option to sync stable 749ff01 Add test/..../finish.sql d71400c Merge branch 'release' 15d2567 Pull pgxntool from ../pgxntool master 732514e Fix $(DATA) 0c84d43 Improve print- recipe 6f7701e Merge branch 'master' into stable 5942cc9 Fix tap schema a8fa40e Move deps to a file c9a0b40 Add pgTap to deps a0ab465 Fix REGRESS rules 882447c Fix test file detection d152f6e Merge branch 'master' of github.com:decibel/pgxntool a7a7dfe Formatting 393f301 Unset MODULES if it's empty 84fa612 Always attempt to build source files 9922a17 Adjust META.in.json so it's valid 411dfaf .source inputs need to be in an input directory 03b542a Fix .source test file support cef41f7 Support .source test files 3a08445 Make testing more extensible Add TESTDIR and TESTOUT variables Explicitly tell pg_regress where to put output acc8c17 Minor tweak to refer to new features. ea3c67f Merge branch 'release' dce257a Merge branch 'test' 0596bdd Merge stable: Fix for pgtap, keep public in search path 5e6305e Merge branch 'master' into stable 4473ed5 Fix problem with pgtap dependencies. d043f2a Keep public in the search path for TAP c38a392 Merge branch 'master' into stable c6a31f0 Merge branch 'test' into stable ac48ccf Merge branch 'release' 6a10733 Merge back from release 8d9f9b6 Fix README.asc after merge aa3ea7f Merge branch 'test' b0bf5b1 0.1.2: Add a bunch of features 4fb946f Don't export README c17abbe Suggest initial setup is a squash commit. f99608e Ready to test 0.1.2 e895a08 Have setup add META.json to git 1629702 Fix off-by-1 in tail f133686 Deps issues ea3e515 Fix META.json build issues ecd0291 Make META.json variables recursive 5feee21 Copy META.in.json over. 979d836 Merge branch 'test' 77cd2da Support empty doc directory. Other improvements 1b32e06 Improve setup commit message 1ab4aa3 Add misssing file f3f28d2 First stab at build_meta.sh 963b040 Missing comma b984240 Don't puke if test/pgxntool already exists 5ade3be Fix file tests 9e97951 Better error handling; safecp() be55d12 Make sure we're not accidentally ignoring META.json 48df320 Add template META.in.json 5f0dcd9 Update final message 992dca3 Add git stuff c7bae04 Move .gitignore to a file c05317b Add target to list all targets da11d03 Add test helpers 1d1935f Add .gitignore 4632361 Fix typo 5ec6aa4 Add license and README d27b1e2 Add dist-only rule 57c52fa Fix variable reference bc0c837 Fix file path problem e4bafbb Add sync commands 5069d9c Add JSON.sh 2ade7d9 Numerous changes to support test_factory c556c41 Add .gitignore b7a9cac Add initial makefiles from test_factory git-subtree-dir: pgxntool git-subtree-split: e80485ea77ec6dbdfa1dc9217d1c8b7ed2a94f43
test/install/load.sql'sRAISE EXCEPTIONs were decorative: pg_regress resolves its expected and result paths to the same file, so nothing ever failed on error, and turning onON_ERROR_STOPis what makes it actually fail as its header claims.That couldn't be turned on while the file used
\if, which is psql 10 while CI covers back to 9.4 — there psql reports it as an invalid command and runs every branch, soTEST_LOAD_SOURCE=updatehad never actually exercised the update path on 9.4/9.5/9.6. Mode selection now happens server-side instead, and the shared schema-creation/install logic moves intotest/helpers/extension_installer.sql.