Subject: Re: Test 041_checkpoint_at_promote.pl faild in installcheck due to missing injection_points

From: Tom Lane <[email-masked]>
Date: Fri, 23 Aug 2024 18:45:02 -0400

Alvaro Herrera <[email-masked]> writes:
> On 2024-Aug-23, Michael Paquier wrote:
>>> I've been curious about what exactly does this Makefile line
>>> export enable_injection_points enable_injection_points
>>> achieve.

>> Without this line, the TAP tests would not be able to know if
>> injection points are enabled or not, no?

> Right, I figured out afterwards that what that does is export the
> make-level variable as an environment variable.

It exports it twice, though, which is pretty confusing.

			regards, tom lane

---

From: Maxim Orlov <[email-masked]>
Date: Wed, 28 Aug 2024 09:14:35 +0300

By the way, we have the same kind of "problem" with the meson build. As we
are deliberately
not want to install src/test/modules, after b6a0d469cae and 0d237aeebaee we
must add step
"meson compile install-test-files" in order to "meson test -q --setup
running" to be successful.

To be honest, this step is not obvious. Especially than there was no such
step before. But
docs and https://wiki.postgresql.org/wiki/Meson are completely silenced
about it.

-- 
Best regards,
Maxim Orlov.

---

From: Michael Paquier <[email-masked]>
Date: Mon, 2 Sep 2024 09:53:30 +0900

> It exports it twice, though, which is pretty confusing.

Right.  I am not sure what was my state of mind back then, but this
pattern has spread a bit.

REL_17_STABLE is frozen for a few more days, so I'll address all the
items of this thread that once the release of this week is tagged: the
export duplicates and the installcheck issue.  These are staged on a
local branch for now.
--
Michael

---

From: Michael Paquier <[email-masked]>
Date: Wed, 4 Sep 2024 09:40:01 +0900

> export duplicates and the installcheck issue.  These are staged on a

There are two TAP tests in REL_17_STABLE that use the module
injection_points, and we have 6 of them on HEAD.  For now, I have
applied a patch that addresses the problems for v17 to avoid any
problems with the upcoming release, fixing the two tests that exist in
REL_17_STABLE.

For HEAD, I'd like to be slightly more ambitious and propose a routine
in Cluster.pm that scans for available extensions, as of the attached.
This simplifies the injection point test in libpq, as the
injection_point is one portion of the test so we don't need the check
based on the environment variable.  There is no need for checks in the
TAP tests injection_points's 001_stats.pl and test_slru's
001_multixact.pl as these modules disable installcheck.

Any thoughts about the attached?  This makes installcheck work here
with and without the configure switch.
--
Michael
From 74deadf240a521ef774dbb540f411e2bccb5b421 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Wed, 4 Sep 2024 09:38:36 +0900
Subject: [PATCH] Check availability of module injection_points in TAP tests

This fixes various defects with installcheck for TAP tests that expect
the module to exist in an installation, but it is not installed by
default with installcheck.

The check is refactored as a new routine in Cluster.pm.
---
 src/interfaces/libpq/Makefile                 |  2 +-
 src/interfaces/libpq/meson.build              |  1 -
 .../libpq/t/005_negotiate_encryption.pl       |  7 ++++--
 src/test/modules/test_misc/t/005_timeouts.pl  |  5 +----
 .../test_misc/t/006_signal_autovacuum.pl      |  9 ++++++++
 src/test/perl/PostgreSQL/Test/Cluster.pm      | 22 +++++++++++++++++++
 .../recovery/t/041_checkpoint_at_promote.pl   |  5 +----
 7 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 27f8499d8a..c1bf33dbdc 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -15,7 +15,7 @@ subdir = src/interfaces/libpq
 top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global
 
-export with_ssl with_gssapi with_krb_srvnam enable_injection_points
+export with_ssl with_gssapi with_krb_srvnam
 
 PGFILEDESC = "PostgreSQL Access Library"
 
diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build
index 7623aeadab..ed2a4048d1 100644
--- a/src/interfaces/libpq/meson.build
+++ b/src/interfaces/libpq/meson.build
@@ -121,7 +121,6 @@ tests += {
       't/005_negotiate_encryption.pl',
     ],
     'env': {
-      'enable_injection_points': get_option('injection_points') ? 'yes' : 'no',
       'with_ssl': ssl_library,
       'with_gssapi': gssapi.found() ? 'yes' : 'no',
       'with_krb_srvnam': 'postgres',
diff --git a/src/interfaces/libpq/t/005_negotiate_encryption.pl b/src/interfaces/libpq/t/005_negotiate_encryption.pl
index 73f0056f10..06d67de2db 100644
--- a/src/interfaces/libpq/t/005_negotiate_encryption.pl
+++ b/src/interfaces/libpq/t/005_negotiate_encryption.pl
@@ -90,8 +90,6 @@ my $kerberos_enabled =
   $ENV{PG_TEST_EXTRA} && $ENV{PG_TEST_EXTRA} =~ /\bkerberos\b/;
 my $ssl_supported = $ENV{with_ssl} eq 'openssl';
 
-my $injection_points_supported = $ENV{enable_injection_points} eq 'yes';
-
 ###
 ### Prepare test server for GSSAPI and SSL authentication, with a few
 ### different test users and helper functions. We don't actually
@@ -151,6 +149,11 @@ if ($ssl_supported != 0)
 
 $node->start;
 
+# Check if the extension injection_points is available, as it may be
+# possible that this script is run with installcheck, where the module
+# would not be installed by default.
+my $injection_points_supported = $node->check_extension('injection_points');
+
 $node->safe_psql('postgres', 'CREATE USER localuser;');
 $node->safe_psql('postgres', 'CREATE USER testuser;');
 $node->safe_psql('postgres', 'CREATE USER ssluser;');
diff --git a/src/test/modules/test_misc/t/005_timeouts.pl b/src/test/modules/test_misc/t/005_timeouts.pl
index 53e44016e3..d9b7219121 100644
--- a/src/test/modules/test_misc/t/005_timeouts.pl
+++ b/src/test/modules/test_misc/t/005_timeouts.pl
@@ -28,10 +28,7 @@ $node->start;
 # Check if the extension injection_points is available, as it may be
 # possible that this script is run with installcheck, where the module
 # would not be installed by default.
-my $result = $node->safe_psql('postgres',
-	"SELECT count(*) > 0 FROM pg_available_extensions WHERE name = 'injection_points';"
-);
-if ($result eq 'f')
+if (!$node->check_extension('injection_points'))
 {
 	plan skip_all => 'Extension injection_points not installed';
 }
diff --git a/src/test/modules/test_misc/t/006_signal_autovacuum.pl b/src/test/modules/test_misc/t/006_signal_autovacuum.pl
index 929253f754..aaea569c10 100644
--- a/src/test/modules/test_misc/t/006_signal_autovacuum.pl
+++ b/src/test/modules/test_misc/t/006_signal_autovacuum.pl
@@ -25,6 +25,15 @@ $node->init;
 # This ensures a quick worker spawn.
 $node->append_conf('postgresql.conf', 'autovacuum_naptime = 1');
 $node->start;
+
+# Check if the extension injection_points is available, as it may be
+# possible that this script is run with installcheck, where the module
+# would not be installed by default.
+if (!$node->check_extension('injection_points'))
+{
+	plan skip_all => 'Extension injection_points not installed';
+}
+
 $node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
 
 $node->safe_psql(
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index fe6ebf10f7..143dc8c101 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2837,6 +2837,28 @@ sub lsn
 
 =pod
 
+=item $node->check_extension(extension_name)
+
+Scan pg_available_extensions to check that an extension is available in an
+installation.
+
+Returns 1 if the extension is available, 0 otherwise.
+
+=cut
+
+sub check_extension
+{
+	my ($self, $extension_name) = @_;
+
+	my $result = $self->safe_psql('postgres',
+		"SELECT count(*) > 0 FROM pg_available_extensions WHERE name = '$extension_name';"
+	);
+
+	return $result eq 't' ? 1 : 0;
+}
+
+=pod
+
 =item $node->wait_for_event(wait_event_name, backend_type)
 
 Poll pg_stat_activity until backend_type reaches wait_event_name.
diff --git a/src/test/recovery/t/041_checkpoint_at_promote.pl b/src/test/recovery/t/041_checkpoint_at_promote.pl
index 905662353d..3c21d18e3a 100644
--- a/src/test/recovery/t/041_checkpoint_at_promote.pl
+++ b/src/test/recovery/t/041_checkpoint_at_promote.pl
@@ -38,10 +38,7 @@ $node_primary->start;
 # Check if the extension injection_points is available, as it may be
 # possible that this script is run with installcheck, where the module
 # would not be installed by default.
-my $result = $node_primary->safe_psql('postgres',
-	"SELECT count(*) > 0 FROM pg_available_extensions WHERE name = 'injection_points';"
-);
-if ($result eq 'f')
+if (!$node_primary->check_extension('injection_points'))
 {
 	plan skip_all => 'Extension injection_points not installed';
 }
-- 
2.45.2