Skip to content

Add update+upgrade (U&U) test infrastructure (stack 2/3) - #18

Open
jnasbyupgrade wants to merge 2 commits into
reconcile-object-functionsfrom
u-and-u-testing
Open

Add update+upgrade (U&U) test infrastructure (stack 2/3)#18
jnasbyupgrade wants to merge 2 commits into
reconcile-object-functionsfrom
u-and-u-testing

Conversation

@jnasbyupgrade

Copy link
Copy Markdown
Contributor

Summary

Second link of a 3-PR stack. Stacked on #17 (reconcile-object-functions) —
review this diff on top of that PR's changes, not against master.

Implements advanced-extension-testing.md checklist items 1-6 (the local
test/Makefile machinery for fresh/update/existing-mode testing). CI wiring
(a dedicated job that actually invokes TEST_LOAD_SOURCE=update /
=existing) is deliberately not part of this PR — that's link 3 of the
stack. CI here just runs the normal test job in fresh mode and stays green.

  • PGXNTOOL_ENABLE_TEST_INSTALL / PGXNTOOL_ENABLE_VERIFY_RESULTS set
    explicitly; TEST_LOAD_SOURCE (fresh/update/existing) + TEST_UPDATE_FROM/
    TEST_UPDATE_TO make vars, parse-time validated, propagated as placeholder
    GUCs via PGOPTIONS; make test-update wrapper.
  • test/install/load.sql: single, committed-once installer for the
    extension, covering all three load modes — drop-first reset (with a
    pg_temp.drop_role() helper for object_reference__usage /
    object_reference__dependency, the extension's own global roles) for
    fresh/update, and a presence+version assertion for existing.
  • sql/object_reference--0.1.0--stable.sql: hand-authored update script.
    There was previously no update path at all from 0.1.0 (the only real
    historical PGXN release) to current. Diffed the two files in full to find
    every delta (see the file's own header comment for the itemized list —
    reg* pseudotype column removal, the count_nulls-backed trigger, new
    object-info functions, temp-schema rejection, etc.). Every function/view
    that changed is recreated via the same private-helper-schema
    bootstrap/teardown convention the fresh install itself uses (not hand-typed
    DROP FUNCTION/CREATE FUNCTION/REVOKE/GRANT/COMMENT), so the
    update path can't silently diverge from a fresh install's template.
  • Makefile: DATA += sql/object_reference--0.1.0.sql — pgxntool's DATA
    wildcard only picks up the current version file and two-dash update
    scripts, never a one-dash historical full-install file, so without this
    CREATE EXTENSION object_reference VERSION '0.1.0' fails outright (same
    gap as Postgres-Extensions/pgxntool#48). Also a conditional
    count_nulls install step, gated on TEST_LOAD_SOURCE=update: 0.1.0's own
    install script still creates a trigger that calls count_nulls'
    not_null_count_trigger(), and object_reference.control's requires
    (cat_tools only now) no longer CASCADEs it in — current object_reference
    itself has no runtime dependency on count_nulls at all, so this is scoped
    to the update-mode floor only, not folded into the unconditional
    install: cat_tools.
  • test/finish.sql: the one permanent pgTAP assertion recommended instead of
    a TEST_SCHEMA dimension (object_reference is genuinely schema-pinned —
    schema= set, relocatable=false, zero @extschema@ usage) — modeled on
    pg_count_nulls'/extension_tools' own schema-qualification checks.
    Excludes object_reference/_object_reference from search_path (already
    true via test/load.sql → pgxntool's tap_setup.sql) and asserts via
    current_schemas(), checked at the end of every test/sql/*.sql file (so
    a test that mutates search_path mid-file and never restores it gets
    caught). Every existing test file's plan() count bumped by 1 accordingly.
  • Moved the pre-existing raw-source-load sanity check
    (test/sql/zzz_build.sqltest/build/zzz_build.sql, pgxntool's own
    test-build feature) — it manually does CREATE SCHEMA object_reference;
    and loads sql/object_reference.sql directly, which now collides with the
    real extension test/install/load.sql already installed in the shared
    main-suite database. test-build runs its own separate, freshly
    drop+recreated database each time, matching what this check always
    assumed.
  • Two non-obvious correctness fixes found while writing the update script,
    both empirically hit and fixed (not theoretical):
    • The _object_v/_object_v__for_update views must be dropped before
      the _object_oid table's reg* columns (they SELECT those columns
      directly) — got the order backwards on the first pass; Postgres's own
      "cannot drop column ... other objects depend on it" caught it.
    • object_reference's own event triggers (already installed and active
      from the 0.1.0 base) fire on every sql_drop/ddl_command_end in the
      session — including the update script's own DROP VIEW/ALTER TABLE
      statements. zzz__object_reference_drop's body queries
      _object_reference._object_v, so it errored the instant the script
      dropped that view. Fixed by disabling all three event triggers for the
      structural portion of the update script and re-enabling them once
      everything is back in its final shape.

Convergence/divergence vs. cat_tools (the reference implementation)

Per advanced-extension-testing.md's stated goal (find out what's generic
enough to belong in pgxntool itself), cloned Postgres-Extensions/cat_tools
fresh (ca12802) and read test/install/load.sql, test/roles.sql,
test/deps.sql, test/finish.sql, Makefile's TEST_LOAD_SOURCE block,
and bin/test_existing before writing anything.

Took verbatim (structure/pattern, names adapted):

  • The whole TEST_LOAD_SOURCE/TEST_UPDATE_FROM/TEST_UPDATE_TO GUC-via-
    PGOPTIONS propagation mechanism, including the parse-time
    fresh|update|existing validation and the test-update recursive-$(MAKE)
    wrapper.
  • test/install/load.sql's three-mode \if structure, the
    pg_temp.drop_role() helper (DROP OWNED BY then DROP ROLE IF EXISTS),
    and the existing-mode presence/version DO block — all copied nearly
    line-for-line, only object_reference's own names substituted in.
  • test/install/.gitignore (load.out / install.out.diff) plus the
    EXTRA_CLEAN wiring in the Makefile — cat_tools deliberately never commits
    meaningful content there because pgxntool's ../install/<name> schedule
    trick resolves the expected-output path and the results path to the exact
    same file, making it self-comparing. Only realized this after initially
    trying to treat it like a normal expected-output file and getting confused
    by spurious "ok" results; cat_tools's own Makefile comment ("the
    self-comparing result .out and its diff") is what clarified it.
  • test/finish.sql's exact current_schemas(false) assertion shape (adapted
    from pg_count_nulls/extension_tools, which cat_tools's own equivalent
    also draws from) per test-fixes.md's item 9 recommendation.

Adapted:

  • cat_tools has no TEST_UPDATE_FROM-floor dependency gap (its own oldest
    supported version installs cleanly with just its own requires).
    object_reference's 0.1.0 floor needs count_nulls, which the current
    control file no longer declares — required inventing the conditional
    install: count_nulls Makefile step and the explicit
    CREATE EXTENSION IF NOT EXISTS count_nulls; in update mode's branch.
    Nothing in cat_tools's load.sql needed an equivalent, since it has no
    extra-dependency floor gap of its own.
  • cat_tools's update path only has to ALTER TYPE ... ADD VALUE (additive);
    object_reference's has real DROP COLUMN/DROP VIEW/signature changes,
    so the update script itself is structured very differently (the
    private-helper-schema bootstrap/teardown convention, the explicit
    constraint/index/trigger drop ordering, the event-trigger disable/enable
    bracket) — none of that has a cat_tools analog to copy from.

Skipped (explicitly out of scope per the containing doc/task):

  • bin/test_existing-equivalent script and any CI wiring
    (extension-update-test, pg-upgrade-test jobs, dependency-guard planting
    in CI) — next link in the stack.
  • TEST_SCHEMA dimension — object_reference is schema-pinned; test-fixes.md
    item 9 explicitly says not to build this, use the one permanent assertion
    instead (done above).
  • Multiple update-origin testing (§6's "two kinds of content" distinction) —
    object_reference has exactly one real historical version (0.1.0), so there
    is only one update-from floor to test, no shortest-path duplicity concern.
  • The bridge-update pattern (§6c) — not needed; 0.1.0 installs and updates
    cleanly on every currently-supported PostgreSQL major (verified by hand on
    12 and 17), so there's no known unsafe old version needing a bridge.
  • structural_diff.sql/bin/structural_diff as committed tooling — I did
    the structural comparison (function bodies via pg_get_functiondef,
    table/view columns, comments, ACLs) by hand with one-off SQL for this PR's
    own verification and confirmed a byte-for-byte match, but didn't commit it
    as reusable tooling; flagging as a candidate for a future PR/pgxntool
    feature rather than building it now.

Invented (not in cat_tools):

  • The count_nulls version-floor handling above.
  • \set ON_ERROR_STOP 1 at the top of test/install/load.sql. cat_tools's
    own load.sql doesn't set it either (same latent gap) — without it, a
    genuine script error doesn't abort; it just prints and keeps going
    statement-by-statement, which is exactly what made the two bugs above (view
    drop ordering, event-trigger self-interference) hard to see clearly at
    first — errors were present in the output but buried, and the ../install/
    self-comparing .out file made everything downstream of them look
    spuriously "ok". Worth a follow-up issue against cat_tools itself.
  • Disabling the extension's own event triggers around the update script's
    structural changes. A generic pattern for any extension whose update path
    needs to modify objects that its own already-active event triggers
    reference — cat_tools has no event triggers of its own so this never came
    up there, but it's a first-instance discovery of a bug class advanced-
    extension-testing.md's doc doesn't currently mention (an update script's
    own DDL becoming an unwitting trigger of the previous version's event
    handlers). Worth surfacing back into the doc/pgxntool guidance.

Dependency-guard anchor

Picked _object_reference.object's row type (the composite type Postgres
gives every table): SELECT NULL::_object_reference.object AS guarded_member in a guard view. Reasons:

  • It's object_reference-owned (unlike cat_tools.object_type, which this
    extension only consumes — using an external type wouldn't demonstrate
    object_reference's own extension membership blocking the drop).
  • The table's existence, PK, and row type are untouched by
    0.1.0--stable.sql (only _object_oid changes structurally), so the
    guard is stable across the one real update path that exists today.

Manually verified end-to-end (not yet wired into CI — that's the next
stack link):

  • Fresh install: guard view created, non-CASCADE DROP EXTENSION fails
    with _object_reference.object's type listed as the blocker, CASCADE
    correctly drops both the extension and the guard view together.
  • Update path (0.1.0 → stable): guard planted on the 0.1.0 install, survives
    ALTER EXTENSION UPDATE unchanged, still blocks a non-CASCADE drop
    afterward.

Verification performed by hand

  • make lint — clean, both before and after every change in this PR.
  • make test (fresh) — all 7 pgTAP files + zzz_build (now under
    test/build/) pass, on PostgreSQL 12 and 17.
  • make test TEST_LOAD_SOURCE=update / make test-update — same suite,
    same expected output, passes identically to fresh, on PostgreSQL 12 and 17.
  • make test TEST_LOAD_SOURCE=existing CONTRIB_TESTDB=<db> EXTRA_REGRESS_OPTS=--use-existing PGXNTOOL_ENABLE_TEST_BUILD=no against a
    correctly-installed database (passes) and a deliberately-broken one (stuck
    at 0.1.0) — fails loudly at the install/load step with a clear "installed
    at version 0.1.0 but the current default_version is stable" error, not
    silently.
  • Structural fresh-vs-update comparison (function bodies via
    pg_get_functiondef, table/view columns, comments, ACLs) — byte-for-byte
    identical.
  • Dependency guard: proven to block a non-CASCADE drop and let CASCADE
    through, both on a fresh install and after the update path, as described
    above.

Not fully verified

  • Binary pg_upgrade across PostgreSQL majors — out of scope for this PR
    (no pg-upgrade-test/bin/test_existing CI infrastructure exists yet;
    that's part of the next stack link).
  • CI itself does not exercise any of TEST_LOAD_SOURCE=update/existing
    by design, per the task this PR implements; confirmed CI still runs the
    plain test job in fresh mode and stays green.

Supersedes

Supersedes jnasbyupgrade/object_reference#3 ("u-and-u-foundation"), which
was built against the wrong version model (TEST_UPDATE_TO=0.2.0, no longer
current — everything is stable now) and included a TEST_SCHEMA dimension
that test-fixes.md's item 9 identifies as a mistake for this schema-pinned
extension. Please close #3 once this merges (leaving that to the repo
owner rather than closing it myself).

🤖 Generated with Claude Code

…ript,

test/install/load.sql three-mode loader, dependency-guard anchor, and a
permanent schema-qualification pgTAP assertion

Implements advanced-extension-testing.md checklist items 1-6 on top of
PR #17 (reconcile-object-functions):

- PGXNTOOL_ENABLE_TEST_INSTALL / PGXNTOOL_ENABLE_VERIFY_RESULTS set
  explicitly; TEST_LOAD_SOURCE (fresh/update/existing) + TEST_UPDATE_FROM/TO
  make vars, parse-time validated, propagated as placeholder GUCs via
  PGOPTIONS; `make test-update` wrapper.
- test/install/load.sql: single committed-once installer for the extension,
  covering all three load modes, including a drop-first reset (with
  pg_temp.drop_role() for the extension's own global roles) and an
  existing-mode presence/version assertion.
- sql/object_reference--0.1.0--stable.sql: hand-authored update script (there
  was previously no update path at all from the only real historical release
  to current). Recreates every function/view that changed via the same
  private-helper-schema bootstrap/teardown convention the fresh install uses,
  so the update path is verified byte-for-byte structurally identical to a
  fresh install (function bodies, comments, ACLs, table/view columns).
- Makefile: DATA += sql/object_reference--0.1.0.sql (pgxntool#48 workaround,
  needed for CREATE EXTENSION ... VERSION '0.1.0' to work at all); a
  conditional count_nulls install step for the update-mode floor only (0.1.0's
  own install script still needs it, even though current object_reference no
  longer requires it).
- test/finish.sql: one permanent pgTAP assertion (modeled on
  pg_count_nulls'/extension_tools' own schema-qualification checks) proving
  object_reference/_object_reference are never resolved via search_path.
- Moved the pre-existing raw-source-load sanity check (test/sql/zzz_build.sql)
  to test/build/, pgxntool's own test-build feature: it needs a schema-free
  database to create `object_reference` manually in, which the committed-once
  installer above no longer provides in the shared main-suite database.

Dependency-guard anchor for a future existing-mode CI job: a view typed on
_object_reference.object's row type (object_reference-owned, never dropped or
redefined by the update script) blocks a non-CASCADE DROP EXTENSION; manually
proven to block the drop (and to keep blocking it after the update path) as
part of this PR's own verification, not committed as CI machinery yet.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

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: 16c5fe85-0479-46c0-995e-ac3d37ecdba6

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

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

test/build/zzz_build.sql (moved there in the previous commit) enables
pgxntool's test-build feature, whose run-test-build.sh syncs
test/build/*.sql into test/build/sql/ via rsync -- not present in the
pgxn/pgxn-tools image, causing every PG-matrix leg to fail with
"rsync: command not found".

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant