diff --git a/HISTORY.asc b/HISTORY.asc index fc81966..05e7b7f 100644 --- a/HISTORY.asc +++ b/HISTORY.asc @@ -90,6 +90,12 @@ command, to any value; `\include` and `\include_relative` count as including `ps `test/install/*.out` was never really compared against anything, and is rewritten by every run, it's now gitignored -- stop committing it. +== Reject invalid `PGXNTOOL_VERIFY_RESULTS_MODE` values +Any value other than exactly `pgtap` silently selected `diffs` mode, so a +typo like `pgTap` quietly weakened the `make results` safeguard. The value +is now case-insensitive, and anything other than `pgtap` or `diffs` is a +hard `make` error. + == Guarantee `test/install/*.sql` ordering Install files now always run in byte-value (ASCII) filename order, regardless of filesystem or locale. If one install file depends on another, diff --git a/README.asc b/README.asc index 0dd1aa5..9824449 100644 --- a/README.asc +++ b/README.asc @@ -272,7 +272,7 @@ IMPORTANT: *`make results` requires manual verification first*. The correct work Never run `make results` without first verifying the test changes are correct. The `results` target copies files from `test/results/` to `test/expected/`, so running it blindly will make incorrect output become the new expected behavior. ==== verify-results safeguard -By default, `make results` will refuse to run if your tests are failing, so you can't accidentally promote incorrect output into the new expected results. Which failures it looks for is controlled by `PGXNTOOL_VERIFY_RESULTS_MODE`: +By default, `make results` will refuse to run if your tests are failing, so you can't accidentally promote incorrect output into the new expected results. Which failures it looks for is controlled by `PGXNTOOL_VERIFY_RESULTS_MODE`, which must be one of these two values (case-insensitive; anything else is a hard `make` error): `pgtap` (the default):: Scans `test/results/*.out` for pgTAP `not ok` lines (ignoring `# TODO` items) and plan-count mismatches, then also checks `test/regression.diffs` for any output mismatch pgTAP itself wouldn't catch. A test that has no expected output file yet is the one exception: it always shows up in `regression.diffs` (pgxntool creates an empty expected file for it, because `pg_regress` aborts on a missing one), so `make results` is allowed to create its first expected output — provided its own pgTAP output is clean and free of SQL errors. If a new test legitimately needs output containing a SQL error blessed, use `PGXNTOOL_ENABLE_VERIFY_RESULTS=no` for that one run. `diffs`:: Only checks whether `test/regression.diffs` exists, so it can't create a new test's first expected output; use `pgtap` mode, or `PGXNTOOL_ENABLE_VERIFY_RESULTS=no` for that one run. @@ -786,7 +786,7 @@ Default: `$(TESTDIR)`. Directory `pg_regress` writes actual test output to -- `$ === PGXNTOOL_VERIFY_RESULTS_MODE -Default: `pgtap`. Controls how the <<_verify_results_safeguard,verify-results safeguard>> decides whether tests are failing. +Default: `pgtap`. Controls how the <<_verify_results_safeguard,verify-results safeguard>> decides whether tests are failing. Must be `pgtap` or `diffs` (case-insensitive) when given explicitly; any other value, including an empty one, is a hard `make` error. ==== pgtap diff --git a/README.html b/README.html index 14c32d1..5658ad6 100644 --- a/README.html +++ b/README.html @@ -1082,7 +1082,7 @@

4.7.1. verify-results safeguard

-

By default, make results will refuse to run if your tests are failing, so you can’t accidentally promote incorrect output into the new expected results. Which failures it looks for is controlled by PGXNTOOL_VERIFY_RESULTS_MODE:

+

By default, make results will refuse to run if your tests are failing, so you can’t accidentally promote incorrect output into the new expected results. Which failures it looks for is controlled by PGXNTOOL_VERIFY_RESULTS_MODE, which must be one of these two values (case-insensitive; anything else is a hard make error):

@@ -2118,7 +2118,7 @@

8.9. PGXNTOOL_VERIFY_RESULTS_MODE

-

Default: pgtap. Controls how the verify-results safeguard decides whether tests are failing.

+

Default: pgtap. Controls how the verify-results safeguard decides whether tests are failing. Must be pgtap or diffs (case-insensitive) when given explicitly; any other value, including an empty one, is a hard make error.

8.9.1. pgtap

@@ -2217,7 +2217,7 @@

diff --git a/base.mk b/base.mk index 5b158ea..ae46e51 100644 --- a/base.mk +++ b/base.mk @@ -89,6 +89,15 @@ pgxntool_validate_yesno = $(strip \ $(shell echo "$(1)" | tr '[:upper:]' '[:lower:]'),\ $(error $(2) must be "yes" or "no", got "$(1)"))) +# Helper function: normalize a single-word variable to lowercase and validate +# it against a list of allowed values. +# Usage: $(call pgxntool_validate_choice,VALUE,VARIABLE_NAME,ALLOWED_VALUES) +# Returns the lowercase value, or errors if it isn't exactly one of ALLOWED_VALUES. +pgxntool_validate_choice = $(strip \ + $(if $(and $(filter 1,$(words $(1))),$(filter $(3),$(shell echo "$(1)" | tr '[:upper:]' '[:lower:]'))),\ + $(shell echo "$(1)" | tr '[:upper:]' '[:lower:]'),\ + $(error $(2) must be one of: $(3); got "$(1)"))) + # ------------------------------------------------------------------------------ # test-build: Sanity check extension files before running full test suite # ------------------------------------------------------------------------------ @@ -288,6 +297,8 @@ _CHECK_TEST_INSTALL_ERROR_STOP_SCRIPT ?= $(PGXNTOOL_DIR)/test/bin/check-test-ins # # Variable: PGXNTOOL_VERIFY_RESULTS_MODE # - Controls how verify-results detects test failures +# - Allowed values: "pgtap" or "diffs" (case-insensitive); anything else, +# including an empty value, is a parse-time error # - "pgtap" (default): scans test/results/*.out for "not ok" lines and plan # mismatches (TAP failures). Also checks regression.diffs as a fallback. # Use this mode when your test suite uses pgTap. @@ -306,6 +317,7 @@ endif # Default mode: pgtap (scans results/*.out for TAP failures) PGXNTOOL_VERIFY_RESULTS_MODE ?= pgtap +override PGXNTOOL_VERIFY_RESULTS_MODE := $(call pgxntool_validate_choice,$(PGXNTOOL_VERIFY_RESULTS_MODE),PGXNTOOL_VERIFY_RESULTS_MODE,pgtap diffs) # ------------------------------------------------------------------------------ # check-stale-expected: catch orphaned/unexpected test/expected/ files