Pick the test/install load mode server-side, and turn on ON_ERROR_STOP

bin/test_existing

@@ -183,9 +183,11 @@ update_ext() {
 
 # CREATE EXTENSION count_nulls at VERSION, into a freshly, randomly
 # generated schema - shared with test/install/load.sql's own fresh/update
-# installs via test/helpers/create_test_schema.sql (see that file for the
-# full rationale). This invocation sets no count_nulls.test_load_mode GUC,
-# so count_nulls_load_mode is supplied here instead: it's a genuine fresh
+# installs via test/helpers/extension_installer.sql (see that file for the
+# full rationale). create_test_schema.sql is a thin wrapper that calls it,
+# and is a file only so this script can invoke it standalone. This
+# invocation sets no count_nulls.test_load_mode GUC, so
+# count_nulls_load_mode is supplied here instead: it's a genuine fresh
 # install (prepare-old), never 'existing'.
 create_extension_in_schema() {
   local db=$1 version=$2

test/README.md

@@ -6,15 +6,23 @@ Unusually for a pgTAP suite, assertions aren't written per test file:
 
 ## Layout
 
+A helper named for an action performs it when `\i`'d; one named for a thing
+just defines it, leaving the caller to decide when to use it.
+
 - `install/load.sql` — installs count_nulls once, committed, before the
-  `test/sql/` schedule. Its output isn't tracked; it fails loudly instead.
+  `test/sql/` schedule, per `TEST_LOAD_SOURCE`. Its output isn't tracked; it
+  fails loudly instead.
 - `deps.sql` — per-test-session setup; drops the session to the test user.
 - `core/functions.sql` — `ncs()`, plus the shared `test__*` library.
 - `sql/extension_tests.sql` — adds `test__check_ncs` and
   `test__shutdown__drop_all`, then calls `runtests()`.
-- `helpers/use_test_user.sql` — switches the session to the non-superuser role.
-- `helpers/create_test_schema.sql` — installs count_nulls at `:version` into
-  a fresh, randomly named schema.
+- `helpers/use_test_user.sql` — switches the session to the non-superuser
+  role.
+- `helpers/extension_installer.sql` — defines the functions that clean up
+  leftover test schemas and install count_nulls at a given version into a
+  fresh, randomly named one.
+- `helpers/create_test_schema.sql` — calls both, with `:version`; a file
+  only because `bin/test_existing`'s `prepare-old` runs it standalone.
 - `helpers/find_test_schema.sql` — finds that schema again, from a session
   that didn't create it.
 - `../bin/compare_fresh_vs_update` — not part of this suite: diffs a fresh
@@ -40,6 +48,10 @@ genuinely correct. `test__check_ncs` asserts it. Two consequences:
   overriding pgTAP's own (which embeds the schema). That's what keeps
   `expected/extension_tests.out` a single file valid for every run.
 
+Neither `install/load.sql` nor `helpers/use_test_user.sql` may use `\if`:
+it's psql 10, CI covers back to 9.4, and under `ON_ERROR_STOP` psql aborts on
+it. Both branch server-side instead.
+
 ## Regenerating expected output
 
 Never hand-edit `expected/`. Use `make results`, guarded by `make

test/helpers/create_test_schema.sql

@@ -1,71 +1,32 @@
 /*
- * Creates a fresh, randomly named schema and installs count_nulls into it.
- * Shared by test/install/load.sql (fresh/update modes, same psql session)
- * and bin/test_existing's prepare-old (a separate invocation) - the
- * creation logic is identical in both, so it lives here once.
- */
-SET count_nulls.test_schema_version = :'version';
-
-/*
- * Bridged through a GUC instead of referencing :'version' directly inside
- * the DO block: psql doesn't interpolate variables inside dollar-quoted
- * strings.
+ * Installs count_nulls at :version into a fresh, randomly named schema.
+ * Exists as a file only because bin/test_existing's prepare-old runs it
+ * standalone (psql -v version=<VERSION> -f); the work itself lives in
+ * test/helpers/extension_installer.sql.
  *
- * 'current' means install whatever the current default is, matching the
- * sentinel bin/test_existing already uses; empty is a hard error so an
- * unpropagated :version can't silently install 'current' instead.
+ * The install itself runs as a non-superuser (see
+ * test/helpers/use_test_user.sql), which is what proves count_nulls doesn't
+ * need superuser to install. Cleanup happens before that switch: a leftover
+ * schema can belong to any role, and only the connecting one is sure to be
+ * able to drop it.
  */
-DO $$
-BEGIN
-  IF current_setting('count_nulls.test_schema_version') = '' THEN
-    RAISE EXCEPTION $msg$:version must be set explicitly, or 'current'$msg$;
-  END IF;
-END
-$$;
+\i test/helpers/extension_installer.sql
 
 /*
- * A run that crashed before its own teardown leaves a schema nothing else
- * knows the name of, so match the prefix (see the name generation below)
- * and drop it here - before the test-user switch, since a leftover schema
- * can belong to any role and only the connecting one is sure to be able to
- * drop it. See find_test_schema.sql for how later sessions rediscover the
- * name this creates.
+ * Always a fresh install here, never 'existing' - see the mode's no-op note
+ * on count_nulls_cleanup_test_schemas() itself.
  */
-DO $$
-DECLARE
-  r record;
-BEGIN
-  FOR r IN SELECT nspname FROM pg_namespace WHERE nspname LIKE 'count_nulls test schema %' LOOP
-    EXECUTE format('DROP SCHEMA %I CASCADE', r.nspname);
-  END LOOP;
-END
-$$;
+SELECT pg_temp.count_nulls_cleanup_test_schemas('fresh');
 
 /*
- * :count_nulls_load_mode must already be set by the caller (test/install/
- * load.sql, or bin/test_existing's -v on the command line) - this file
- * installs count_nulls, so it can't know on its own whether that's
- * genuinely a fresh/update run rather than 'existing'.
+ * :count_nulls_load_mode must already be set by the caller
+ * (bin/test_existing's -v on the command line - see the file header) -
+ * this file only installs, so it can't know on its own whether that's
+ * genuinely a fresh run rather than 'existing'.
  */
 \i test/helpers/use_test_user.sql
 
-/*
- * Trailing space alone forces identifier quoting, so every run exercises
- * %I-qualification rather than passing by luck of the random suffix. Must
- * match the prefix cleanup matches on above.
- */
-SELECT 'count_nulls test schema ' || substr(md5(random()::text), 1, 12) AS schema
-\gset
-
-CREATE SCHEMA :"schema";
-
-SELECT CASE WHEN :'version' = 'current' THEN '' ELSE format(' VERSION %L', :'version') END AS version_clause
+SELECT pg_temp.count_nulls_install_extension(:'version') AS count_nulls_test_schema
 \gset
 
-/*
- * WITH SCHEMA rather than arranging search_path first: this way a
- * successful install proves the install script doesn't depend on
- * unqualified name resolution, instead of hiding it behind a search_path
- * that happened to suit.
- */
-CREATE EXTENSION count_nulls WITH SCHEMA :"schema":version_clause;
+-- vi: expandtab sw=2 ts=2

test/helpers/extension_installer.sql

@@ -0,0 +1,78 @@
+/*
+ * Defines, but does not call, the functions that find-and-drop leftover test
+ * schemas and install count_nulls into a fresh one. Split from calling them,
+ * and from each other, so test/install/load.sql can decide server-side
+ * whether to install at all, and so every caller can run cleanup before
+ * switching to the test user (test/helpers/use_test_user.sql) while
+ * installing after - a leftover schema can belong to any role, and only the
+ * connecting one is sure to be able to drop it.
+ */
+CREATE OR REPLACE FUNCTION pg_temp.count_nulls_cleanup_test_schemas(
+  p_mode text
+) RETURNS void LANGUAGE plpgsql AS $body$
+DECLARE
+  r record;
+BEGIN
+  /*
+   * 'existing' asserts against a schema a prior, separate prepare-old run
+   * left behind on purpose - dropping it here would destroy the very thing
+   * pg_upgrade is being tested against, so this mode must be a no-op.
+   */
+  IF p_mode = 'existing' THEN
+    RETURN;
+  END IF;
+
+  /*
+   * A run that died before its own teardown leaves a schema nothing else
+   * knows the name of, so match the prefix (see
+   * count_nulls_install_extension()'s c_prefix below) and drop whatever's
+   * there.
+   */
+  FOR r IN SELECT nspname FROM pg_namespace WHERE nspname LIKE 'count_nulls test schema %' LOOP
+    EXECUTE format('DROP SCHEMA %I CASCADE', r.nspname);
+  END LOOP;
+END
+$body$;
+
+CREATE OR REPLACE FUNCTION pg_temp.count_nulls_install_extension(
+  p_version text
+) RETURNS name LANGUAGE plpgsql AS $body$
+DECLARE
+  /*
+   * The trailing space alone forces SQL identifier quoting, so every run
+   * exercises the suite's %I-qualification rather than passing by luck of
+   * which characters the random suffix drew. Must match the literal
+   * count_nulls_cleanup_test_schemas() matches on above.
+   */
+  c_prefix CONSTANT text := 'count_nulls test schema ';
+  v_schema name;
+BEGIN
+  /*
+   * 'current' means whatever the control file's default_version is, matching
+   * the sentinel bin/test_existing already uses. Empty is a hard error, so an
+   * unpropagated :version can't silently install 'current' instead.
+   */
+  IF p_version = '' THEN
+    RAISE EXCEPTION $$p_version must be set explicitly, or 'current'$$;
+  END IF;
+
+  v_schema := c_prefix || substr(md5(random()::text), 1, 12);
+  EXECUTE format('CREATE SCHEMA %I', v_schema);
+
+  /*
+   * WITH SCHEMA rather than arranging search_path first: this way a
+   * successful install proves the install script doesn't depend on
+   * unqualified name resolution, instead of hiding it behind a search_path
+   * that happened to suit.
+   */
+  EXECUTE format(
+    'CREATE EXTENSION count_nulls WITH SCHEMA %I%s'
+    , v_schema
+    , CASE WHEN p_version = 'current' THEN '' ELSE format(' VERSION %L', p_version) END
+  );
+
+  RETURN v_schema;
+END
+$body$;
+
+-- vi: expandtab sw=2 ts=2

test/helpers/use_test_user.sql

@@ -7,15 +7,13 @@
  * default superuser = true.
  *
  * Included from every entry point that starts a session the suite runs in:
- * test/deps.sql (each test/sql/ session, via pgxntool's setup.sql) and
- * test/helpers/create_test_schema.sql (the install session, and
- * bin/test_existing's prepare-old).
+ * test/deps.sql (each test/sql/ session, via pgxntool's setup.sql),
+ * test/install/load.sql (the install session) and
+ * test/helpers/create_test_schema.sql (bin/test_existing's prepare-old).
  *
- * The RESET ROLE below is what makes a second \i of this file in one
- * session behave exactly like the first: it hands the privileges back
- * before anything that needs them. That isn't hypothetical - psql older
- * than 10 has no \if, so test/install/load.sql's mode branches all run and
- * create_test_schema.sql gets included twice.
+ * RESET ROLE first, so what this file does depends only on how the session
+ * connected and not on anything an earlier \i of it already did - it needs
+ * the connecting role's privileges back before it can grant anything.
  */
 RESET ROLE;
 
@@ -102,6 +100,10 @@ BEGIN
   END IF;
 
   IF c_admin THEN
+    /*
+     * Check-then-create, not atomic - safe only because test/install/load.sql
+     * finishes before pg_regress starts the concurrent test/sql/ sessions.
+     */
     IF NOT EXISTS(SELECT 1 FROM pg_roles WHERE rolname = p_test_user) THEN
       EXECUTE format('CREATE ROLE %I', p_test_user);
     END IF;