While refactoring the Kerberos test module in preparation for adding
libpq encryption negotiation tests [1], I noticed that if the test
script die()s during setup, the whole test is marked as SKIPped rather
than failed. The cleanup END section is missing this trick:
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -203,7 +203,12 @@ system_or_bail $krb5kdc, '-P', $kdc_pidfile;
END
{
+ # take care not to change the script's exit value
+ my $exit_code = $?;
+
kill 'INT', `cat $kdc_pidfile` if defined($kdc_pidfile) && -f
$kdc_pidfile;
+
+ $? = $exit_code;
}
The PostgreSQL::Cluster module got that right, but this test and the
LdapServer module didn't get the memo.
After fixing that, the ldap tests started failing on my laptop:
[12:45:28.997](0.054s) # setting up LDAP server
# Checking port 59839
# Found port 59839
# Checking port 59840
# Found port 59840
# Running: /usr/sbin/slapd -f
/home/heikki/git-sandbox/postgresql/build/testrun/ldap/001_auth/data/ldap-001_auth_j_WZ/slapd.conf
-s0 -h ldap://localhost:59839 ldaps://localhost:59840
Can't exec "/usr/sbin/slapd": No such file or directory at
/home/heikki/git-sandbox/postgresql/src/test/perl/PostgreSQL/Test/Utils.pm
line 349.
[12:45:29.004](0.008s) Bail out! command "/usr/sbin/slapd -f
/home/heikki/git-sandbox/postgresql/build/testrun/ldap/001_auth/data/ldap-001_auth_j_WZ/slapd.conf
-s0 -h ldap://localhost:59839 ldaps://localhost:59840" exited with value 2
That's because I don't have 'slapd' installed. The test script it
supposed to check for that, and mark the test as SKIPped, but it's not
really doing that on Linux. Attached patch fixes that, and also makes
the error message a bit more precise, when the OpenLDAP installation is
not found.
There's a lot more we could do with that code that tries to find the
OpenLDAP installation. It should probably be a configure/meson test.
This patch is just the minimum to keep this working after fixing the END
block.
1st patch fixes the LDAP setup tests, and 2nd patch fixes the error
handling in the END blocks.
[1] https://commitfest.postgresql.org/47/4742/
--
Heikki Linnakangas
Neon (https://neon.tech)
From 6f09361ac0dbac5c2aef59b6a24e92486097cb43 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakan...@iki.fi>
Date: Sun, 7 Apr 2024 13:06:39 +0300
Subject: [PATCH 1/2] Improve check in LDAP test to find the OpenLDAP
installation
If the OpenLDAP installation directory is not found, set $setup to 0
so that the LDAP tests are skipped. The macOS checks were already
doing that, but the checks on other OS's were not. While we're at it,
improve the error message when the tests are skipped, to specify
whether the OS is supported at all, or if we just didn't find the
installation directory.
This was accidentally "working" without this, i.e. we were skipping
the tests if the OpenLDAP installation was not found, because of a bug
in the LdapServer test module: the END block clobbered the exit code
so if the script die()s before running the first subtest, the whole
test script was marked as SKIPped. The next commit will fix that bug,
but we need to fix the setup code first before we can do that.
These checks should probably go into configure/meson in the long run,
but this is better than nothing and allows fixing the bug in the END
block.
---
src/test/ldap/LdapServer.pm | 91 ++++++++++++++++++++++---------
src/test/ldap/t/001_auth.pl | 3 +-
src/test/ldap/t/002_bindpasswd.pl | 3 +-
3 files changed, 67 insertions(+), 30 deletions(-)
diff --git a/src/test/ldap/LdapServer.pm b/src/test/ldap/LdapServer.pm
index d2a043bc8f..91cd9e3762 100644
--- a/src/test/ldap/LdapServer.pm
+++ b/src/test/ldap/LdapServer.pm
@@ -57,49 +57,88 @@ use File::Basename;
# private variables
my ($slapd, $ldap_schema_dir, @servers);
-# visible variable
-our ($setup);
+# visible variables
+our ($setup, $setup_error);
INIT
{
+ # Find the OpenLDAP server binary and directory containing schema
+ # definition files. On success, $setup is set to 1. On failure,
+ # it's set to 0, and an error message is set in $setup_error.
$setup = 1;
- if ($^O eq 'darwin' && -d '/opt/homebrew/opt/openldap')
+ if ($^O eq 'darwin')
{
- # typical paths for Homebrew on ARM
- $slapd = '/opt/homebrew/opt/openldap/libexec/slapd';
- $ldap_schema_dir = '/opt/homebrew/etc/openldap/schema';
- }
- elsif ($^O eq 'darwin' && -d '/usr/local/opt/openldap')
- {
- # typical paths for Homebrew on Intel
- $slapd = '/usr/local/opt/openldap/libexec/slapd';
- $ldap_schema_dir = '/usr/local/etc/openldap/schema';
- }
- elsif ($^O eq 'darwin' && -d '/opt/local/etc/openldap')
- {
- # typical paths for MacPorts
- $slapd = '/opt/local/libexec/slapd';
- $ldap_schema_dir = '/opt/local/etc/openldap/schema';
+ if (-d '/opt/homebrew/opt/openldap')
+ {
+ # typical paths for Homebrew on ARM
+ $slapd = '/opt/homebrew/opt/openldap/libexec/slapd';
+ $ldap_schema_dir = '/opt/homebrew/etc/openldap/schema';
+ }
+ elsif (-d '/usr/local/opt/openldap')
+ {
+ # typical paths for Homebrew on Intel
+ $slapd = '/usr/local/opt/openldap/libexec/slapd';
+ $ldap_schema_dir = '/usr/local/etc/openldap/schema';
+ }
+ elsif (-d '/opt/local/etc/openldap')
+ {
+ # typical paths for MacPorts
+ $slapd = '/opt/local/libexec/slapd';
+ $ldap_schema_dir = '/opt/local/etc/openldap/schema';
+ }
+ else
+ {
+ $setup_error = "OpenLDAP server installation not found";
+ $setup = 0;
+ }
}
elsif ($^O eq 'linux')
{
- $slapd = '/usr/sbin/slapd';
- $ldap_schema_dir = '/etc/ldap/schema' if -d '/etc/ldap/schema';
- $ldap_schema_dir = '/etc/openldap/schema'
- if -d '/etc/openldap/schema';
+ if (-d '/etc/ldap/schema')
+ {
+ $slapd = '/usr/sbin/slapd';
+ $ldap_schema_dir = '/etc/ldap/schema';
+ }
+ elsif (-d '/etc/openldap/schema')
+ {
+ $slapd = '/usr/sbin/slapd';
+ $ldap_schema_dir = '/etc/openldap/schema';
+ }
+ else
+ {
+ $setup_error = "OpenLDAP server installation not found";
+ $setup = 0;
+ }
}
elsif ($^O eq 'freebsd')
{
- $slapd = '/usr/local/libexec/slapd';
- $ldap_schema_dir = '/usr/local/etc/openldap/schema';
+ if (-d '/usr/local/etc/openldap/schema')
+ {
+ $slapd = '/usr/local/libexec/slapd';
+ $ldap_schema_dir = '/usr/local/etc/openldap/schema';
+ }
+ else
+ {
+ $setup_error = "OpenLDAP server installation not found";
+ $setup = 0;
+ }
}
elsif ($^O eq 'openbsd')
{
- $slapd = '/usr/local/libexec/slapd';
- $ldap_schema_dir = '/usr/local/share/examples/openldap/schema';
+ if (-d '/usr/local/share/examples/openldap/schema')
+ {
+ $slapd = '/usr/local/libexec/slapd';
+ $ldap_schema_dir = '/usr/local/share/examples/openldap/schema';
+ }
+ else
+ {
+ $setup_error = "OpenLDAP server installation not found";
+ $setup = 0;
+ }
}
else
{
+ $setup_error = "ldap tests not supported on $^O";
$setup = 0;
}
}
diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl
index 053e322e72..850db34503 100644
--- a/src/test/ldap/t/001_auth.pl
+++ b/src/test/ldap/t/001_auth.pl
@@ -25,8 +25,7 @@ elsif (!$ENV{PG_TEST_EXTRA} || $ENV{PG_TEST_EXTRA} !~ /\bldap\b/)
}
elsif (!$LdapServer::setup)
{
- plan skip_all =>
- "ldap tests not supported on $^O or dependencies not installed";
+ plan skip_all => $LdapServer::setup_error;
}
note "setting up LDAP server";
diff --git a/src/test/ldap/t/002_bindpasswd.pl b/src/test/ldap/t/002_bindpasswd.pl
index c8ec8cf889..a89e363546 100644
--- a/src/test/ldap/t/002_bindpasswd.pl
+++ b/src/test/ldap/t/002_bindpasswd.pl
@@ -25,8 +25,7 @@ elsif (!$ENV{PG_TEST_EXTRA} || $ENV{PG_TEST_EXTRA} !~ /\bldap\b/)
}
elsif (!$LdapServer::setup)
{
- plan skip_all =>
- "ldap tests not supported on $^O or dependencies not installed";
+ plan skip_all => $LdapServer::setup_error;
}
note "setting up LDAP server";
--
2.39.2
From d73e4144d23bad6228d33d1da211a1283f12e080 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakan...@iki.fi>
Date: Sun, 7 Apr 2024 13:17:18 +0300
Subject: [PATCH 2/2] Don't clobber test exit code at cleanup
If the test script die()d before running the first test, the whole test
was interpreted as SKIPped rather than failed. The PostgreSQL::Cluster
module got this right.
---
src/test/kerberos/t/001_auth.pl | 5 +++++
src/test/ldap/LdapServer.pm | 5 +++++
2 files changed, 10 insertions(+)
diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl
index e51e87d0a2..ec311bfed8 100644
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -203,7 +203,12 @@ system_or_bail $krb5kdc, '-P', $kdc_pidfile;
END
{
+ # take care not to change the script's exit value
+ my $exit_code = $?;
+
kill 'INT', `cat $kdc_pidfile` if defined($kdc_pidfile) && -f $kdc_pidfile;
+
+ $? = $exit_code;
}
note "setting up PostgreSQL instance";
diff --git a/src/test/ldap/LdapServer.pm b/src/test/ldap/LdapServer.pm
index 91cd9e3762..3866a56e0a 100644
--- a/src/test/ldap/LdapServer.pm
+++ b/src/test/ldap/LdapServer.pm
@@ -145,6 +145,9 @@ INIT
END
{
+ # take care not to change the script's exit value
+ my $exit_code = $?;
+
foreach my $server (@servers)
{
next unless -f $server->{pidfile};
@@ -152,6 +155,8 @@ END
chomp $pid;
kill 'INT', $pid;
}
+
+ $? = $exit_code;
}
=pod
--
2.39.2
