Add test/install foundation for update+upgrade testing (fresh/update/existing)

[jnasbyupgrade]

Summary

Adds test/install/load.sql, using pgxntool's PGXNTOOL_ENABLE_TEST_INSTALL feature, as the single place that installs the extension for the whole regression run, in one of three modes (TEST_LOAD_SOURCE, default fresh):

• fresh -- drops both extensions first (idempotent re-run against a non-fresh DB), lands pgtap in a dedicated tap schema, then CREATE EXTENSION test_factory_pgtap CASCADE.
• update -- CREATE EXTENSION test_factory VERSION :from then ALTER EXTENSION UPDATE, then installs test_factory_pgtap at current (it's only ever shipped one version, so no update path of its own to exercise yet). make test-update is a shorthand for make test TEST_LOAD_SOURCE=update.
• existing -- the extension is already installed (a real pg_upgrade target, or an out-of-band update) -- only asserts it's present at the current version, plants a dependency guard, and proves it. Run against a real pre-existing install with make test TEST_LOAD_SOURCE=existing CONTRIB_TESTDB=<db> EXTRA_REGRESS_OPTS=--use-existing PGXNTOOL_ENABLE_TEST_BUILD=no.

All three modes leave the same observable end state, so test/sql/base.sql and test/sql/pgtap.sql no longer install anything themselves or branch on mode -- they just assume both extensions are already present. test/helpers/create_extension.sql (the old install-or-skip wrapper they used) is deleted.

Dependency guard

Planted only in existing mode: a view in schema test_factory_drop_guard depending on tf.tap(text,text) blocks a non-CASCADE DROP EXTENSION test_factory_pgtap. test_factory itself doesn't need an artificial guard -- test_factory_pgtap's own control file (requires = 'pgtap, test_factory') already blocks a non-CASCADE DROP EXTENSION test_factory as long as test_factory_pgtap is installed; load.sql proves that natural protection too.

test/sql/pgtap.sql's dependency check

Now a pg_depend catalog inspection (works uniformly in every mode) instead of the old "try to CREATE EXTENSION test_factory_pgtap bare and see if it drags test_factory in" approach, which only worked when this file could assume test_factory_pgtap wasn't installed yet -- an assumption that no longer holds once load.sql installs both extensions in every mode.

Test plan

• make test passes on PG17 and PG12, in fresh, update, and existing modes
• Existing mode verified against a manually pre-populated database
• make lint passes

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2952126d-1656-44da-82ac-f6642b583c43

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[jnasbyupgrade]

@claude pls review

[claude]

Claude finished @jnasbyupgrade's task in 3m 1s —— View job

---

Review complete

• Gather context (PR description, diff, changed files)
• Read changed files in detail
• Review test/install/load.sql (new load-mode switch logic)
• Review test/helpers/create.sql and create_extension.sql deletion
• Review test/sql/pgtap.sql gating changes
• Review test/roles.sql, Makefile, CLAUDE.md docs
• Post final review

Summary

This is a well-executed, carefully reasoned foundation piece. The fresh/update/existing split in test/install/load.sql is clean, the GUC validation (parse-time $(error...) in the Makefile and read-time rejection with no missing_ok) is defensive in the right places, and the extensive inline comments explain non-obvious decisions (nested-comment gotcha, why the dependency guard is scoped to existing mode only, why test_factory gets its drop-guard "for free" via test_factory_pgtap's requires clause) rather than just restating the code. test/CLAUDE.md is fully consistent with the actual diff (no per-mode alternate expected files needed since base.sql/pgtap.sql behave identically across modes now).

Findings

1. PR description is stale relative to the final commits
The "Rebase note" and "What this adds" sections describe things that no longer match the code in this branch:

• They claim test/expected/{base,pgtap}_1.out alternate files were added for the existing-mode leg with "different-but-correct output (skipped sections, a skipped role-restore check)". Those files don't exist anywhere in this diff, and test/CLAUDE.md (also part of this PR) explicitly says the opposite: "no per-mode alternate expected files" because all three modes leave the same observable end state.
• The rebase note describes a crash fix in test/sql/pgtap.sql involving DROP TABLE pre_install_role/post_install_role guarded by to_regclass(...) IS NOT NULL. That code path doesn't exist in the final pgtap.sql either — it was superseded by the unconditional pg_depend-based check (commit ae91d20).

Both seem to be leftover narrative from an earlier iteration of the stack that got simplified in the later commits (ae91d20, 04e71aa) without the description being updated. Worth cleaning up before merge so the PR history/description doesn't mislead future readers (or git log archaeology) about what actually shipped.

2. existing-mode leaves permanent artifacts in the target database
In test/install/load.sql's existing branch, CREATE SCHEMA IF NOT EXISTS test_factory_drop_guard + CREATE OR REPLACE VIEW ...guard are committed (per the file's own header comment, load.sql runs "committed") and never dropped afterward. Since existing mode is specifically meant to run against "a real pg_upgrade target" per the docs, this leaves a schema/view behind in that database indefinitely that isn't part of either extension. Might be intentional (cheap, harmless test residue), but worth a deliberate call — either document it as an accepted side effect of running the suite against a persistent target, or drop the schema at the end of the existing branch once the guard proofs are done.

Neither of these is a functional bug — the load-mode mechanism itself, the GUC plumbing, the security-context handling, and the dependency-guard proof logic all look correct and match the verification the author describes doing locally on PG12/PG17.