Skip to content

Cross extension-update-test/pg-upgrade-test with TEST_SCHEMA - #32

Closed
jnasbyupgrade wants to merge 6 commits into
masterfrom
phase5-cross-schema
Closed

Cross extension-update-test/pg-upgrade-test with TEST_SCHEMA#32
jnasbyupgrade wants to merge 6 commits into
masterfrom
phase5-cross-schema

Conversation

@jnasbyupgrade

@jnasbyupgrade jnasbyupgrade commented Aug 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

⚠️ SUPERSEDED — this PR is meant to be replaced, not merged as-is. Its premise (crossing pg-upgrade-test/extension-update-test with a fixed ["", Quoted] TEST_SCHEMA matrix) is being replaced by an always-randomize-the-install-schema design instead. Left open for reference only.

Stacked on #31 (phase 4: real pg_upgrade support). This is the novel enhancement the whole redesign was building toward.

Why this is novel

Checked directly: nobody in the org currently tests update/upgrade crossed with schema scenarios. cat_tools' own extension-update-test/pg-upgrade-test matrices are PG-version-only (no schema axis at all - checked its actual ci.yml). extension_tools has no U&U testing whatsoever. So this is genuinely new coverage, not something to copy from a reference implementation.

What changed

  • extension-update-test: added schema: ["", Quoted] to the matrix + a TEST_SCHEMA env var - the job's own make verify-results TEST_LOAD_SOURCE=update picks it up automatically (Make auto-imports matching-named environment variables).
  • pg-upgrade-test: added the same schema axis. old_pg/new_pg were already plain matrix dimensions (not an include: list), so adding a third axis cross-products cleanly into 4 legs (2 old_pg values × 2 schema values). Threaded matrix.schema through to bin/test_existing's prepare-old/run-suite calls, previously hardcoded to "".

Why this is "free": phase 2's schema-invariant assertion descriptions mean crossing either job with TEST_SCHEMA needs zero new expected-output files - every leg of every job (fresh, update, real pg_upgrade) × (no schema, Quoted schema) passes against the exact same test/expected/extension_tests.out (+ the one genuine alternate from phase 2).

Verification

Locally against PG17: prepare-old → update → run-suite passes end to end with TEST_SCHEMA=Quoted (previously only verified with an untargeted schema in phase 4).

@coderabbitai

coderabbitai Bot commented Aug 1, 2026 •
edited
Loading

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: e9c92029-e901-43c9-afa5-c1b93a656843

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.

@jnasbyupgrade jnasbyupgrade mentioned this pull request Aug 1, 2026
Merged
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

Code review

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

@jnasbyupgrade jnasbyupgrade changed the title Phase 5: cross extension-update-test/pg-upgrade-test with TEST_SCHEMA Cross extension-update-test/pg-upgrade-test with TEST_SCHEMA Aug 4, 2026
@jnasbyupgrade
jnasbyupgrade marked this pull request as draft August 4, 2026 21:11
@jnasbyupgrade
jnasbyupgrade force-pushed the phase4-pg-upgrade branch 2 times, most recently from 6c1d36f to e64f80f Compare August 5, 2026 18:14
@jnasbyupgrade jnasbyupgrade mentioned this pull request Aug 5, 2026
Open
@jnasbyupgrade
jnasbyupgrade force-pushed the phase5-cross-schema branch 2 times, most recently from 1d942ca to 1694173 Compare August 5, 2026 21:45
@jnasbyupgrade
jnasbyupgrade force-pushed the phase5-cross-schema branch 2 times, most recently from 293db9f to ee0f795 Compare August 6, 2026 19:16
@jnasbyupgrade
jnasbyupgrade force-pushed the phase4-pg-upgrade branch 2 times, most recently from 0f9ede2 to f38f82d Compare August 6, 2026 20:57
@jnasbyupgrade
jnasbyupgrade force-pushed the phase5-cross-schema branch 2 times, most recently from f3da5c0 to 6e3cca1 Compare August 6, 2026 22:51
jnasbyupgrade and others added 6 commits August 6, 2026 17:56
@jnasbyupgrade @claude
Phase 3.5: CI hygiene - docs-only gate, all-checks-passed, single-sou…
4a5a7c9 
…rce PG list

Independent of the U&U testing work itself, but best done now that
multiple CI jobs exist and before the next phase adds the most expensive
one (a real pg_upgrade job):

- `changes` job: computes the actual per-push diff and skips test/
  extension-update-test/pg-tle-test entirely on doc-only pushes, always
  triggering itself (no workflow-level paths-ignore, which would leave
  all-checks-passed stuck Pending on doc-only pushes in branch protection).
- Derives the supported-PostgreSQL-major list from ONE set of constants
  (NEWEST/FLOOR) in that same job, consumed by both the `test` and
  `extension-update-test` matrices via fromJSON - they can't silently drift
  onto different lists, and a new major is a one-line change.
- `all-checks-passed`: single stable required-status-check name, with a
  self-check that its own needs list can't silently omit a newly-added job.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@jnasbyupgrade
CI: gate draft PRs down to lint + newest-PG test only
f7c8395 
The org-wide Actions runner queue backs up easily; a draft PR being
actively iterated on doesn't need the full PG matrix or the heavy
pg-tle-test job re-run on every push. Add a newest_pg scalar output
(single source alongside supported_pg) and reduce the test job's
matrix to just that value on a draft PR, while skipping pg-tle-test
(and any later heavy job following the same needs:[changes]/if:
docs_only pattern) outright. Non-draft PRs and push events (e.g.
post-merge on master) are unaffected.
@jnasbyupgrade @claude
Phase 4: real pg_upgrade support via a reduced bin/test_existing
b11df4d 
Adds the pg-upgrade-test CI job: install 0.9.6 on an old PostgreSQL major,
plant + prove a dependency guard, binary pg_upgrade to a newer major,
ALTER EXTENSION UPDATE the migrated objects, then run the suite against
the real upgraded database in existing mode.

bin/test_existing is much smaller than the equivalent script would have
been pre-test/install: only prepare-old and run-suite are genuinely
external-to-pg_regress concerns (a real pg_upgrade binary run isn't
something pg_regress can invoke itself), plus a small `update` subcommand
for the post-upgrade ALTER EXTENSION UPDATE step. There's no
update-scenario subcommand at all - that entire scenario is just `make
test-update` now (test/install/load.sql's own 'update' mode, added in
phase 3), since an in-place update has no external step to drive.

run_suite() gates on plain `make test`, not the old belt-and-suspenders
`make test && make verify-results` - pgxntool 2.3.0 (this repo's phase 0)
already made `make test` itself exit non-zero on regression failures.

Not yet crossed with TEST_SCHEMA - that's the next phase, once both this
job and extension-update-test can cross it together.

Verified locally against PG17 (prepare-old -> update -> run-suite, without
a real pg_upgrade - this container's clusters are persistent shared
infra, so the actual binary pg_upgrade leg is left for CI's ephemeral
containers, same reasoning as the pg-tle-test work).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@jnasbyupgrade
pg-upgrade-test: run ALTER EXTENSION UPDATE before binary pg_upgrade,…
a3b7c81 
… not after

Reorders prepare-old -> update -> pg_upgrade -> run-suite (was prepare-old
-> pg_upgrade -> update -> run-suite). The old order proved pg_upgrade
could migrate 0.9.6's frozen objects, then updated afterward - not
actionable, since that version already shipped. This job's whole point is
proving pg_upgrade correctly migrates the objects count_nulls' CURRENT code
creates, which requires updating BEFORE the binary upgrade runs. make
install (into the old cluster) already happens earlier in the job, so the
current version's update scripts are on disk in time for the moved step.

Updates the job's step names/comments and bin/test_existing's own
file-header sequence description to match the new order.
@jnasbyupgrade
CI: skip pg-upgrade-test on draft PRs too
3c19783 
Propagates the draft-PR gating from phase3.5-ci-hygiene to the
pg-upgrade-test job introduced by this branch: same needs:[changes]/
if: docs_only pattern as pg-tle-test, so it gets the same
&& github.event.pull_request.draft != true guard.
@jnasbyupgrade @claude
Cross extension-update-test/pg-upgrade-test with TEST_SCHEMA via make…
508e17a 
…/shell loops, not a matrix

Redesign of the original approach (which crossed TEST_SCHEMA into both
jobs' CI matrices) per the same reasoning as the `test` job's collapse: a
schema name is just an input the same assertions run against, not a real
environment difference.

- extension-update-test: added `make test-update-schema-all` (Makefile),
  the same TEST_SCHEMA loop as test-schema-all but with
  TEST_LOAD_SOURCE=update. Job step calls it instead of crossing schema
  into the matrix.
- pg-upgrade-test: no make-level loop is possible here (bin/test_existing's
  steps are shell, not `make test`), so instead prepares TWO databases -
  count_nulls_upgrade_none and count_nulls_upgrade_quoted, one per TEST_SCHEMA
  value - before the SINGLE pg_upgrade call, which migrates the whole
  cluster (every database in it) in one pass. This is strictly better than
  a doubled matrix would have been: it also halves the number of actual
  pg_upgrade binary invocations (the single most expensive operation in
  this job), not just container/checkout overhead.

Verified locally against PG17: prepare-old -> update -> run-suite passes
for both databases in the same cluster/session (no real pg_upgrade run,
same reasoning as prior phases - this container's clusters are persistent
shared infra); make test-update-schema-all passes both TEST_SCHEMA legs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@jnasbyupgrade
jnasbyupgrade force-pushed the phase4-pg-upgrade branch 2 times, most recently from d714ef5 to 83b6214 Compare August 7, 2026 20:49
Base automatically changed from phase4-pg-upgrade to master August 7, 2026 22:30
@jnasbyupgrade jnasbyupgrade mentioned this pull request Aug 7, 2026
Merged
jnasbyupgrade added a commit that referenced this pull request Aug 8, 2026
@jnasbyupgrade @claude
Always install count_nulls into a fresh, randomly named schema for te…
040b9db 
…sts (#55)

Replace the two-value TEST_SCHEMA axis (fixed "" and "Quoted" legs, run
via `make test-schema-all`'s in-Makefile loop) with a single install per
run into a freshly, randomly generated schema whose constant prefix (a
literal trailing space) always requires SQL identifier quoting. This
exercises `%I`-qualification on every run instead of only on a dedicated
quoting leg, and removes the Makefile/GUC-propagation infrastructure
that existed solely to support the two-value axis.

Cleanup-before-create matches on the constant prefix (`count_nulls test
schema %`) to find and drop any schema left behind by a run that crashed
before its own teardown, so stale schemas don't accumulate run over run.
`test/helpers/find_test_schema.sql` lets separate sessions
(`bin/test_existing`'s per-step `psql -f ...` invocations, each a fresh
connection) rediscover the randomly generated name without having
created it themselves.

`bin/test_existing`'s `prepare-old`/`plant_guard`/`run-suite` no longer
take a schema argument, since every install always targets its own
randomly generated schema now; `create_extension_in_schema()` moved to a
proper `-f` script (`bin/test_existing.sql/create_extension.sql`) since
it needs multiple statements including a `\gset`.

This supersedes PR #32's cross-schema approach, which is left open only
for reference.

---------

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@jnasbyupgrade