Skip to content

Update docs: make test now returns non-zero on regressions (pgxntool 2.3.0) #60

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 1 commit into
base: master
Choose a base branch
from
+12 −9
Open

Update docs: make test now returns non-zero on regressions (pgxntool 2.3.0) #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 4 additions & 3 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,9 +310,10 @@ jobs:
make check-relkind-source
# verify-results is the real gate: base.mk declares `verify-results:
# test`, so this runs the suite via its `test` prerequisite and then
# checks the pgtap/regression.diffs. A bare `make test` is redundant here
# and would not gate anyway -- it never exits non-zero on regressions
# (pgxntool marks installcheck `.IGNORE`), so failures would pass silently.
# checks the pgtap/regression.diffs. A bare `make test` is redundant here:
# it now also exits non-zero on regressions (pgxntool 2.3.0+), but
# verify-results is still the stricter, documented check, and confirms
# test actually ran.
make verify-results

# Style linter (https://github.com/Postgres-Extensions/linter, vendored at
Expand Down
8 changes: 5 additions & 3 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ and `pgxntool/CLAUDE.md` first. High-value gotchas that those docs explain:

- `DATA`, `MODULES`, `DOCS`, and `installcheck` are PGXS variables/targets, not
pgxntool's; pgxntool only wraps/seeds them. Don't assume they belong to pgxntool.
- `make test` does **not** return non-zero on test regressions — pgxntool marks
`installcheck` as `.IGNORE`. To actually detect failures, use `make verify-results`
(or inspect `test/regression.diffs`). This is why CI must gate on `verify-results`.
- `make test` returns non-zero on test regressions (pgxntool 2.3.0+): `installcheck`
itself is still marked `.IGNORE`, but `test`'s own recipe now checks
`test/regression.diffs` and exits non-zero if it's non-empty. `make verify-results`
remains the stricter, documented CI gate (it also confirms `test` actually ran), but
a bare `make test` failing locally is now a real signal too.
- `test/install/*.sql` runs once, committed, before the suite in the same `pg_regress`
Comment on lines 16 to 22

@github-actions github-actions Bot Aug 1, 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.

The new claim that make verify-results "also confirms test actually ran" doesn't match pgxntool 2.3.0's actual Makefile: verify-results depends on $(TEST_DEPS), not on test (pgxntool/base.mk:417). base.mk's own comment (lines 404-409) explains this was deliberate — test's recipe now exits non-zero as soon as it sees a regression, which would abort the chain before verify-results got to run if it depended on test. So make verify-results never executes the test recipe; it only guarantees $(TEST_DEPS) completed (which includes installcheck, running the suite, but not the test target itself).

The same "confirms test actually ran" claim was added in two other places by this PR and has the same problem:

  • cat_tools/.github/workflows/ci.yml

    Lines 314 to 317 in 1dc632f

    # it now also exits non-zero on regressions (pgxntool 2.3.0+), but
    # verify-results is still the stricter, documented check, and confirms
    # test actually ran.
    make verify-results

  • cat_tools/RELEASE.md

    Lines 42 to 44 in 1dc632f

    regression as of pgxntool 2.3.0, but `verify-results` remains the stricter,
    documented gate (it also confirms `test` actually ran).

Suggest rephrasing along the lines of: "verify-results does not depend on test; it depends on $(TEST_DEPS) directly (by design — see base.mk's comment on why) so test's early exit can't abort the check, and it applies a stricter pgtap-aware check on top."

invocation, so its state persists into every test. `test/build/*.sql` are separate
build sanity checks run before the suite.
Expand Down
6 changes: 3 additions & 3 deletions RELEASE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ what that version actually shipped.
- [ ] Open issues/PRs for this release reviewed, merged or deferred.
- [ ] CI green on all supported PostgreSQL versions (the `all-checks-passed` job on master).
- [ ] Locally: `make verify-results` passes. It depends on `test` (so it runs the suite
first, then gates on the results). `make test` alone is non-gating — pgxntool marks
`installcheck` `.IGNORE`, so it never returns non-zero on a regression; only
`verify-results` (which inspects `test/regression.diffs`) is a real gate.
first, then gates on the results). `make test` itself also returns non-zero on a
regression as of pgxntool 2.3.0, but `verify-results` remains the stricter,
documented gate (it also confirms `test` actually ran).

## 3. Decide the version and what to track
- [ ] Pick the new version (semantic versioning).
Expand Down
Loading