From c4c221484842041b2b3584f4a7c857ce2f6f744e Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 3 Aug 2026 16:37:18 -0500 Subject: [PATCH 1/5] test/setup.sql: explicitly assert cat_tools' own schemas are never on search_path cat_tools' control file pins schema = 'cat_tools' with relocatable = false -- it can never be installed into a user-chosen schema, so there was never a "which schema" question to test. The real, narrower risk is that some cat_tools view/function could call another cat_tools object unqualified, relying on search_path to resolve it -- which would keep working by accident in any session that happens to have 'cat_tools' searchable, and only break for a real caller who doesn't. Verified empirically before adding this: pgxntool's own tap_setup.sql (\i'd by every test/sql/*.sql file via test/setup.sql) already sets `search_path = tap, public` -- cat_tools' schemas are already never on it today, for every test in the suite, with no toggle needed. This assertion just makes that fact explicit and self-documenting instead of true by accident of the shared baseline, so a future change to it fails loudly instead of silently widening what "passing" means. current_schemas(false) is Postgres's own unqualified-name search list, so checking cat_tools/_cat_tools are absent from it is a direct proof, not a proxy for one (confirmed the check fires: manually SET search_path to include 'cat_tools' and confirmed the RAISE EXCEPTION triggers with the expected message). Always-on, no GUC, no variable, no separate test target -- this is not a toggleable dimension, just a permanent fact this suite now checks. --- test/setup.sql | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/setup.sql b/test/setup.sql index 9097b62..f227585 100644 --- a/test/setup.sql +++ b/test/setup.sql @@ -1,6 +1,29 @@ -- Pulls in deps.sql \i test/pgxntool/setup.sql +/* + * Self-check, not a toggle: pgxntool's own tap_setup.sql (just \i'd above) + * sets search_path = tap, public for every test file -- cat_tools' own + * schemas are never on it. That's what makes every pgTAP pass in this suite + * mean cat_tools' internal views/functions fully schema-qualify their own + * cross-references, rather than happening to resolve by search_path + * accident. current_schemas(false) IS Postgres's own unqualified-name + * search list, so confirming cat_tools/_cat_tools are absent from it is a + * direct proof, not a proxy for one. Asserted here (always, unconditionally) + * so a future change to the shared search_path baseline fails loudly instead + * of silently widening what "passing" means. + */ +DO $$ +BEGIN + IF 'cat_tools' = ANY (current_schemas(false)) OR '_cat_tools' = ANY (current_schemas(false)) THEN + RAISE EXCEPTION + 'cat_tools schema(s) must not be part of the resolved search_path during testing -- got %' + , current_schemas(false) + ; + END IF; +END +$$; + GRANT USAGE ON SCHEMA tap TO :"use_role", :"no_use_role"; CREATE FUNCTION pg_temp.exec( From cdfa9687d44f49cd3576ff66d3e3177581ab201a Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 3 Aug 2026 17:29:08 -0500 Subject: [PATCH 2/5] test: move search_path assertion into a real pgTAP test, checked at file end Replace test/setup.sql's DO-block assertion (checked once, at the START of every test file) with test/finish.sql, a real, plan()-counted pgTAP test \i'd right before finish() at the END of every test/sql/*.sql file. This is strictly stronger: a start-only check cannot see a test that mutates search_path partway through a file. Confirmed empirically that each test/sql file runs as its own psql connection wrapped in a transaction that never commits (rolled back when the connection closes), so a plain SET search_path already cannot leak into a LATER file regardless of when it's checked -- the real gap a start-only check leaves open is a mutation that stays in effect for the rest of THAT SAME file's own tests, which checking again at the end now catches. Not foolproof: a test that mutates search_path and then restores it before this check would still slip through. Making it a counted pgTAP test (not a DO block) means a new test file author bumping their own plan() count has to notice this test exists -- a natural reminder not to add a search_path-dependent test without thinking about it. Every test/sql/*.sql file's plan() count is bumped by 1 and expected output regenerated via `make results` accordingly. Co-Authored-By: Claude Sonnet 5 --- test/expected/attribute.out | 3 ++- test/expected/enum.out | 3 ++- test/expected/extension.out | 3 ++- test/expected/function.out | 3 ++- test/expected/general.out | 3 ++- test/expected/name__check.out | 3 ++- test/expected/object_type.out | 3 ++- test/expected/permissions.out | 3 ++- test/expected/pg_depends.out | 3 ++- test/expected/relation__.out | 3 ++- test/expected/routine__.out | 3 ++- test/expected/sequence.out | 3 ++- test/expected/trigger.out | 3 ++- test/finish.sql | 30 ++++++++++++++++++++++++++++++ test/setup.sql | 27 +++++++-------------------- test/sql/attribute.sql | 2 ++ test/sql/enum.sql | 2 ++ test/sql/extension.sql | 3 +++ test/sql/function.sql | 2 ++ test/sql/general.sql | 2 ++ test/sql/name__check.sql | 6 +++++- test/sql/object_type.sql | 3 +++ test/sql/permissions.sql | 2 ++ test/sql/pg_depends.sql | 2 ++ test/sql/relation__.sql | 2 ++ test/sql/routine__.sql | 2 ++ test/sql/sequence.sql | 2 ++ test/sql/trigger.sql | 3 +++ 28 files changed, 95 insertions(+), 34 deletions(-) create mode 100644 test/finish.sql diff --git a/test/expected/attribute.out b/test/expected/attribute.out index c9fe346..6a85915 100644 --- a/test/expected/attribute.out +++ b/test/expected/attribute.out @@ -1,8 +1,9 @@ \set ECHO none -1..5 +1..6 ok 1 - Verify public has no perms ok 2 - Non-existent relation throws error ok 3 - Non-existent column throws error ok 4 - Verify details of pg_class.relname ok 5 - Verify details of pg_tables.tablename +ok 6 - cat_tools schema(s) must not be part of the resolved search_path -- got {tap,public} # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/enum.out b/test/expected/enum.out index c010e3a..490b589 100644 --- a/test/expected/enum.out +++ b/test/expected/enum.out @@ -1,7 +1,8 @@ \set ECHO none -1..4 +1..5 ok 1 - Create test enum ok 2 - Permission denied trying to run functions ok 3 - Permission denied trying to run functions ok 4 +ok 5 - cat_tools schema(s) must not be part of the resolved search_path -- got {tap,public} # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/extension.out b/test/expected/extension.out index 05eda82..5e91526 100644 --- a/test/expected/extension.out +++ b/test/expected/extension.out @@ -1,5 +1,5 @@ \set ECHO none -1..7 +1..8 ok 1 - cat_tools.pg_extension view row count ok 2 - Sanity-check that we get a row for our extension ok 3 - pg_extension__get() returns correct data @@ -7,4 +7,5 @@ ok 4 - pg_extension__get() for non-existent extension throws an error ok 5 - Verify extension__schemas_unique(text) returns correct data ok 6 - Verify extension__schemas(text) returns correct data ok 7 - extension__schemas_unique with bogus extension fails +ok 8 - cat_tools schema(s) must not be part of the resolved search_path -- got {tap,public} # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/function.out b/test/expected/function.out index 8be118c..6160517 100644 --- a/test/expected/function.out +++ b/test/expected/function.out @@ -1,5 +1,5 @@ \set ECHO none -1..7 +1..8 ok 1 - Function _cat_tools.function__arg_to_regprocedure(text, text, text) should not be security definer ok 2 - Function _cat_tools.function__drop_temp(regprocedure, text) should not be security definer ok 3 - Verify regprocedure() @@ -11,4 +11,5 @@ WARNING: function__arg_types_text() is deprecated, use routine__parse_arg_types ok 6 - Verify deprecated function__arg_types_text() with INOUT and OUT WARNING: function__arg_types_text() is deprecated, use routine__parse_arg_types_text instead ok 7 - Verify deprecated function__arg_types_text() with simple args +ok 8 - cat_tools schema(s) must not be part of the resolved search_path -- got {tap,public} # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/general.out b/test/expected/general.out index 84e08e2..afa2175 100644 --- a/test/expected/general.out +++ b/test/expected/general.out @@ -1,4 +1,5 @@ \set ECHO none -1..1 +1..2 ok 1 - Schema __cat_tools should not exist +ok 2 - cat_tools schema(s) must not be part of the resolved search_path -- got {tap,public} # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/name__check.out b/test/expected/name__check.out index c966947..9e75dfb 100644 --- a/test/expected/name__check.out +++ b/test/expected/name__check.out @@ -1,8 +1,9 @@ \set ECHO none -1..5 +1..6 ok 1 - Verify public has no perms ok 2 - Simple name ok 3 - Name with spaces ok 4 - NULL ok 5 - Error on overflow +ok 6 - cat_tools schema(s) must not be part of the resolved search_path -- got {tap,public} # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/object_type.out b/test/expected/object_type.out index 1c9029b..7e57300 100644 --- a/test/expected/object_type.out +++ b/test/expected/object_type.out @@ -1,5 +1,5 @@ \set ECHO none -1..230 +1..231 ok 1 - sanity check size of pg_temp.extra_types() ok 2 - sanity check size of pg_temp.obj_type ok 3 - Permission denied trying to use types @@ -230,4 +230,5 @@ ok 227 - Simple 'regclass'::regtype should not return pg_catalog.regtype ok 228 - cat_tools.object__catalog('table') returns pg_catalog.pg_class ok 229 - cat_tools.object__catalog('table') returns pg_catalog.pg_class ok 230 - Verify objects__shared_src() returns correct values +ok 231 - cat_tools schema(s) must not be part of the resolved search_path -- got {public,tap,pg_catalog} # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/permissions.out b/test/expected/permissions.out index 4984217..4fcff24 100644 --- a/test/expected/permissions.out +++ b/test/expected/permissions.out @@ -1,5 +1,5 @@ \set ECHO none -1..160 +1..161 ok 1 - Permission denied trying to execute cat_tools.currval(table_name text, column_name text) ok 2 - Permission denied trying to execute cat_tools.enum_range(enum regtype) ok 3 - Permission denied trying to execute cat_tools.enum_range_srf(enum regtype) @@ -160,4 +160,5 @@ ok 157 - Permission granted to use type cat_tools.routine_type ok 158 - Permission granted to use type cat_tools.routine_volatility ok 159 - no_use_role has no USAGE on _cat_tools schema ok 160 - use_role has USAGE on _cat_tools schema +ok 161 - cat_tools schema(s) must not be part of the resolved search_path -- got {tap,public} # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/pg_depends.out b/test/expected/pg_depends.out index 9a89a9c..244f27e 100644 --- a/test/expected/pg_depends.out +++ b/test/expected/pg_depends.out @@ -1,6 +1,7 @@ \set ECHO none -1..2 +1..3 ok 1 - Verify base data on ok 2 - Verify public has no perms +ok 3 - cat_tools schema(s) must not be part of the resolved search_path -- got {tap,public} # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/relation__.out b/test/expected/relation__.out index ff14a93..ed633d4 100644 --- a/test/expected/relation__.out +++ b/test/expected/relation__.out @@ -1,5 +1,5 @@ \set ECHO none -1..38 +1..39 ok 1 - Verify count from kinds ok 2 - Simple sanity check of relation__kind() ok 3 - Simple sanity check of relation__relkind() @@ -38,4 +38,5 @@ ok 35 - Temp table returns expected column names ok 36 - Drop middle column from temp table ok 37 - Temp table with dropped column returns expected column names ok 38 - NULL input returns NULL (STRICT function) +ok 39 - cat_tools schema(s) must not be part of the resolved search_path -- got {tap,public} # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/routine__.out b/test/expected/routine__.out index e9736f0..cffeaaa 100644 --- a/test/expected/routine__.out +++ b/test/expected/routine__.out @@ -1,5 +1,5 @@ \set ECHO none -1..89 +1..90 ok 1 - Verify type and prokind enums have same size ok 2 - Simple sanity check of routine__type() ok 3 - Simple sanity check of routine__type() with enum @@ -89,4 +89,5 @@ ok 86 - Verify routine__arg_names_text() formatting ok 87 - Verify routine__arg_names_text() with unnamed arguments ok 88 - Verify routine__arg_names_text() with built-in function ok 89 - Verify routine__arg_names_text() with no arguments +ok 90 - cat_tools schema(s) must not be part of the resolved search_path -- got {tap,public} # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/sequence.out b/test/expected/sequence.out index d36acbe..8328f2b 100644 --- a/test/expected/sequence.out +++ b/test/expected/sequence.out @@ -1,5 +1,5 @@ \set ECHO none -1..50 +1..51 ok 1 - Permission denied trying to use cat_tools.currval() ok 2 - Permission denied trying to use cat_tools.setval() ok 3 - Permission denied trying to use cat_tools.nextval() @@ -50,4 +50,5 @@ ok 47 - sequence__next returns correct value ok 48 - INSERT INTO seqtest VALUES(default) ok 49 - currval ok 50 - currval +ok 51 - cat_tools schema(s) must not be part of the resolved search_path -- got {tap,public} # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/trigger.out b/test/expected/trigger.out index d4577ea..5ee9594 100644 --- a/test/expected/trigger.out +++ b/test/expected/trigger.out @@ -1,5 +1,5 @@ \set ECHO none -1..10 +1..11 ok 1 - Verify public has no perms ok 2 - Verify public has no perms ok 3 - Verify public has no perms @@ -10,4 +10,5 @@ ok 7 - SELECT cat_tools.trigger__parse( 1259 ) throws correct error for missing ok 8 - verify results of trigger__parse(oid) ok 9 - verify results of trigger__parse(regclass,text) ok 10 - verify trigger__args_as_text() +ok 11 - cat_tools schema(s) must not be part of the resolved search_path -- got {tap,public} # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/finish.sql b/test/finish.sql new file mode 100644 index 0000000..e0d8fda --- /dev/null +++ b/test/finish.sql @@ -0,0 +1,30 @@ +/* + * Real, counted pgTAP test (not a silent DO block, unlike the check this + * replaces) that cat_tools' own schemas are still absent from the resolved + * search_path at the END of this file -- not just at the start. \i'd by every + * SQL file under test/sql right before finish(), so a future test file + * forgetting to bump its own plan() count for it is caught immediately, a + * natural nudge not to add a search_path-dependent test without thinking + * about it. + * + * Confirmed empirically: each SQL file under test/sql runs as its own psql + * connection, wrapped in a transaction that is never committed (see + * test/pgxntool/finish.sql's "TRANSACTION INTENTIONALLY LEFT OPEN") and so + * always rolls back when that connection closes; a plain SET search_path + * made anywhere in the file is undone by that rollback, so it can never leak + * into a LATER file no matter when we check. What a start-only check (as + * test/setup.sql's used to be) cannot see is a mutation that remains in + * effect for the rest of THIS SAME file's own tests after it happens -- + * checking again here, immediately before that rollback, catches that case. + * Not foolproof: a test that mutates search_path and then restores it before + * this check runs would still slip through. + */ +SELECT ok( + NOT ( 'cat_tools' = ANY (current_schemas(false)) OR '_cat_tools' = ANY (current_schemas(false)) ) + , format( + 'cat_tools schema(s) must not be part of the resolved search_path -- got %s' + , current_schemas(false) + ) +); + +-- vi: expandtab ts=2 sw=2 diff --git a/test/setup.sql b/test/setup.sql index f227585..fe66b06 100644 --- a/test/setup.sql +++ b/test/setup.sql @@ -2,27 +2,14 @@ \i test/pgxntool/setup.sql /* - * Self-check, not a toggle: pgxntool's own tap_setup.sql (just \i'd above) - * sets search_path = tap, public for every test file -- cat_tools' own - * schemas are never on it. That's what makes every pgTAP pass in this suite - * mean cat_tools' internal views/functions fully schema-qualify their own - * cross-references, rather than happening to resolve by search_path - * accident. current_schemas(false) IS Postgres's own unqualified-name - * search list, so confirming cat_tools/_cat_tools are absent from it is a - * direct proof, not a proxy for one. Asserted here (always, unconditionally) - * so a future change to the shared search_path baseline fails loudly instead - * of silently widening what "passing" means. + * pgxntool's own tap_setup.sql (just \i'd above) sets search_path = tap, public + * for every test file -- cat_tools' own schemas are never on it. That's what + * makes every pgTAP pass in this suite mean cat_tools' internal views/functions + * fully schema-qualify their own cross-references, rather than happening to + * resolve by search_path accident. See test/finish.sql, \i'd by every SQL + * file under test/sql, for the actual assertion of this fact -- checked + * there, at the END of each file, rather than here at the start. */ -DO $$ -BEGIN - IF 'cat_tools' = ANY (current_schemas(false)) OR '_cat_tools' = ANY (current_schemas(false)) THEN - RAISE EXCEPTION - 'cat_tools schema(s) must not be part of the resolved search_path during testing -- got %' - , current_schemas(false) - ; - END IF; -END -$$; GRANT USAGE ON SCHEMA tap TO :"use_role", :"no_use_role"; diff --git a/test/sql/attribute.sql b/test/sql/attribute.sql index 6fadecc..cc90a55 100644 --- a/test/sql/attribute.sql +++ b/test/sql/attribute.sql @@ -16,6 +16,7 @@ SELECT plan( + (SELECT count(*)::int FROM func_calls) + 4 -- pg_attribute__get() + + 1 -- search_path still clean (test/finish.sql) ); SET LOCAL ROLE :"no_use_role"; @@ -110,6 +111,7 @@ SELECT results_eq( ); +\i test/finish.sql \i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/enum.sql b/test/sql/enum.sql index 8cc9c0c..2eac2ae 100644 --- a/test/sql/enum.sql +++ b/test/sql/enum.sql @@ -10,6 +10,7 @@ SELECT plan( + 1 + 2 -- no_use tests + 1 + + 1 -- search_path still clean (test/finish.sql) ); SELECT lives_ok( @@ -35,6 +36,7 @@ SELECT results_eq( , $$VALUES ( 'ZZZ Label 1' ), ( 'Label 2' )$$ ); +\i test/finish.sql \i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/extension.sql b/test/sql/extension.sql index 904132e..5cfac09 100644 --- a/test/sql/extension.sql +++ b/test/sql/extension.sql @@ -17,6 +17,8 @@ SELECT plan( + 1 -- extension__schemas_unique + 1 -- extension__schemas + 1 -- extension__schemas_unique with bad schema + + + 1 -- search_path still clean (test/finish.sql) ); SELECT is( @@ -73,6 +75,7 @@ SELECT throws_ok( , 'extension__schemas_unique with bogus extension fails' ); +\i test/finish.sql \i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/function.sql b/test/sql/function.sql index e0b0f59..50da057 100644 --- a/test/sql/function.sql +++ b/test/sql/function.sql @@ -9,6 +9,7 @@ SELECT plan( 2 -- security definer checks for _cat_tools helpers + 1 -- regprocedure() + 4 -- deprecated function__arg_types() wrappers + + 1 -- search_path still clean (test/finish.sql) ); /* @@ -66,6 +67,7 @@ SELECT is( , 'Verify deprecated function__arg_types_text() with simple args' ); +\i test/finish.sql \i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/general.sql b/test/sql/general.sql index 08d6f2d..eb2500a 100644 --- a/test/sql/general.sql +++ b/test/sql/general.sql @@ -5,12 +5,14 @@ SELECT plan( 0 +1 + +1 -- search_path still clean (test/finish.sql) ); SELECT hasnt_schema( '__cat_tools' ); +\i test/finish.sql \i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/name__check.sql b/test/sql/name__check.sql index e65db56..0127626 100644 --- a/test/sql/name__check.sql +++ b/test/sql/name__check.sql @@ -5,7 +5,10 @@ \set s cat_tools \set f name__check -SELECT plan(5); +SELECT plan( + 5 + + 1 -- search_path still clean (test/finish.sql) +); SET LOCAL ROLE :"no_use_role"; @@ -56,6 +59,7 @@ SELECT throws_like( , 'Error on overflow' ); +\i test/finish.sql \i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/object_type.sql b/test/sql/object_type.sql index 822a89a..c14ce99 100644 --- a/test/sql/object_type.sql +++ b/test/sql/object_type.sql @@ -60,6 +60,8 @@ SELECT plan( + 3 * 2 + 1 -- objects__shared + + + 1 -- search_path still clean (test/finish.sql) ); SELECT is( @@ -261,6 +263,7 @@ SELECT bag_eq( , 'Verify objects__shared_src() returns correct values' ); +\i test/finish.sql \i test/pgxntool/finish.sql --select name,setting from pg_settings where name ~ '^lc_'; diff --git a/test/sql/permissions.sql b/test/sql/permissions.sql index d26f592..54f9205 100644 --- a/test/sql/permissions.sql +++ b/test/sql/permissions.sql @@ -30,6 +30,7 @@ SELECT plan( (SELECT count(*)::int FROM cat_types) * 2 -- no_use denied + use allowed + (SELECT count(*)::int FROM cat_functions) * 2 -- no_use denied + use allowed + 2 -- _cat_tools schema access (no_use denied + use allowed) + + 1 -- search_path still clean (test/finish.sql) ); /* @@ -97,6 +98,7 @@ SELECT is( , 'use_role has USAGE on _cat_tools schema' ); +\i test/finish.sql \i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/pg_depends.sql b/test/sql/pg_depends.sql index d279917..11cb335 100644 --- a/test/sql/pg_depends.sql +++ b/test/sql/pg_depends.sql @@ -34,6 +34,7 @@ SELECT plan( --+ (SELECT count(*)::int FROM func_calls) + 1 -- _cat_tools.pg_depend_v + + 1 -- search_path still clean (test/finish.sql) ); /* @@ -89,6 +90,7 @@ SELECT CASE SET LOCAL ROLE :"use_role"; +\i test/finish.sql \i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/relation__.sql b/test/sql/relation__.sql index 3c3e558..8c627f6 100644 --- a/test/sql/relation__.sql +++ b/test/sql/relation__.sql @@ -43,6 +43,7 @@ SELECT plan( + 4 -- relation__is_catalog + 5 -- relation__column_names + 1 -- relkind drift check vs pg_class.h + + 1 -- search_path still clean (test/finish.sql) ); -- relation_type enum mapping @@ -214,6 +215,7 @@ SELECT is( , 'NULL input returns NULL (STRICT function)' ); +\i test/finish.sql \i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/routine__.sql b/test/sql/routine__.sql index 5b67b93..802ab4f 100644 --- a/test/sql/routine__.sql +++ b/test/sql/routine__.sql @@ -25,6 +25,7 @@ SELECT plan( + 1 -- isnt_definer: routine__parse_arg_names_text + 3 -- routine__arg_names() tests + 4 -- routine__arg_names_text() tests + + 1 -- search_path still clean (test/finish.sql) ); \set kind type @@ -287,6 +288,7 @@ SELECT is( , 'Verify routine__arg_names_text() with no arguments' ); +\i test/finish.sql \i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/sequence.sql b/test/sql/sequence.sql index a408b32..824c6fd 100644 --- a/test/sql/sequence.sql +++ b/test/sql/sequence.sql @@ -26,6 +26,7 @@ SELECT plan(( +2 -- next +8 -- set +3 -- currval + +1 -- search_path still clean (test/finish.sql) )::int); GRANT SELECT ON functions TO public; @@ -135,6 +136,7 @@ SELECT is( , 'currval' ); +\i test/finish.sql \i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/trigger.sql b/test/sql/trigger.sql index 76a3f49..28958fb 100644 --- a/test/sql/trigger.sql +++ b/test/sql/trigger.sql @@ -17,6 +17,8 @@ SELECT plan( + 2 -- verify trigger__parse output + 1 -- verify trigger__args_as_array() + + + 1 -- search_path still clean (test/finish.sql) ); SET LOCAL ROLE :"no_use_role"; @@ -149,6 +151,7 @@ SELECT is( , 'verify trigger__args_as_text()' ); +\i test/finish.sql \i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 From 6e7a82acf92626442406ef1af6ec461301d45c3b Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 3 Aug 2026 17:32:49 -0500 Subject: [PATCH 3/5] test/finish.sql: chain to pgxntool/finish.sql internally instead of a second \i Each test/sql/*.sql file now has exactly one finish-side \i (test/finish.sql), mirroring how they already have exactly one setup-side \i (test/setup.sql, which itself \i's test/pgxntool/setup.sql as its first line). test/finish.sql runs the search_path assertion and then \i's test/pgxntool/finish.sql as its last line, rather than every test file calling both directly. Co-Authored-By: Claude Sonnet 5 --- test/finish.sql | 4 ++++ test/sql/attribute.sql | 1 - test/sql/enum.sql | 1 - test/sql/extension.sql | 1 - test/sql/function.sql | 1 - test/sql/general.sql | 1 - test/sql/name__check.sql | 1 - test/sql/object_type.sql | 1 - test/sql/permissions.sql | 1 - test/sql/pg_depends.sql | 1 - test/sql/relation__.sql | 1 - test/sql/routine__.sql | 1 - test/sql/sequence.sql | 1 - test/sql/trigger.sql | 1 - 14 files changed, 4 insertions(+), 13 deletions(-) diff --git a/test/finish.sql b/test/finish.sql index e0d8fda..c70c421 100644 --- a/test/finish.sql +++ b/test/finish.sql @@ -27,4 +27,8 @@ SELECT ok( ) ); +-- Chain through to pgxntool's own finish, same wrapping pattern as test/setup.sql +-- does for test/pgxntool/setup.sql. +\i test/pgxntool/finish.sql + -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/attribute.sql b/test/sql/attribute.sql index cc90a55..06b2239 100644 --- a/test/sql/attribute.sql +++ b/test/sql/attribute.sql @@ -112,6 +112,5 @@ SELECT results_eq( \i test/finish.sql -\i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/enum.sql b/test/sql/enum.sql index 2eac2ae..a78a63e 100644 --- a/test/sql/enum.sql +++ b/test/sql/enum.sql @@ -37,6 +37,5 @@ SELECT results_eq( ); \i test/finish.sql -\i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/extension.sql b/test/sql/extension.sql index 5cfac09..1dd1446 100644 --- a/test/sql/extension.sql +++ b/test/sql/extension.sql @@ -76,6 +76,5 @@ SELECT throws_ok( ); \i test/finish.sql -\i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/function.sql b/test/sql/function.sql index 50da057..e814737 100644 --- a/test/sql/function.sql +++ b/test/sql/function.sql @@ -68,6 +68,5 @@ SELECT is( ); \i test/finish.sql -\i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/general.sql b/test/sql/general.sql index eb2500a..d81060f 100644 --- a/test/sql/general.sql +++ b/test/sql/general.sql @@ -13,6 +13,5 @@ SELECT hasnt_schema( ); \i test/finish.sql -\i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/name__check.sql b/test/sql/name__check.sql index 0127626..f062442 100644 --- a/test/sql/name__check.sql +++ b/test/sql/name__check.sql @@ -60,6 +60,5 @@ SELECT throws_like( ); \i test/finish.sql -\i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/object_type.sql b/test/sql/object_type.sql index c14ce99..ab1aa5d 100644 --- a/test/sql/object_type.sql +++ b/test/sql/object_type.sql @@ -264,7 +264,6 @@ SELECT bag_eq( ); \i test/finish.sql -\i test/pgxntool/finish.sql --select name,setting from pg_settings where name ~ '^lc_'; diff --git a/test/sql/permissions.sql b/test/sql/permissions.sql index 54f9205..aa75a23 100644 --- a/test/sql/permissions.sql +++ b/test/sql/permissions.sql @@ -99,6 +99,5 @@ SELECT is( ); \i test/finish.sql -\i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/pg_depends.sql b/test/sql/pg_depends.sql index 11cb335..8d9ca70 100644 --- a/test/sql/pg_depends.sql +++ b/test/sql/pg_depends.sql @@ -91,6 +91,5 @@ SELECT CASE SET LOCAL ROLE :"use_role"; \i test/finish.sql -\i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/relation__.sql b/test/sql/relation__.sql index 8c627f6..d811762 100644 --- a/test/sql/relation__.sql +++ b/test/sql/relation__.sql @@ -216,6 +216,5 @@ SELECT is( ); \i test/finish.sql -\i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/routine__.sql b/test/sql/routine__.sql index 802ab4f..4933c51 100644 --- a/test/sql/routine__.sql +++ b/test/sql/routine__.sql @@ -289,6 +289,5 @@ SELECT is( ); \i test/finish.sql -\i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/sequence.sql b/test/sql/sequence.sql index 824c6fd..aac96c2 100644 --- a/test/sql/sequence.sql +++ b/test/sql/sequence.sql @@ -137,6 +137,5 @@ SELECT is( ); \i test/finish.sql -\i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 diff --git a/test/sql/trigger.sql b/test/sql/trigger.sql index 28958fb..6b361c5 100644 --- a/test/sql/trigger.sql +++ b/test/sql/trigger.sql @@ -152,6 +152,5 @@ SELECT is( ); \i test/finish.sql -\i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2 From effbfbcb9f9c9106745c8e89b9e96d2b9b65c43b Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 3 Aug 2026 17:47:21 -0500 Subject: [PATCH 4/5] test/finish.sql: cut the comment down to what a maintainer needs The previous comment recounted the empirical investigation that justified this design (psql-connection-per-file, transaction rollback semantics) -- that belongs in the PR description, not as a permanent code comment. Keep only the conclusion: what this asserts, why file-end (not just setup), the one caveat, and the plan()-bump reminder. Co-Authored-By: Claude Sonnet 5 --- test/finish.sql | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/test/finish.sql b/test/finish.sql index c70c421..8ef7f01 100644 --- a/test/finish.sql +++ b/test/finish.sql @@ -1,23 +1,10 @@ /* - * Real, counted pgTAP test (not a silent DO block, unlike the check this - * replaces) that cat_tools' own schemas are still absent from the resolved - * search_path at the END of this file -- not just at the start. \i'd by every - * SQL file under test/sql right before finish(), so a future test file - * forgetting to bump its own plan() count for it is caught immediately, a - * natural nudge not to add a search_path-dependent test without thinking - * about it. - * - * Confirmed empirically: each SQL file under test/sql runs as its own psql - * connection, wrapped in a transaction that is never committed (see - * test/pgxntool/finish.sql's "TRANSACTION INTENTIONALLY LEFT OPEN") and so - * always rolls back when that connection closes; a plain SET search_path - * made anywhere in the file is undone by that rollback, so it can never leak - * into a LATER file no matter when we check. What a start-only check (as - * test/setup.sql's used to be) cannot see is a mutation that remains in - * effect for the rest of THIS SAME file's own tests after it happens -- - * checking again here, immediately before that rollback, catches that case. - * Not foolproof: a test that mutates search_path and then restores it before - * this check runs would still slip through. + * Asserts cat_tools' own schema(s) are absent from the resolved search_path -- + * checked here (file end, before finish()) rather than only at setup, so a + * test that mutates search_path mid-file and never restores it is caught. Not + * foolproof: mutate-then-restore before this line still slips through. \i'd by + * every SQL file under test/sql; bump that file's plan() by one to account + * for it. */ SELECT ok( NOT ( 'cat_tools' = ANY (current_schemas(false)) OR '_cat_tools' = ANY (current_schemas(false)) ) From a38750be73c9bd13dcfa74eb44e096fa3a47e326 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 3 Aug 2026 18:23:09 -0500 Subject: [PATCH 5/5] test/finish.sql: comment cleanup -- block comment style, drop plan()-bump aside Convert the trailing two-line -- comment to a /* */ block comment per CLAUDE.md's Code Style section (never -- for multi-line explanations). Also drop the "bump that file's plan() by one" clause from the main comment -- anyone writing a test already knows to keep plan() in sync with its assertions. Co-Authored-By: Claude Sonnet 5 --- test/finish.sql | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/finish.sql b/test/finish.sql index 8ef7f01..8b85457 100644 --- a/test/finish.sql +++ b/test/finish.sql @@ -3,8 +3,7 @@ * checked here (file end, before finish()) rather than only at setup, so a * test that mutates search_path mid-file and never restores it is caught. Not * foolproof: mutate-then-restore before this line still slips through. \i'd by - * every SQL file under test/sql; bump that file's plan() by one to account - * for it. + * every SQL file under test/sql. */ SELECT ok( NOT ( 'cat_tools' = ANY (current_schemas(false)) OR '_cat_tools' = ANY (current_schemas(false)) ) @@ -14,8 +13,10 @@ SELECT ok( ) ); --- Chain through to pgxntool's own finish, same wrapping pattern as test/setup.sql --- does for test/pgxntool/setup.sql. +/* + * Chain through to pgxntool's own finish, same wrapping pattern as + * test/setup.sql does for test/pgxntool/setup.sql. + */ \i test/pgxntool/finish.sql -- vi: expandtab ts=2 sw=2