Skip to content

Switch to pgxn-tools based testing #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jnasbyupgrade wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Switch to pgxn-tools based testing #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
fa15ccb
Switch to pgxn-tools based testing
jnasbyupgrade Aug 25, 2025
99083b8
Remove .travis.yml
jnasbyupgrade Aug 25, 2025
aa53b91
Remove errant vim swapfile
jnasbyupgrade Aug 25, 2025
68e8afc
Update test workflow
jnasbyupgrade Aug 25, 2025
9859566
Add missing PGUSER argument
jnasbyupgrade Aug 25, 2025
9a1e50b
Fix dump test
jnasbyupgrade Aug 25, 2025
594d248
Squashed 'pgxntool/' changes from 890053c..e9c24de
jnasbyupgrade Aug 25, 2025
a217824
Pull pgxntool from master
jnasbyupgrade Aug 25, 2025
8dc5cfc
dump test is actually included in make test
jnasbyupgrade Aug 25, 2025
728f815
Switch to github CI testing
jnasbyupgrade Jul 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: CI
on: [push, pull_request]
jobs:
test:
strategy:
matrix:
pg: [17, 16, 15, 14, 13, 12, 11, 10]
name: 🐘 PostgreSQL ${{ matrix.pg }}
runs-on: ubuntu-latest
container: pgxn/pgxn-tools
steps:
- name: Start PostgreSQL ${{ matrix.pg }}
run: pg-start ${{ matrix.pg }}
- name: Check out the repo
uses: actions/checkout@v4
Comment on lines +10 to +15

@coderabbitai coderabbitai Bot Jul 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Pin container image and action, and disable credential persistence.

The pgxn/pgxn-tools container and actions/checkout@v4 are referenced by tag rather than by immutable digest/SHA, which is a supply-chain risk. Additionally, actions/checkout persists the GITHUB_TOKEN in the local git config by default; since this job doesn't push, set persist-credentials: false.

🔒️ Proposed fix 
-    container: pgxn/pgxn-tools
+    container: pgxn/pgxn-tools@sha256:<digest>
     steps:
       - name: Start PostgreSQL ${{ matrix.pg }}
         run: pg-start ${{ matrix.pg }}
       - name: Check out the repo
-        uses: actions/checkout@v4
+        uses: actions/checkout@<pin-to-sha>
+        with:
+          persist-credentials: false
🧰 Tools
🪛 zizmor (1.26.1)

[warning] 14-15: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

[warning] 13-13: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

[error] 15-15: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

[error] 10-10: unpinned image references (unpinned-images): container image is unpinned

(unpinned-images)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 10 - 15, Pin the pgxn/pgxn-tools
container to an immutable image digest and pin actions/checkout to a full commit
SHA instead of mutable tags. In the checkout step, set persist-credentials to
false.

Source: Linters/SAST tools

- name: Test on PostgreSQL ${{ matrix.pg }}
run: make test PGUSER=postgres
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ regression.out
# Misc
tmp/
.DS_Store
.claude/settings.local.json
13 changes: 0 additions & 13 deletions .travis.yml
View file
Open in desktop

This file was deleted.

36 changes: 0 additions & 36 deletions pg-travis-test.sh
View file
Open in desktop

This file was deleted.

13 changes: 10 additions & 3 deletions pgxntool/HISTORY.asc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
STABLE
------
== Support 13+
The `--load-language` option was removed from `pg_regress` in 13.

== Reduce verbosity from test setup
As part of this change, you will want to review the changes to test/deps.sql.

=== Support asciidoc documentation targets
By default, if asciidoctor or asciidoc exists on the system, any files in doc/ that end in .adoc or .asciidoc will be processed to html.
See the README for full details.
Expand All @@ -12,11 +18,12 @@ If a test input file changes we certainly need to re-run tests.

=== Have test/pgxntool/setup.sql install tap before running deps.sql

=== Reduce verbosity from test setup
As part of this change, you will want to review the changes to test/deps.sql.

=== Support other asciidoc extensions

=== Create the test/sql/ directory during setup

=== Use `--sudo` option when installing pgtap

0.2.0
-----
### Stop using $(VERSION)
Expand Down
14 changes: 10 additions & 4 deletions pgxntool/base.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ TEST_SQL_FILES += $(wildcard $(TESTDIR)/sql/*.sql)
TEST_RESULT_FILES = $(patsubst $(TESTDIR)/sql/%.sql,$(TESTDIR)/expected/%.out,$(TEST_SQL_FILES))
TEST_FILES = $(TEST_SOURCE_FILES) $(TEST_SQL_FILES)
REGRESS = $(sort $(notdir $(subst .source,,$(TEST_FILES:.sql=)))) # Sort is to get unique list
REGRESS_OPTS = --inputdir=$(TESTDIR) --outputdir=$(TESTOUT) --load-language=plpgsql
REGRESS_OPTS = --inputdir=$(TESTDIR) --outputdir=$(TESTOUT) # See additional setup below
MODULES = $(patsubst %.c,%,$(wildcard src/*.c))
ifeq ($(strip $(MODULES)),)
MODULES =# Set to NUL so PGXS doesn't puke
Expand All @@ -57,8 +57,10 @@ GE91 = $(call test, $(MAJORVER), -ge, 91)

ifeq ($(GE91),yes)
all: $(EXTENSION_VERSION_FILES)
endif

#DATA = $(wildcard sql/*--*.sql)
ifeq ($($call test, $(MAJORVER), -lt 13), yes)
REGRESS_OPTS += --load-language=plpgsql
endif
Comment on lines +60 to 64

@coderabbitai coderabbitai Bot Jul 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Fix Makefile syntax error: --load-language=plpgsql is never added for any PostgreSQL version.

Line 62 has two defects that prevent the version-gated REGRESS_OPTS addition from ever executing:

  1. $($call typo — should be $(call. The extra $ causes GNU Make to interpret $c as a single-char variable reference (undefined → empty), making the outer $(all test, …) resolve to an undefined variable → empty string. The ifeq always evaluates to false.

  2. Missing comma and wrong comparison value — -lt 13 is passed as a single argument to the test function (which expects 3 comma-separated args), and the value should be 130 not 13 because MAJORVER is already multiplied by 10 (line 51: PG 12 → 120, PG 13 → 130). Even with the $(call fix, test 120 -lt 13 is false for PG 12.

Compare with the correct pattern on line 56: $(call test, $(MAJORVER), -ge, 91).

🐛 Proposed fix for line 62 
-ifeq ($($call test, $(MAJORVER), -lt 13), yes)
+ifeq ($(call test, $(MAJORVER), -lt, 130), yes)
 	REGRESS_OPTS += --load-language=plpgsql
 endif
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
endif
#DATA = $(wildcard sql/*--*.sql)
ifeq ($($call test, $(MAJORVER), -lt 13), yes)
REGRESS_OPTS += --load-language=plpgsql
endif
endif
ifeq ($(call test, $(MAJORVER), -lt, 130), yes)
REGRESS_OPTS += --load-language=plpgsql
endif
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pgxntool/base.mk` around lines 60 - 64, Fix the version-gated condition in
the Makefile by changing the malformed `ifeq` expression to use `$(call test,
$(MAJORVER), -lt, 130)`, matching the argument pattern used by the existing
condition and the scaled `MAJORVER` values. Keep the `REGRESS_OPTS +=
--load-language=plpgsql` assignment unchanged.


PGXS := $(shell $(PG_CONFIG) --pgxs)
Expand All @@ -77,8 +79,12 @@ installcheck: $(TEST_RESULT_FILES) $(TEST_OUT_FILES) $(TEST_SQL_FILES) $(TEST_SO

# make test: run any test dependencies, then do a `make install installcheck`.
# If regressions are found, it will output them.
#
# This used to depend on clean as well, but that causes problems with
# watch-make if you're generating intermediate files. If tests end up needing
# clean it's an indication of a missing dependency anyway.
.PHONY: test
test: clean testdeps install installcheck
test: testdeps install installcheck
@if [ -r $(TESTOUT)/regression.diffs ]; then cat $(TESTOUT)/regression.diffs; fi

# make results: runs `make test` and copy all result files to expected
Expand Down Expand Up @@ -220,6 +226,6 @@ installcheck: pgtap
pgtap: $(DESTDIR)$(datadir)/extension/pgtap.control

$(DESTDIR)$(datadir)/extension/pgtap.control:
pgxn install pgtap
pgxn install pgtap --sudo

endif # fndef PGXNTOOL_NO_PGXS_INCLUDE
1 change: 1 addition & 0 deletions pgxntool/setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ git add META.json
mkdir -p sql test src

cd test
mkdir -p sql
safecp ../pgxntool/test/deps.sql deps.sql
[ -d pgxntool ] || ln -s ../pgxntool/test/pgxntool .
git add pgxntool
Expand Down
Binary file removed sql/.object_reference.sql.swo
View file
Open in desktop
Binary file not shown.
8 changes: 4 additions & 4 deletions test/dump/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ if [ "$1" == "-f" ]; then
fi

echo Creating dump database
createdb test_dump && psql -f test/dump/load_all.sql test_dump > $create_log || die 3 "Unable to create dump database"
createdb test_dump && psql -Xf test/dump/load_all.sql test_dump > $create_log || die 3 "Unable to create dump database"

# Ensure no errors in log
check_log() {
Expand All @@ -45,16 +45,16 @@ check_log $create_log creation

echo Running dump and restore
# No real need to cat the log on failure here; psql will generate an error and even if not verify will almost certainly catch it
createdb test_load && PAGER='' psql -c '\df pg_get_object_address' test_load || die 5 'crap'
(echo 'BEGIN;' && pg_dump test_dump && echo 'COMMIT;') | psql -q -v VERBOSITY=verbose -v ON_ERROR_STOP=true test_load > $restore_log
createdb test_load && PAGER='' psql -Xc '\df pg_get_object_address' test_load || die 5 'crap'
(echo 'BEGIN;' && pg_dump test_dump && echo 'COMMIT;') | psql -q -X -v VERBOSITY=verbose -v ON_ERROR_STOP=true test_load > $restore_log
rc=$?
if [ $rc -ne 0 ]; then
cat $restore_log
die 4 "Unable to load database"
fi

echo Verifying restore
psql -f test/dump/verify.sql test_load > $verify_log || die 5 "Test failed"
psql -Xf test/dump/verify.sql test_load > $verify_log || die 5 "Test failed"

check_log $create_log verify

Expand Down
Loading