Skip to content

Validate PGXNTOOL_VERIFY_RESULTS_MODE - #132

Merged
jnasbyupgrade merged 3 commits into
Postgres-Extensions:masterfrom
jnasbyupgrade:pre-2.4.0-verify-results-mode
Oct 7, 2026
Merged

jnasbyupgrade merged 3 commits into
Postgres-Extensions:masterfrom
jnasbyupgrade:pre-2.4.0-verify-results-mode

Conversation

@jnasbyupgrade

@jnasbyupgrade jnasbyupgrade commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

PGXNTOOL_VERIFY_RESULTS_MODE is now validated: it accepts pgtap or diffs, case-insensitively, and any other value is a parse-time make error. Before this, any value other than exactly pgtap (for example the typo pgTap) silently selected diffs mode and weakened the make results safeguard.

Merge this PR first, before its paired pgxntool-test PR (link below). This change adds behavior that the pgxntool-test PR's new tests depend on. See pgxntool-test's CLAUDE.md, "Merge Order for Paired PRs".

Changes

  • base.mk: new pgxntool_validate_choice helper. It lowercases a single-word value, checks it against an allowed list, and otherwise calls $(error) with the variable name and the value it got. It uses the same case handling as pgxntool_validate_yesno, which is unchanged.
  • PGXNTOOL_VERIFY_RESULTS_MODE is normalized through that helper. An empty value is rejected too. Valid values select the same mode as before.
  • README.asc documents the accepted values and case handling, both in the verify-results safeguard section and in the Configuration Variables entry.
  • HISTORY.asc has a new STABLE entry. There is no issue number, so the "Issues fixed" line is unchanged.
  • README.html is regenerated. It was already behind README.asc on master (the check-duplicate-docs section was missing), so the diff includes that catch-up as well as this change.

Sibling sites checked

  • base.mk: the comment block for the variable is updated. ifeq ($(PGXNTOOL_VERIFY_RESULTS_MODE),pgtap) is unaffected, because it now sees the normalized value.
  • README.asc: the = diffs # or pgtap example is unaffected.
  • HISTORY.asc: the existing mention under the make results fix is unaffected.
  • CLAUDE.md: does not mention the variable.

Conflict note

Open PRs #129 and #130 also regenerate README.html, so whichever of them merges later will conflict on that file. Resolve it by regenerating README.html, not by hand-merging. HISTORY.asc and README.asc merge cleanly against #128, #129 and #130.

Paired pgxntool-test PR: Postgres-Extensions/pgxntool-test#99

🤖 Generated with Claude Code

Any value other than exactly `pgtap` silently selected `diffs` mode, so a
typo such as `pgTap` quietly weakened the `make results` safeguard.

- New `pgxntool_validate_choice` helper in `base.mk`: lowercases a
  single-word value and checks it against an allowed list, otherwise a
  parse-time `$(error)` naming the variable and the value.
- `PGXNTOOL_VERIFY_RESULTS_MODE` now accepts `pgtap` or `diffs`
  case-insensitively; anything else (including empty) is an error. Mode
  selection for valid values is unchanged.
- README.asc documents the accepted values and case handling; HISTORY.asc
  gets a STABLE entry. README.html is regenerated (it was already behind
  README.asc).

Related changes in pgxntool-test:
- `make-test.bats` covers default, mixed-case, invalid and empty values at
  the `print-` layer, plus mixed-case recipe selection via `make -n`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 7, 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: Advanced
  • Run ID: b7e346ef-efb6-4038-a29b-1a23862e776c

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
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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 merged commit 065dd27 into Postgres-Extensions:master Oct 7, 2026
4 checks passed
@jnasbyupgrade
jnasbyupgrade deleted the pre-2.4.0-verify-results-mode branch October 7, 2026 22:40
jnasbyupgrade added a commit to Postgres-Extensions/pgxntool-test that referenced this pull request Oct 7, 2026
This PR adds test coverage for Postgres-Extensions/pgxntool#132, which
validates `PGXNTOOL_VERIFY_RESULTS_MODE`. The variable now accepts
`pgtap` or `diffs` case-insensitively, and any other value is a
parse-time error.
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