diff --git a/CLAUDE.md b/CLAUDE.md index d2eff7a..e2ebd94 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -222,8 +222,9 @@ section once resolved. whole framework to rename (see issue #87 discussion); carrying the misleading prefix is judged lower-risk than touching it. 4. **Scripts a user is realistically expected to invoke by hand**: - `setup.sh`, `pgxntool-sync.sh`, `update-setup-files.sh`, and `pgtle.sh` - (including its own CLI flags, not just the make targets that wrap it). + `setup.sh`, `pgxntool-sync.sh`, `update-setup-files.sh`, `bin/version`, + and `pgtle.sh` (including its own CLI flags, not just the make targets + that wrap it). 5. **`DEBUG` is a special case**: its existence may be documented (it's fine for users to know it exists), but the specific level numbers are an internal implementation detail, not a documented contract — changing @@ -332,15 +333,15 @@ invokes, or an external build framework (PGXS) pgxntool builds on top of: - **An external framework's own established behavior** (e.g. PGXS's `clean`/`EXTRA_CLEAN` deletion mechanics) is that framework's responsibility, not pgxntool's to re-verify. What pgxntool IS responsible for is correctly *populating* the configuration PGXS consumes (e.g. `EXTRA_CLEAN` containing the right entry) and that the configuration genuinely *reaches* the mechanism it's supposed to configure (e.g. that entry actually appearing in the real `clean` recipe, not just in an isolated variable dump) -- not that PGXS then does the right thing with it, which is PGXS's own well-established behavior. - **The Makefile's own responsibility**, in either case, is wiring: does the recipe invoke the right script/target with the right arguments/variables, does it run at the correct point relative to other targets, does disabling a feature actually skip invoking it, and does `make` correctly propagate the invoked script's exit status and output. None of this requires re-running the invoked code's real decision logic -- it should be verified with a stub standing in for the real script, a `make -n` dry-run, or both, so the Makefile-layer tests would pass or fail identically no matter what the real invoked code's internals did. -Concrete example 1 -- `check-stale-expected` (script: `../pgxntool/test/bin/check-stale-expected.sh`; recipe: `../pgxntool/base.mk`): +Concrete example 1 -- `_check-stale-expected` (script: `../pgxntool/test/bin/check-stale-expected.sh`; recipe: `../pgxntool/base.mk`): - `test/standard/check-stale-expected-script.bats` invokes the script directly against a bare scratch directory and owns all of the decision-logic coverage: orphan detection, the pg_regress `_N.out` alternate-file convention, the `test/build`/`test/build/expected` pair, the file-type sub-check, and argument validation. -- `test/standard/make-test.bats` owns only the Makefile-integration concerns: the `check-stale-expected: installcheck` ordering guarantee (via `make -n` dry-run), that the recipe passes the right positional args (also via dry-run), that `PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED=no` genuinely skips invoking the script (via a stub -- see below), that `make` correctly propagates a stub's exit status/output either way, and one real end-to-end `make test` failure on a stale file (needs actual pg_regress output to prove the ordering held in practice). It does **not** re-test orphan/alternate-file/file-type decision logic -- that would just be the same assertion running twice, once cheaply and once expensively. +- `test/standard/make-test.bats` owns only the Makefile-integration concerns: the `_check-stale-expected: installcheck` ordering guarantee (via `make -n` dry-run), that the recipe passes the right positional args (also via dry-run), that `PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED=no` genuinely skips invoking the script (via a stub -- see below), that `make` correctly propagates a stub's exit status/output either way, and one real end-to-end `make test` failure on a stale file (needs actual pg_regress output to prove the ordering held in practice). It does **not** re-test orphan/alternate-file/file-type decision logic -- that would just be the same assertion running twice, once cheaply and once expensively. Concrete example 2 -- `EXTRA_CLEAN`/PGXS (`test/standard/make-test.bats`): `make clean` actually deleting `test/results/` is PGXS's own `pg_regress_clean_files`/`EXTRA_CLEAN` mechanism at work -- established, external behavior pgxntool doesn't need to re-verify by creating a real directory, running `make clean`, and checking it's gone. What pgxntool owns is (a) `EXTRA_CLEAN` listing the right entry (`make print-EXTRA_CLEAN`) and (b) that entry actually percolating into the real `clean` recipe's command line (`make -n clean` dry-run) -- proving pgxntool's own wiring is correct without needing to invoke or verify PGXS's deletion mechanics at all. #### Proving a Script Was/Wasn't Invoked (or That Its Result Was Propagated Correctly) -See `../ai/CLAUDE.md`'s Testability section for the general stub-vs-directory-fake guidance. Concrete example: the `check-stale-expected` recipe in `../pgxntool/base.mk` invokes `$(_PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT)` (default `$(PGXNTOOL_DIR)/test/bin/check-stale-expected.sh`) instead of a hardcoded path. `make_stub_script` in `test/lib/helpers.bash` writes a throwaway stub (configurable exit code, stdout text, marker file) to prove either that the script was never invoked (`PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED=no` test) or that `make` correctly propagates the script's exit status/output (the dedicated propagation test) -- both in `test/standard/make-test.bats`, both pointing `_PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT` at the stub instead of touching `PGXNTOOL_DIR`. +See `../ai/CLAUDE.md`'s Testability section for the general stub-vs-directory-fake guidance. Concrete example: the `_check-stale-expected` recipe in `../pgxntool/base.mk` invokes `$(_PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT)` (default `$(PGXNTOOL_DIR)/test/bin/check-stale-expected.sh`) instead of a hardcoded path. `make_stub_script` in `test/lib/helpers.bash` writes a throwaway stub (configurable exit code, stdout text, marker file) to prove either that the script was never invoked (`PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED=no` test) or that `make` correctly propagates the script's exit status/output (the dedicated propagation test) -- both in `test/standard/make-test.bats`, both pointing `_PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT` at the stub instead of touching `PGXNTOOL_DIR`. This is deliberately narrower than overriding `PGXNTOOL_DIR` itself. `PGXNTOOL_DIR` is read by many unrelated targets (`META.json`, `control.mk`, `verify-results`, `pgtle.sh`, etc.), so faking it out wholesale to intercept one script would require a full copy of the real directory just to swap that one file -- expensive, and it defeats the purpose of the narrower variable. Reach for "add a dedicated variable, then stub just that seam" before reaching for a directory-level fake or an in-place edit of a live, shared checkout (e.g. temporarily `mv`-ing the real script aside). diff --git a/test/lib/pgxntool-variables.txt b/test/lib/pgxntool-variables.txt new file mode 100644 index 0000000..7f4ab5f --- /dev/null +++ b/test/lib/pgxntool-variables.txt @@ -0,0 +1,79 @@ +# Every make variable pgxntool's own *.mk files define, with its class. +# test/standard/make-variables.bats fails on any variable missing from this +# list (or listed but no longer defined), so adding a variable to pgxntool +# means deciding here which class it belongs to. +# +# Classes: +# api PGXNTOOL_* -- user-facing override point; document it in +# README.asc and treat changes as API changes. +# internal _PGXNTOOL_* (or _pgxntool_* for functions) -- plumbing, +# including test stub seams. Every new internal +# variable must use this shape. +# legacy Pre-convention names. Frozen: consumers may reference them, so +# they are not renamed, and nothing new may be added here. +# external Names owned by PGXS or GNU make that pgxntool assigns. +# +# Format: " ", one per line, sorted by name. Lines starting +# with # and blank lines are ignored. + +external .DEFAULT_GOAL +legacy ASCIIDOC +legacy ASCIIDOC_EXTS +legacy ASCIIDOC_FILES +legacy ASCIIDOC_HTML +legacy ASCIIDOC_template +external CONTRIB_TESTDB +external DATA +external DOCS +legacy DOCS_HTML +legacy DOC_DIRS +external EXTRA_CLEAN +legacy GE91 +legacy MAJORVER +external MODULEDIR +external MODULES +legacy PGVERSION +api PGXNTOOL_CHECK_EXPECTED_FILE_TYPES +legacy PGXNTOOL_DIR +api PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED +api PGXNTOOL_ENABLE_FS_INSTALL +api PGXNTOOL_ENABLE_PGXN_INSTALL +api PGXNTOOL_ENABLE_TEST_BUILD +api PGXNTOOL_ENABLE_TEST_INSTALL +api PGXNTOOL_ENABLE_TEST_INSTALL_ERROR_STOP_CHECK +api PGXNTOOL_ENABLE_VERIFY_RESULTS +api PGXNTOOL_POST_TAG_VERSION +api PGXNTOOL_VERIFY_RESULTS_MODE +api PGXNTOOL_distclean +legacy PGXN_REMOTE +external PGXS +external PG_CONFIG +external REGRESS +legacy REGRESS_DBHASH +legacy REGRESS_DBNAME +legacy TESTDIR +legacy TESTOUT +legacy TEST_BUILD_FILES +legacy TEST_BUILD_REGRESS +legacy TEST_BUILD_SQL_DIR +legacy TEST_BUILD_SQL_FILES +legacy TEST_DEPS +legacy TEST_FILES +legacy TEST_INSTALL_SQL_FILES +legacy TEST_RESULT_FILES +legacy TEST_SQL_FILES +internal _PGXNTOOL_BASE_MK_INCLUDED +internal _PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT +internal _PGXNTOOL_CHECK_TEST_INSTALL_ERROR_STOP_SCRIPT +internal _PGXNTOOL_CONTROL_FILES +internal _PGXNTOOL_DUPLICATE_DOCS +internal _PGXNTOOL_EXTENSIONS +internal _PGXNTOOL_INSTALL_SCHEDULE +internal _PGXNTOOL_POST_TAG_VERSION_BUMP_SCRIPT +internal _pgxntool_docs_named +internal _pgxntool_validate_choice +legacy pgxntool-sync-local +legacy pgxntool-sync-local-master +legacy pgxntool-sync-master +legacy pgxntool_validate_yesno +legacy test diff --git a/test/standard/make-test.bats b/test/standard/make-test.bats index 8116936..9ab4169 100755 --- a/test/standard/make-test.bats +++ b/test/standard/make-test.bats @@ -15,7 +15,7 @@ # for "existing mode"/pg_tle-style testing (issues #55, #90) # - PGXNTOOL_ENABLE_PGXN_INSTALL can independently disable the pgtap # dependency's own `pgxn install --sudo` auto-install -# - check-stale-expected catches orphaned test/expected/*.out files (issue #14) +# - _check-stale-expected catches orphaned test/expected/*.out files (issue #14) # - `make test` exits non-zero on a real regression.diffs mismatch (issue #49) # - verify-results blocks `make results` when tests are failing, lets a # brand-new test's first expected output through (issue #119), and can be @@ -151,7 +151,7 @@ EOF # Test: installcheck must run after install, even when pulled in indirectly # (issue #79) # -# `test`'s TEST_DEPS lists `install installcheck` (and check-stale-expected, +# `test`'s TEST_DEPS lists `install installcheck` (and _check-stale-expected, # which itself depends on installcheck -- see issue #14 below) as # independent, unordered prerequisites -- Make doesn't guarantee unrelated # same-target prerequisites build left-to-right. Before this fix, @@ -332,30 +332,30 @@ EOF # Test: test-build must gate the full suite, not run after it (issue #108) # -# TEST_DEPS lists `check-stale-expected test-build install installcheck` as +# TEST_DEPS lists `_check-stale-expected test-build install installcheck` as # independent, unordered prerequisites of `test` (see the issue #79 section # above for why position in TEST_DEPS is not an ordering guarantee). -# check-stale-expected's own `check-stale-expected: installcheck` edge pulls +# _check-stale-expected's own `_check-stale-expected: installcheck` edge pulls # in installcheck (and, via `installcheck: install`, install) while Make is -# still resolving check-stale-expected -- before Make ever reaches the +# still resolving _check-stale-expected -- before Make ever reaches the # separately-listed `test-build` prerequisite. So the full pg_regress suite # actually runs before test-build's own sanity check, even though test-build # exists to catch build errors before the full suite runs. An explicit # `installcheck: test-build` edge (added only when # PGXNTOOL_ENABLE_TEST_BUILD=yes) is the fix, mirroring the `installcheck: -# install` and `check-stale-expected: installcheck` edges already in place. +# install` and `_check-stale-expected: installcheck` edges already in place. @test "test-build's recipe runs before installcheck's pg_regress invocation (issue #108)" { # No PostgreSQL needed: the recipe order in `make -n test` output is enough # to prove the dependency edge is (or isn't) real, same approach as the - # check-stale-expected ordering test below. + # _check-stale-expected ordering test below. run make -n test PGXNTOOL_ENABLE_TEST_BUILD=yes assert_success local build_line pg_regress_line build_line=$(echo "$output" | grep -n "run-test-build.sh" | head -1 | cut -d: -f1) # Exclude test-build's own pg_regress invocation (identified by - # --outputdir=test/build, same convention as the check-stale-expected + # --outputdir=test/build, same convention as the _check-stale-expected # ordering test below) to isolate the main suite's invocation. pg_regress_line=$(echo "$output" | grep -n "pg_regress " | grep -v -- '--outputdir=test/build' | tail -1 | cut -d: -f1) @@ -365,12 +365,12 @@ EOF error "test-build (dry-run line $build_line) must come before installcheck's pg_regress (line $pg_regress_line) -- test-build is meant to gate the full suite, not run after it" } -# Test: check-stale-expected (issue #14) +# Test: _check-stale-expected (issue #14) # # `make test` never caught a stale test/expected/*.out left behind after a -# test/sql/*.sql file was renamed or removed. check-stale-expected fails +# test/sql/*.sql file was renamed or removed. _check-stale-expected fails # loudly instead. It runs AFTER install/installcheck (pg_regress), via an -# explicit `check-stale-expected: installcheck` dependency edge in base.mk, +# explicit `_check-stale-expected: installcheck` dependency edge in base.mk, # not as an early fail-fast check -- see the "runs after pg_regress, not # before" test below. # @@ -378,13 +378,13 @@ EOF # check-stale-expected-script.bats for the script's own decision-logic tests # (not duplicated here). -@test "check-stale-expected depends on installcheck, so it runs after pg_regress, not before" { +@test "_check-stale-expected depends on installcheck, so it runs after pg_regress, not before" { # Capture dry-run make output and ensure that pg_regress is called before # check-stale-expected.sh. # - # check-stale-expected must run AFTER pg_regress (installcheck), not + # _check-stale-expected must run AFTER pg_regress (installcheck), not # before -- Make only guarantees order via a real dependency edge, not - # position in TEST_DEPS, so this is enforced by `check-stale-expected: + # position in TEST_DEPS, so this is enforced by `_check-stale-expected: # installcheck` in base.mk. No PostgreSQL needed for this: the recipe # order in `make -n test` output is enough to prove the dependency edge # is real. @@ -394,13 +394,13 @@ EOF local pg_regress_line check_line # Exclude test-build's own (unrelated) pg_regress invocation, identified # by its --outputdir=test/build. test-build has no dependency relationship - # with check-stale-expected -- position in TEST_DEPS is not an ordering + # with _check-stale-expected -- position in TEST_DEPS is not an ordering # guarantee (see comment above) -- so depending on where test-build lands - # in TEST_DEPS, its recipe can print before or after check-stale-expected's + # in TEST_DEPS, its recipe can print before or after _check-stale-expected's # in this dry-run, which would make a plain "last pg_regress mention" # search pick up the wrong invocation. Filtering it out leaves only the # main suite's pg_regress call, whose ordering relative to - # check-stale-expected IS guaranteed (by the explicit dependency edge). + # _check-stale-expected IS guaranteed (by the explicit dependency edge). pg_regress_line=$(echo "$output" | grep -n "pg_regress " | grep -v -- '--outputdir=test/build' | tail -1 | cut -d: -f1) check_line=$(echo "$output" | grep -n "check-stale-expected.sh" | head -1 | cut -d: -f1) @@ -410,25 +410,25 @@ EOF error "check-stale-expected.sh (dry-run line $check_line) must come after pg_regress (line $pg_regress_line)" } -@test "check-stale-expected passes on clean template state" { +@test "_check-stale-expected passes on clean template state" { # Not a decision-logic test (no orphan/alternate-file scenario is # crafted) -- this is an end-to-end smoke check that the real template # stays in the passing state the Template Requirements section of # CLAUDE.md requires, exercised through the real recipe and real script. - run make check-stale-expected + run make _check-stale-expected assert_success } -@test "check-stale-expected recipe invokes the script with TESTDIR and the file-types arg" { +@test "_check-stale-expected recipe invokes the script with TESTDIR and the file-types arg" { # base.mk's responsibility, not the script's: does the recipe actually # pass the right positional arguments? Verified via dry-run (no # Postgres/script execution needed) for both the default value and an # explicit override, so this only exercises the make plumbing. - run make -n check-stale-expected + run make -n _check-stale-expected assert_success assert_contains "$output" "test/bin/check-stale-expected.sh test yes" - run make -n check-stale-expected PGXNTOOL_CHECK_EXPECTED_FILE_TYPES=no + run make -n _check-stale-expected PGXNTOOL_CHECK_EXPECTED_FILE_TYPES=no assert_success assert_contains "$output" "test/bin/check-stale-expected.sh test no" } @@ -436,13 +436,13 @@ EOF @test "PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED=no: make test never invokes check-stale-expected.sh" { skip_if_no_postgres - # Disabling via this variable drops the check-stale-expected target from + # Disabling via this variable drops the _check-stale-expected target from # TEST_DEPS (and its own definition) entirely -- see base.mk -- so the # script must never even be invoked, not merely have a failure from it # ignored. That's a materially stronger claim than "make test succeeds # despite a stale file", so prove it directly: point # _PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT -- the one variable the - # check-stale-expected recipe actually invokes (see base.mk) -- at a stub + # _check-stale-expected recipe actually invokes (see base.mk) -- at a stub # that only touches a marker file and fails. No need to fake out # PGXNTOOL_DIR itself, since this variable is the sole thing standing # between the target and the real script. If the marker never appears, @@ -461,13 +461,13 @@ EOF @test "make test fails on a stale expected file, but only after pg_regress has already run" { skip_if_no_postgres - # Unlike an early fail-fast design, check-stale-expected now depends on + # Unlike an early fail-fast design, _check-stale-expected now depends on # installcheck, so pg_regress must have already produced real actual-output # files by the time make test fails on the stale file. test/results/*.out # is written for every test regardless of pass/fail (regression.diffs is # only written when a test actually differs, which the template's own # passing suite never triggers) -- so its presence is the correct proof - # that pg_regress actually ran, not just that check-stale-expected itself + # that pg_regress actually ran, not just that _check-stale-expected itself # failed. touch test/expected/orphan_test.out rm -rf test/results @@ -478,7 +478,7 @@ EOF local out_count out_count=$(find test/results -name '*.out' 2>/dev/null | wc -l) - [ "$out_count" -gt 0 ] || error "test/results has no .out files -- pg_regress apparently never ran before check-stale-expected failed" + [ "$out_count" -gt 0 ] || error "test/results has no .out files -- pg_regress apparently never ran before _check-stale-expected failed" rm -f test/expected/orphan_test.out } @@ -486,7 +486,7 @@ EOF @test "make correctly propagates check-stale-expected.sh's exit status and output" { # base.mk's responsibility, not the script's decision logic (the real # script's distinct exit codes and messages are already covered directly - # in check-stale-expected-script.bats): does `make check-stale-expected` + # in check-stale-expected-script.bats): does `make _check-stale-expected` # correctly surface whatever _PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT does? A # stub that deterministically prints a message and exits nonzero must # make the target (and `make`'s own recipe-failure handling) fail and @@ -495,36 +495,36 @@ EOF local stub_script stub_script=$(make_stub_script fail-stub 5 "STUB SENTINEL MESSAGE") - run make check-stale-expected _PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT="$stub_script" + run make _check-stale-expected _PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT="$stub_script" assert_failure assert_contains "$output" "STUB SENTINEL MESSAGE" stub_script=$(make_stub_script pass-stub 0) - run make check-stale-expected _PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT="$stub_script" + run make _check-stale-expected _PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT="$stub_script" assert_success } -# Test: check-test-install-error-stop (issue #97) +# Test: _check-test-install-error-stop (issue #97) # # test/install/*.sql files never get a real diff (see the IMPORTANT note in # base.mk's test/install section) -- ON_ERROR_STOP is the only thing that -# still turns a hard error into a build failure. check-test-install-error-stop +# still turns a hard error into a build failure. _check-test-install-error-stop # is a pure static scan enforcing that every test/install/*.sql file sets it. # The script's own pass/fail decision logic is covered directly in # check-test-install-error-stop-script.bats; this file covers only base.mk's # wiring: does `make test` actually invoke the script, and can it be disabled. -@test "check-test-install-error-stop passes on clean template state" { +@test "_check-test-install-error-stop passes on clean template state" { # End-to-end smoke check that the real template stays in the passing state # CLAUDE.md's Template Requirements section requires, through the real # recipe and real script. - run make check-test-install-error-stop + run make _check-test-install-error-stop assert_success } -@test "check-test-install-error-stop recipe invokes the script with TESTDIR" { - run make -n check-test-install-error-stop +@test "_check-test-install-error-stop recipe invokes the script with TESTDIR" { + run make -n _check-test-install-error-stop assert_success assert_contains "$output" "test/bin/check-test-install-error-stop.sh test" } @@ -532,15 +532,15 @@ EOF @test "PGXNTOOL_ENABLE_TEST_INSTALL_ERROR_STOP_CHECK=no: make test never invokes the script" { skip_if_no_postgres - # Same proof pattern as check-stale-expected's disable test above: point - # _CHECK_TEST_INSTALL_ERROR_STOP_SCRIPT at a stub that only touches a - # marker file and fails. If the marker never appears, the script was + # Same proof pattern as _check-stale-expected's disable test above: point + # _PGXNTOOL_CHECK_TEST_INSTALL_ERROR_STOP_SCRIPT at a stub that only touches + # a marker file and fails. If the marker never appears, the script was # genuinely never invoked, not merely tolerated. local marker="$BATS_TEST_TMPDIR/check-test-install-error-stop-invoked" local stub_script stub_script=$(make_stub_script check-test-install-error-stop-stub 1 "" "$marker") - run make test PGXNTOOL_ENABLE_TEST_INSTALL_ERROR_STOP_CHECK=no _CHECK_TEST_INSTALL_ERROR_STOP_SCRIPT="$stub_script" + run make test PGXNTOOL_ENABLE_TEST_INSTALL_ERROR_STOP_CHECK=no _PGXNTOOL_CHECK_TEST_INSTALL_ERROR_STOP_SCRIPT="$stub_script" assert_success assert_file_not_exists "$marker" } @@ -549,13 +549,13 @@ EOF local stub_script stub_script=$(make_stub_script fail-stub 5 "STUB SENTINEL MESSAGE") - run make check-test-install-error-stop _CHECK_TEST_INSTALL_ERROR_STOP_SCRIPT="$stub_script" + run make _check-test-install-error-stop _PGXNTOOL_CHECK_TEST_INSTALL_ERROR_STOP_SCRIPT="$stub_script" assert_failure assert_contains "$output" "STUB SENTINEL MESSAGE" stub_script=$(make_stub_script pass-stub 0) - run make check-test-install-error-stop _CHECK_TEST_INSTALL_ERROR_STOP_SCRIPT="$stub_script" + run make _check-test-install-error-stop _PGXNTOOL_CHECK_TEST_INSTALL_ERROR_STOP_SCRIPT="$stub_script" assert_success } diff --git a/test/standard/make-variables.bats b/test/standard/make-variables.bats new file mode 100755 index 0000000..14d1c8b --- /dev/null +++ b/test/standard/make-variables.bats @@ -0,0 +1,108 @@ +#!/usr/bin/env bats + +# Test: inventory of the make variables pgxntool defines +# +# Fails whenever pgxntool's *.mk files define a variable that +# test/lib/pgxntool-variables.txt doesn't classify, or stop defining one it +# lists, so every new variable gets a deliberate api/internal/legacy/external +# decision. Needs pg_config (for PGXS) but no foundation env or Postgres. +# +# The list comes from make's own database (`make -p`), keeping each variable +# whose origin line ("# makefile (from 'pgxntool/.mk', ...)", or +# "# 'override' directive (from ...)") points into pgxntool. That origin is +# the variable's *last* assignment, so a variable +# pgxntool sets but PGXS later appends to (REGRESS_OPTS) shows up as PGXS's and +# isn't listed. Variables that only ever arrive on a sub-make command line +# (_PGXNTOOL_TEST_BUILD_ACTIVE) don't appear either. +# +# Variables generated into the extension's meta.mk/control.mk (PGXN, +# PGXNVERSION, EXTENSION_*) are deliberately out of scope: those files live +# outside pgxntool/, so the origin filter drops them. Whether they need their +# own naming rule is a separate question. + +load ../lib/helpers + +setup_file() { + setup_topdir + load_test_env "make-variables" +} + +setup() { + load_test_env "make-variables" +} + +# Print the sorted names of every variable whose last assignment is in +# pgxntool/*.mk, from a bare scratch extension that includes base.mk. +pgxntool_make_variables() { + local dir="$BATS_TEST_TMPDIR/inventory" + mkdir -p "$dir/test/sql" "$dir/test/build" "$dir/test/install" + ln -s "$PGXNREPO" "$dir/pgxntool" + echo 'include pgxntool/base.mk' > "$dir/Makefile" + # Test-build and test/install variables are defined only when those + # directories hold .sql files. + touch "$dir/test/sql/a.sql" "$dir/test/build/a.sql" "$dir/test/install/a.sql" + # Pre-existing generated files keep make from trying to regenerate them + # (which fails noisily without a control file or META.in.json). + touch "$dir/META.json" "$dir/meta.mk" "$dir/control.mk" + + # A clean environment keeps env/command-line origins (MAKEFLAGS from the + # outer make, an exported TESTDIR, ...) from masking pgxntool's own + # assignments. PG_CONFIG is honored by putting its directory on PATH. + local path="$PATH" + [ -n "${PG_CONFIG:-}" ] && path="$(dirname "$PG_CONFIG"):$path" + + # The goal doesn't exist on purpose: -p dumps the database regardless, and + # nothing gets built. make's exit status is therefore always nonzero. + (cd "$dir" && env -i PATH="$path" HOME="$HOME" make -pn __no_such_target__ 2>/dev/null) | + awk ' + /^# (makefile|.override. directive) \(from .pgxntool\/[^\047]*\.mk., line [0-9]+\)$/ { + if ((getline line) > 0) { + n = split(line, f, /[ \t]+/) + name = (f[1] == "define" || f[1] == "override") ? f[2] : f[1] + print name + } + }' | LC_ALL=C sort -u +} + +@test "every make variable pgxntool defines is classified in test/lib/pgxntool-variables.txt" { + command -v pg_config >/dev/null || [ -n "${PG_CONFIG:-}" ] || + error "pg_config not found; PGXS is needed to parse base.mk" + + local rel=test/lib/pgxntool-variables.txt + local list="$TOPDIR/$rel" + local actual expected + actual=$(pgxntool_make_variables) + expected=$(grep -v -e '^#' -e '^[[:space:]]*$' "$list" | awk '{print $2}' | LC_ALL=C sort -u) + + # Guard against a vacuous pass if the enumeration itself breaks. + echo "$actual" | grep -qx '_PGXNTOOL_BASE_MK_INCLUDED' || + error "variable enumeration found nothing from pgxntool/base.mk; got: $actual" + + local problems='' + local unclassified stale + unclassified=$(LC_ALL=C comm -23 <(echo "$actual") <(echo "$expected")) + stale=$(LC_ALL=C comm -13 <(echo "$actual") <(echo "$expected")) + [ -z "$unclassified" ] || problems+=" +New variable(s) not classified -- add each to $rel as api (PGXNTOOL_*), internal (_PGXNTOOL_*), or, for a PGXS/make-owned name, external; new legacy entries are not allowed: +$unclassified +" + [ -z "$stale" ] || problems+=" +Listed in $rel but no longer defined by pgxntool -- remove: +$stale +" + + # Each class must match its naming rule. + local mismatched + mismatched=$(grep -v -e '^#' -e '^[[:space:]]*$' "$list" | awk ' + $1 !~ /^(api|internal|legacy|external)$/ { print "unknown class: " $0; next } + $1 == "api" && $2 !~ /^PGXNTOOL_/ { print "api name must start with PGXNTOOL_: " $2 } + $1 == "internal" && $2 !~ /^_(PGXNTOOL|pgxntool)_/ { print "internal name must start with _PGXNTOOL_: " $2 } + $1 != "internal" && $2 ~ /^_(PGXNTOOL|pgxntool)_/ { print "_PGXNTOOL_ name must be classed internal: " $2 } + ') + [ -z "$mismatched" ] || problems+=" +Class/name mismatches in $rel: +$mismatched +" + + [ -z "$problems" ] || error "$problems" +}