From 396050a946b600541555911bb538145a839520e1 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Sat, 1 Aug 2026 17:29:26 -0500 Subject: [PATCH] Update internal-variable references for pgxntool's `_PGXNTOOL_` rename Add tests/updates for pgxntool commit e807c46 (renamed 5 internal-only Make variables to the `_PGXNTOOL_` shape): - `PGXNTOOL_BASE_MK_INCLUDED`, `PGXNTOOL_CONTROL_FILES`, `PGXNTOOL_EXTENSIONS`, `PGXNTOOL_INSTALL_SCHEDULE` -> `_PGXNTOOL_*` - `_CHECK_STALE_EXPECTED_SCRIPT` -> `_PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT` Updated the only test-suite references to these variables by name: - `test/standard/make-test.bats`: 3 sites passing `_CHECK_STALE_EXPECTED_SCRIPT` on the `make` command line to stub the script - `test/sequential/04-pgtle.bats`: `make print-PGXNTOOL_CONTROL_FILES` debug check - `test/lib/helpers.bash` and `CLAUDE.md`: updated comments/prose documenting the naming convention itself, including calling out `PGXNTOOL_DIR` as the deliberate, age-based exception left unrenamed Co-Authored-By: Claude --- CLAUDE.md | 25 ++++++++++++++++--------- test/lib/helpers.bash | 2 +- test/sequential/04-pgtle.bats | 2 +- test/standard/make-test.bats | 10 +++++----- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index cc376fe..20656c5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -213,14 +213,21 @@ section once resolved. 3. **Variables prefixed `PGXNTOOL_` that are designed for override** — defaulted with `?=`, or normalized via the validate/override pattern in `lib.sh` (e.g. `pgxntool_validate_yesno`). Excludes pure internal - plumbing like `PGXNTOOL_DIR`, `PGXNTOOL_CONTROL_FILES`, - `PGXNTOOL_EXTENSIONS` — never meant to be set by users. Internal-only - override points (seams that exist for pgxntool-test's own tests to - stub, not for end users to set — e.g. `_CHECK_STALE_EXPECTED_SCRIPT`) - are named with a leading `_` instead of the `PGXNTOOL_` prefix - specifically so the name itself signals "not part of the user-facing - surface," rather than relying on this prose exclusion list to catch - every case. + plumbing — never meant to be set by users — which is named + `_PGXNTOOL_...` instead (a leading underscore *before* the `PGXNTOOL_` + prefix, not replacing it) specifically so the name itself signals "not + part of the user-facing surface" while staying namespaced, rather than + relying on this prose exclusion list to catch every case: e.g. + `_PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT` (a seam that exists for + pgxntool-test's own tests to stub, not for end users to set), + `_PGXNTOOL_CONTROL_FILES`, `_PGXNTOOL_EXTENSIONS`, + `_PGXNTOOL_INSTALL_SCHEDULE`, `_PGXNTOOL_BASE_MK_INCLUDED`. One + deliberate exception: `PGXNTOOL_DIR` is equally pure internal plumbing + but keeps the bare `PGXNTOOL_` prefix — it's been present since + pgxntool's very first release and is referenced across `base.mk` plus + four separate shell scripts, making it the highest-risk name in the + 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). @@ -333,7 +340,7 @@ Concrete example 2 -- `EXTRA_CLEAN`/PGXS (`test/standard/make-test.bats`): `make When a Makefile-layer test needs to prove a script genuinely was (or wasn't) called, or that its exit status/output was correctly bubbled up -- not just that a failure from it was tolerated, which is a materially weaker claim -- prefer a dedicated "which script path to run" make variable over faking out an entire directory tree, and stub the script rather than relying on its real behavior. -Concrete example: the `check-stale-expected` recipe in `../pgxntool/base.mk` invokes `$(_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 `_CHECK_STALE_EXPECTED_SCRIPT` at the stub instead of touching `PGXNTOOL_DIR`. +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/helpers.bash b/test/lib/helpers.bash index ee401e2..d2dab21 100644 --- a/test/lib/helpers.bash +++ b/test/lib/helpers.bash @@ -909,7 +909,7 @@ ensure_foundation() { # General-purpose building block for "swap in a fake script and assert it # was/wasn't called, or that its exit status/output was correctly # propagated" -- pair it with whatever variable names the real script's -# path in the code under test (e.g. _CHECK_STALE_EXPECTED_SCRIPT for +# path in the code under test (e.g. _PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT for # check-stale-expected.sh). See "Proving a Script Was/Wasn't Invoked" in # CLAUDE.md for the reasoning, including why this is a make-variable # override rather than the PATH-shadowing technique BATS-ecosystem mocking diff --git a/test/sequential/04-pgtle.bats b/test/sequential/04-pgtle.bats index a020729..3f237dc 100644 --- a/test/sequential/04-pgtle.bats +++ b/test/sequential/04-pgtle.bats @@ -176,7 +176,7 @@ teardown_file() { # Verify control file is in Makefile dependencies # Use make print-VARIABLE to debug Makefile variable values - run make print-PGXNTOOL_CONTROL_FILES + run make print-_PGXNTOOL_CONTROL_FILES assert_success assert_contains "pgxntool-test.control" diff --git a/test/standard/make-test.bats b/test/standard/make-test.bats index 4b93eb5..f07700f 100755 --- a/test/standard/make-test.bats +++ b/test/standard/make-test.bats @@ -269,7 +269,7 @@ EOF # 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 - # _CHECK_STALE_EXPECTED_SCRIPT -- the one variable the + # _PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT -- the one variable the # 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 @@ -281,7 +281,7 @@ EOF local stub_script stub_script=$(make_stub_script check-stale-expected-stub 1 "" "$marker") - run make test PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED=no _CHECK_STALE_EXPECTED_SCRIPT="$stub_script" + run make test PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED=no _PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT="$stub_script" assert_success assert_file_not_exists "$marker" } @@ -315,7 +315,7 @@ EOF # 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` - # correctly surface whatever _CHECK_STALE_EXPECTED_SCRIPT does? A + # 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 # show that message; a stub that exits 0 must let it pass -- regardless @@ -323,13 +323,13 @@ EOF local stub_script stub_script=$(make_stub_script fail-stub 5 "STUB SENTINEL MESSAGE") - run make check-stale-expected _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 _CHECK_STALE_EXPECTED_SCRIPT="$stub_script" + run make check-stale-expected _PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT="$stub_script" assert_success }