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
11 changes: 6 additions & 5 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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).

Expand Down
79 changes: 79 additions & 0 deletions test/lib/pgxntool-variables.txt
Original file line number Diff line number Diff line change
@@ -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: "<class> <name>", 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
Loading
Loading