pg-tle-test: extend to the 0.9.6 -> current update path via DESTDIR sandboxing

[jnasbyupgrade]

pgxntool 2.3.0's installcheck-ordering fix (pgxntool#83) made installcheck
(and so make test) unconditionally depend on install, which writes a real
.control file to disk - defeating the whole point of a pg_tle-only-deployment
proof, and with no upstream fix yet (pgxntool#90). bin/test_existing's
run_suite gains a TEST_EXISTING_DEPLOY=pgtle mode that instead redirects
make test's install step through a scratch DESTDIR (harmless here since a
pg_tle-deployed database never needs those files), pre-seeding a stub
pgtap.control at the DESTDIR-prefixed path pgxntool's pgtap prerequisite
checks for - otherwise its DESTDIR-blind recipe (pgxn install pgtap --sudo)
would trigger a real, unwanted reinstall against the scratch dir.

Also fixes test/install/load.sql's existing-mode assertion, which cross-checks
the installed version against pg_available_extensions.default_version -
that view never sees pg_tle registrations (it only reads .control files off
disk), so it would return NULL under a pg_tle deployment even though CREATE
EXTENSION resolves correctly through pg_tle's own catalog. A new
count_nulls.test_existing_deploy GUC (driven by the same TEST_EXISTING_DEPLOY
var, Makefile-exported like TEST_SCHEMA/TEST_LOAD_SOURCE) selects
pgtle.available_extensions() instead when deploy=pgtle.

The pg-tle-test CI job now installs count_nulls at 0.9.6 into a second
scratch database (inheriting the job's template1 pg_tle registration),
plants + proves the dependency guard, ALTER EXTENSION UPDATEs to current,
then runs the real pgTAP suite against it via the new mode - with
bin/assert_fs_clean verify checked after every one of those steps, not just
at the end, since that's what actually proves the DESTDIR sandboxing worked.

Verified locally (no real pg_tle in this container): confirmed via
mtime comparison that the real count_nulls.control and pgtap.control are
untouched by a full prepare-old/update/run-suite(pgtle) cycle, that the
scratch DESTDIR is cleaned up on both success and a forced regression
failure (EXIT trap surviving set -e), and exercised the
count_nulls.test_existing_deploy branch in load.sql against a stubbed
pgtle.available_extensions() function. The genuine pg_tle-registered
version needs a real CI run to confirm end to end.

[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: ec12d2a8-63f1-45df-9faa-f959856e2cf4

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.}

[github-actions]

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

[github-actions]

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

[github-actions]

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

---

Tasks

• Gather context (PR body, changed files, CLAUDE.md)
• Review diff across all 4 changed files
• Verify the DESTDIR/GUC/CI plumbing end-to-end
• Post review findings

Review

Read through the full diff (.github/workflows/ci.yml, Makefile, bin/test_existing, test/install/load.sql) and traced the mechanism end-to-end rather than just reading it in isolation:

• seed_pgtap_stub (bin/test_existing:205-215): confirmed datadir is a real PGXS variable (from Makefile.global, included via pgxntool/base.mk:657), and print-% (pgxntool/base.mk:609) is a real generic rule, so make -s print-datadir resolves correctly and the sed extraction matches pgxntool's own pgtap: $(DESTDIR)$(datadir)/extension/pgtap.control prerequisite (pgxntool/base.mk:683) exactly. The stub path lines up.
• TEST_EXISTING_DEPLOY threading: Makefile:73-77 exports it via the same PGOPTIONS -c pattern already used for TEST_LOAD_SOURCE/TEST_SCHEMA, bin/test_existing:278 passes it through explicitly on the make test command line (not left to environment inheritance), and test/install/load.sql:70-88 reads it via current_setting. All three points agree on the same two valid values (filesystem/pgtle), each independently validated (Makefile, script, SQL) — consistent with this repo's existing pattern for TEST_LOAD_SOURCE.
• Trap-based cleanup (bin/test_existing:263): correctly uses EXIT (not RETURN), which is necessary since a make test regression failure under set -e wouldn't otherwise unwind through a RETURN trap. Only registered in the pgtle branch, and run_suite is only invoked once per script invocation, so no risk of clobbering an existing trap.
• CI job ordering (ci.yml): assert_fs_clean verify runs after every step that could plausibly write to disk, not just at the end — matches the PR's stated intent, and the pre-existing count_nulls_smoke database's registration inheritance (via template1) applies the same way to the new count_nulls_pgtle_update database.
• pgtle.available_extensions() cross-check in load.sql: correct function name/shape for pg_tle's non-integrated analog to pg_available_extensions.

No bugs found. The two automated claude-code-review passes on this PR also reported no issues, and my independent pass agrees. The PR is unusually well-documented — every non-obvious decision (env var reuse, trap type, DESTDIR scoping, GUC cross-check source) has an inline comment explaining why, which made verification straightforward.

No changes recommended.

[github-actions]

Claude finished @jnasbyupgrade's task in 2m 6s —— View job

---

Tasks

• Gather context (PR body, changed files, CLAUDE.md)
• Review diff across all 4 changed files
• Verify DESTDIR/GUC/CI plumbing end-to-end
• Post review findings

Review

Reviewed the full diff (.github/workflows/ci.yml, Makefile, bin/test_existing, test/install/load.sql) and traced the mechanism end-to-end:

• TEST_EXISTING_DEPLOY GUC plumbing: Makefile:73-77 unconditionally exports count_nulls.test_existing_deploy via PGOPTIONS (defaulting to filesystem), so current_setting(...) in test/install/load.sql:70 (read without missing_ok) is always satisfied regardless of TEST_LOAD_SOURCE. bin/test_existing:278 passes the var through explicitly on the make test command line rather than relying on environment inheritance — consistent with the existing TEST_LOAD_SOURCE/TEST_SCHEMA pattern.
• seed_pgtap_stub (bin/test_existing:205-215): datadir resolves via the real PGXS print-% generic rule (pgxntool/base.mk:609), and the stub path matches pgxntool's own DESTDIR-aware pgtap prerequisite check (pgxntool/base.mk:683) exactly — writes into the scratch $destdir, never the real /usr/share/postgresql/.../extension dir that bin/assert_fs_clean checks, so the sandboxing and the verification don't collide.
• Trap-based cleanup (bin/test_existing:263): EXIT (not RETURN) is correct — a make test regression failure under set -e needs EXIT to unwind reliably. destdir_opt=() expanding via "${destdir_opt[@]}" under set -u is safe since it's always declared (never unset), even on older bash.
• test/install/load.sql (lines 67-94): correctly branches on count_nulls.test_existing_deploy to cross-check against pgtle.available_extensions() (schema-qualified, no search_path dependency) instead of pg_available_extensions — the latter never sees pg_tle registrations since it only reads .control files off disk. The ELSE branch guards against an invalid GUC value.
• CI job ordering (ci.yml): assert_fs_clean verify runs after every step that could write to disk (0.9.6 install, update, and the pgTAP suite run) — the last one is the load-bearing check that actually proves the DESTDIR sandbox worked, not just a final sanity check.

No bugs found. This is the second independent pass on this PR to reach that conclusion (two automated claude-code-review runs also reported no issues); I traced the mechanism rather than just reading it, and it holds together consistently across all four files.

[github-actions]

Claude finished @jnasbyupgrade's task in 2m 43s —— View job

---

Tasks

• Gather context (PR body, changed files, CLAUDE.md)
• Review diff across all 4 changed files
• Verify DESTDIR/GUC/CI plumbing end-to-end
• Post review findings

Review

Read the full diff (git diff 827473b..8453516 — .github/workflows/ci.yml, Makefile, bin/test_existing, test/install/load.sql) and traced the mechanism against the actual pgxntool/base.mk source rather than taking the PR description at face value:

• seed_pgtap_stub (bin/test_existing:205-215): verified directly in pgxntool/base.mk — print-% (line 609) emits $(info $* is $(flavor $*) variable set to "$($*)"), which is exactly the format the sed -n 's/.*set to "\(.*\)"$/\1/p' extraction expects, and the pgtap target's prerequisite (line 683) is literally $(DESTDIR)$(datadir)/extension/pgtap.control. The stub writes to "${destdir}${datadir}/extension/pgtap.control" (using bare datadir from make -s print-datadir, then prepending destdir itself) — matches the target's own path construction exactly.
• TEST_EXISTING_DEPLOY GUC plumbing: Makefile:73-77 exports count_nulls.test_existing_deploy unconditionally via PGOPTIONS (defaulting to filesystem), validated the same way as the pre-existing TEST_LOAD_SOURCE pattern immediately above it. bin/test_existing:278 passes it through explicitly on the make test command line rather than relying on environment inheritance, and test/install/load.sql:70-94 reads it with an explicit ELSE-branch RAISE EXCEPTION guard against an invalid value. All three validate independently and agree on the same two values.
• Trap-based cleanup (bin/test_existing:259-266): EXIT (not RETURN) is the right choice — a make test regression failure under set -e needs EXIT to reliably unwind. Only registered in the pgtle branch, and run_suite runs once per script invocation (each CI step is a separate process), so there's no risk of a second trap call clobbering the first.
• CI job ordering (ci.yml:581-618): bin/assert_fs_clean verify runs after every step that could plausibly write to disk (0.9.6 install, the update, and the pgTAP suite run itself) — not just once at the end. The last one is the load-bearing check: it's the only thing that actually proves the DESTDIR sandbox held during make test's otherwise-unavoidable install step.
• test/install/load.sql existing-mode branch (lines 67-94): correctly cross-checks against pgtle.available_extensions() instead of pg_available_extensions when deploy = 'pgtle' — the latter only ever reads .control files off disk and would return NULL for a pg_tle-only registration, turning a real bug into a false pass. create_test_schema.sql:78's CREATE EXTENSION ... VERSION '0.9.6' also confirmed to route through pg_tle's CREATE EXTENSION hook rather than needing a filesystem version file, so prepare-old's use of it under pg_tle is sound.

No bugs found. This is now the third independent review pass on this PR (two automated claude-code-review runs plus this one) to reach that conclusion, and this pass additionally verified the underlying pgxntool/base.mk mechanics directly rather than relying on the PR body's description of them — they check out.

No changes recommended.