Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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).

Expand Down
2 changes: 1 addition & 1 deletion test/lib/helpers.bash
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions test/standard/make-test.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
}
Expand Down Expand Up @@ -266,21 +266,21 @@ 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
# of what the real script would have decided for the same directory.
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
}

Expand Down
Loading