Skip to content

Commit 508e17a

Browse files
jnasbyupgradeclaude
andcommitted
Cross extension-update-test/pg-upgrade-test with TEST_SCHEMA via make/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>
1 parent 3c19783 commit 508e17a

2 files changed

Lines changed: 60 additions & 25 deletions

File tree

.github/workflows/ci.yml

Lines changed: 50 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,19 @@
3434
# instead of a filesystem .control file.
3535
#
3636
# Every TEST_SCHEMA value (empty - no schema targeting at all - and
37-
# 'Quoted', a name requiring SQL identifier quoting) is exercised too, via
38-
# `make test-schema-all`'s in-Makefile loop rather than a CI matrix
39-
# dimension - a schema name is just an input the same assertions run
40-
# against, not a real environment difference, so crossing it into the
41-
# matrix would only multiply job count for no added confidence (see the
42-
# Makefile's TEST_SCHEMA_VALUES comment). Every leg passes against the SAME
37+
# 'Quoted', a name requiring SQL identifier quoting) is exercised in every
38+
# job above too, but never as a CI matrix dimension - a schema name is just
39+
# an input the same assertions run against, not a real environment
40+
# difference, so crossing it into the matrix would only multiply job count
41+
# for no added confidence (see the Makefile's TEST_SCHEMA_VALUES comment).
42+
# `test` loops it (both its fresh and update legs) via `make
43+
# test-schema-all` / `make test-update-schema-all`; `pg-upgrade-test`
44+
# (shell, not `make test`, for the parts that matter here) prepares two
45+
# databases - one per schema -
46+
# ahead of a single pg_upgrade call that migrates both at once, which is
47+
# strictly better than a doubled matrix would have been: it also halves the
48+
# number of actual pg_upgrade binary invocations, not just container/
49+
# checkout overhead. Every leg passes against the SAME
4350
# test/expected/extension_tests.out (see test/README.md for how the suite
4451
# keeps its output schema-invariant).
4552
#
@@ -249,8 +256,8 @@ jobs:
249256
run: make test-schema-all
250257
- name: Install count_nulls
251258
run: make install
252-
- name: Update 0.9.6 -> current and run the suite
253-
run: make verify-results TEST_LOAD_SOURCE=update
259+
- name: Update 0.9.6 -> current and run the suite, across every TEST_SCHEMA value
260+
run: make test-update-schema-all
254261

255262
# Proves count_nulls survives a BINARY pg_upgrade (in-place catalog
256263
# migration to a newer PostgreSQL major). Installs 0.9.6 on an old
@@ -275,8 +282,16 @@ jobs:
275282
# json/jsonb, nothing version-sensitive to break at a specific boundary.
276283
# Revisit if count_nulls ever grows something catalog-touching.
277284
#
278-
# Not yet crossed with TEST_SCHEMA (a later phase adds that, once it can
279-
# do so for both this job and the test job's update leg together).
285+
# Every TEST_SCHEMA value is exercised here too, but NOT via a matrix
286+
# dimension (would double this job's already-expensive count) and not
287+
# via a make-level loop either (bin/test_existing's steps below are
288+
# shell, not `make test`) - instead, TWO databases (one per schema) are
289+
# prepared before the SINGLE pg_upgrade call, which migrates the WHOLE
290+
# cluster (every database in it) in one pass. This is strictly better
291+
# than a doubled matrix would have been, not just cheaper: it also
292+
# halves the number of actual pg_upgrade binary invocations (the single
293+
# most expensive operation in this job) instead of just avoiding
294+
# redundant container/checkout overhead.
280295
pg-upgrade-test:
281296
needs: [changes]
282297
# Skipped outright (not just matrix-reduced like `test`) on a draft PR:
@@ -311,23 +326,31 @@ jobs:
311326
uses: actions/checkout@v4
312327
- name: Install count_nulls into old cluster
313328
run: make install
314-
- name: Prepare the old cluster (install + dependency guard)
329+
- name: Prepare the old cluster (install + dependency guard), across every TEST_SCHEMA value
315330
# prepare-old installs count_nulls at 0.9.6, then plants + proves
316331
# the dependency guard, so a later accidental CASCADE drop anywhere
317332
# in this job cannot silently make the eventual existing-mode run
318-
# test a fresh install instead.
319-
run: bin/test_existing prepare-old count_nulls_upgrade "" 0.9.6
320-
- name: Update the extension to the current version (still on the old cluster)
333+
# test a fresh install instead. Two separate databases (distinct
334+
# names, one per TEST_SCHEMA value) so both exist in the SAME
335+
# cluster ahead of the single pg_upgrade call below - that one
336+
# binary upgrade migrates both at once.
337+
run: |
338+
bin/test_existing prepare-old count_nulls_upgrade_none "" 0.9.6
339+
bin/test_existing prepare-old count_nulls_upgrade_quoted Quoted 0.9.6
340+
- name: Update the extension to the current version (still on the old cluster), across every TEST_SCHEMA value
321341
# Exercises ALTER EXTENSION UPDATE on the OLD cluster, BEFORE the
322-
# binary pg_upgrade below, running the 0.9.6->stable update script -
323-
# deliberately in this order (not update-after-upgrade): this job
324-
# exists to prove pg_upgrade correctly migrates the objects
325-
# count_nulls' CURRENT code creates, so pg_upgrade must run against
326-
# already-current objects, not 0.9.6 ones. `make install` above
327-
# already installed the current version's update scripts/control
328-
# file into this (old) cluster's sharedir, so they're in place for
329-
# this ALTER EXTENSION UPDATE to use.
330-
run: bin/test_existing update count_nulls_upgrade
342+
# binary pg_upgrade below, running the 0.9.6->stable update script,
343+
# once per database prepared above - deliberately in this order
344+
# (not update-after-upgrade): this job exists to prove pg_upgrade
345+
# correctly migrates the objects count_nulls' CURRENT code creates,
346+
# so pg_upgrade must run against already-current objects, not 0.9.6
347+
# ones. `make install` above already installed the current
348+
# version's update scripts/control file into this (old) cluster's
349+
# sharedir, so they're in place for this ALTER EXTENSION UPDATE to
350+
# use.
351+
run: |
352+
bin/test_existing update count_nulls_upgrade_none
353+
bin/test_existing update count_nulls_upgrade_quoted
331354
- name: Install PostgreSQL ${{ matrix.new_pg }}
332355
run: apt-get install -y postgresql-${{ matrix.new_pg }} postgresql-server-dev-${{ matrix.new_pg }}
333356
- name: Install count_nulls into new cluster
@@ -354,14 +377,16 @@ jobs:
354377
/var/lib/postgresql/${{ matrix.new_pg }}/test/pg_upgrade_output.d \
355378
-name '*.log' 2>/dev/null | sort | xargs -r tail -n +1; exit 1; }
356379
pg_ctlcluster ${{ matrix.new_pg }} test start
357-
- name: Run the suite against the pg_upgraded database (existing mode)
380+
- name: Run the suite against the pg_upgraded database (existing mode), across every TEST_SCHEMA value
358381
# run-suite asserts the version, re-proves the dependency guard
359382
# still blocks a non-CASCADE drop (i.e. it survived both the update
360383
# and pg_upgrade), drops the guard, then runs the suite against the
361384
# REAL pg_upgraded database via --use-existing (so pg_regress does
362385
# not drop/recreate it) - a plain fresh `make test` would silently
363386
# test a fresh install instead of the migrated objects.
364-
run: bin/test_existing run-suite count_nulls_upgrade ""
387+
run: |
388+
bin/test_existing run-suite count_nulls_upgrade_none ""
389+
bin/test_existing run-suite count_nulls_upgrade_quoted Quoted
365390
366391
pg-tle-test:
367392
needs: [changes]

Makefile

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,3 +101,13 @@ export PGOPTIONS := $(PGOPTIONS) -c count_nulls.test_load_mode=$(TEST_LOAD_SOURC
101101
.PHONY: test-update
102102
test-update:
103103
$(MAKE) test TEST_LOAD_SOURCE=update
104+
105+
# Same TEST_SCHEMA loop as test-schema-all, but in update mode - used by the
106+
# test CI job's update leg instead of crossing TEST_SCHEMA into ITS matrix
107+
# too, same reasoning as test-schema-all above.
108+
.PHONY: test-update-schema-all
109+
test-update-schema-all:
110+
@for schema in $(TEST_SCHEMA_VALUES); do \
111+
echo "=== TEST_SCHEMA=$$schema (update) ==="; \
112+
$(MAKE) test TEST_LOAD_SOURCE=update TEST_SCHEMA="$$schema" || exit 1; \
113+
done

0 commit comments

Comments
 (0)