diff --git a/CLAUDE.md b/CLAUDE.md index 4851281..ab5ee66 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -189,7 +189,13 @@ section once resolved. 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. + `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. 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). @@ -292,7 +298,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 `$(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`. +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`. 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 0700ed2..ee401e2 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. PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT for +# path in the code under test (e.g. _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/standard/make-test.bats b/test/standard/make-test.bats index c0077c5..be161a6 100755 --- a/test/standard/make-test.bats +++ b/test/standard/make-test.bats @@ -220,7 +220,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 - # PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT -- the one variable the + # _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 @@ -232,7 +232,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 PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT="$stub_script" + run make test PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED=no _CHECK_STALE_EXPECTED_SCRIPT="$stub_script" assert_success assert_file_not_exists "$marker" } @@ -266,7 +266,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 PGXNTOOL_CHECK_STALE_EXPECTED_SCRIPT does? A + # correctly surface whatever _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 @@ -274,13 +274,13 @@ 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 _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 _CHECK_STALE_EXPECTED_SCRIPT="$stub_script" assert_success }