On Sat, Mar 26, 2022 at 6:52 AM Robert Haas <robertmh...@gmail.com> wrote:
> I don't think that the Windows CI is running the TAP tests for
> contrib. At least, I can't find any indication of it in the output. So
> it doesn't really help to assess how portable this test is, unless I'm
> missing something.

Yeah :-(  vcregress.pl doesn't yet have an easy way to run around and
find contrib modules with tap tests and run them, for the CI script to
call.  (I think there was a patch somewhere?  I've been bitten by the
lack of this recently...)

In case it's helpful, here's how to run a specific contrib module's
TAP test by explicitly adding it.  That'll run once I post this email,
but I already ran in it my own github account and it looks like this:

https://cirrus-ci.com/task/5637156969381888
https://api.cirrus-ci.com/v1/artifact/task/5637156969381888/log/contrib/basebackup_to_shell/tmp_check/log/regress_log_001_basic
https://api.cirrus-ci.com/v1/artifact/task/5637156969381888/log/contrib/basebackup_to_shell/tmp_check/log/001_basic_primary.log
From 989258eb413978f342a67e6e3ddd27fecd5dd3d4 Mon Sep 17 00:00:00 2001
From: Robert Haas <rh...@postgresql.org>
Date: Fri, 25 Mar 2022 12:19:44 -0400
Subject: [PATCH 1/2] Tests.

---
 contrib/basebackup_to_shell/Makefile       |   4 +
 contrib/basebackup_to_shell/t/001_basic.pl | 110 +++++++++++++++++++++
 2 files changed, 114 insertions(+)
 create mode 100644 contrib/basebackup_to_shell/t/001_basic.pl

diff --git a/contrib/basebackup_to_shell/Makefile b/contrib/basebackup_to_shell/Makefile
index f31dfaae9c..bbfebcc9a1 100644
--- a/contrib/basebackup_to_shell/Makefile
+++ b/contrib/basebackup_to_shell/Makefile
@@ -7,6 +7,10 @@ OBJS = \
 
 PGFILEDESC = "basebackup_to_shell - target basebackup to shell command"
 
+TAP_TESTS = 1
+
+export TAR
+
 ifdef USE_PGXS
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
diff --git a/contrib/basebackup_to_shell/t/001_basic.pl b/contrib/basebackup_to_shell/t/001_basic.pl
new file mode 100644
index 0000000000..a152fcbc1e
--- /dev/null
+++ b/contrib/basebackup_to_shell/t/001_basic.pl
@@ -0,0 +1,110 @@
+# Copyright (c) 2021-2022, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('primary');
+$node->init('allows_streaming' => 1);
+$node->append_conf('postgresql.conf',
+				   "shared_preload_libraries = 'basebackup_to_shell'");
+$node->start;
+$node->safe_psql('postgres', 'CREATE USER backupuser REPLICATION');
+$node->safe_psql('postgres', 'CREATE ROLE trustworthy');
+
+# For nearly all pg_basebackup invocations some options should be specified,
+# to keep test times reasonable. Using @pg_basebackup_defs as the first
+# element of the array passed to IPC::Run interpolate the array (as it is
+# not a reference to an array)...
+my @pg_basebackup_defs = ('pg_basebackup', '--no-sync', '-cfast');
+
+# This particular test module generally wants to run with -Xfetch, because
+# -Xstream is not supported with a backup target, and with -U backupuser.
+my @pg_basebackup_cmd = (@pg_basebackup_defs, '-U', 'backupuser', '-Xfetch');
+
+# Can't use this module without setting basebackup_to_shell.command.
+$node->command_fails_like(
+    [ @pg_basebackup_cmd, '--target', 'shell' ],
+	qr/shell command for backup is not configured/,
+	'fails if basebackup_to_shell.command is not set');
+
+# Configure basebackup_to_shell.command and reload the configuation file.
+my $backup_path = PostgreSQL::Test::Utils::tempdir;
+my $shell_command =
+	$PostgreSQL::Test::Utils::windows_os
+	? qq{type con > "$backup_path\\\\%f"} : qq{cat > "$backup_path/%f"};
+$node->append_conf('postgresql.conf',
+				   "basebackup_to_shell.command='$shell_command'");
+$node->reload();
+
+# Should work now.
+$node->command_ok(
+    [ @pg_basebackup_cmd, '--target', 'shell' ],
+	'backup with no detail: pg_basebackup');
+verify_backup('', $backup_path, "backup with no detail");
+
+# Should fail with a detail.
+$node->command_fails_like(
+    [ @pg_basebackup_cmd, '--target', 'shell:foo' ],
+	qr/a target detail is not permitted because the configured command does not include %d/,
+	'fails if detail provided without %d');
+
+# Reconfigure to restrict access and require a detail.
+$shell_command =
+	$PostgreSQL::Test::Utils::windows_os
+	? qq{type con > "$backup_path\\\\%d.%f"} : qq{cat > "$backup_path/%d.%f"};
+$node->append_conf('postgresql.conf',
+				   "basebackup_to_shell.command='$shell_command'");
+$node->append_conf('postgresql.conf',
+				   "basebackup_to_shell.required_role='trustworthy'");
+$node->reload();
+
+# Should fail due to lack of permission.
+$node->command_fails_like(
+    [ @pg_basebackup_cmd, '--target', 'shell' ],
+	qr/permission denied to use basebackup_to_shell/,
+	'fails if required_role not granted');
+
+# Should fail due to lack of a detail.
+$node->safe_psql('postgres', 'GRANT trustworthy TO backupuser');
+$node->command_fails_like(
+    [ @pg_basebackup_cmd, '--target', 'shell' ],
+	qr/a target detail is required because the configured command includes %d/,
+	'fails if %d is present and detail not given');
+
+# Should work.
+$node->command_ok(
+    [ @pg_basebackup_cmd, '--target', 'shell:bar' ],
+	'backup with detail: pg_basebackup');
+verify_backup('bar.', $backup_path, "backup with detail");
+
+done_testing();
+
+sub verify_backup
+{
+	my ($prefix, $backup_dir, $test_name) = @_;
+
+	ok(-f "$backup_dir/${prefix}backup_manifest",
+	   "$test_name: backup_manifest was created");
+	ok(-f "$backup_dir/${prefix}base.tar",
+	   "$test_name: base.tar was created");
+
+	SKIP: {
+		my $tar = $ENV{TAR};
+		skip "no tar program available", 1 if (!defined $tar || $tar eq '');
+
+		# Untar.
+		my $extract_path = PostgreSQL::Test::Utils::tempdir;
+		system_or_bail($tar, 'xf', $backup_dir . '/' . $prefix . 'base.tar',
+					   '-C', $extract_path);
+
+		# Verify.
+		$node->command_ok([ 'pg_verifybackup', '-n',
+						  '-m', "${backup_dir}/${prefix}backup_manifest",
+						  '-e', $extract_path ],
+						  "$test_name: backup verifies ok");
+	}
+}
-- 
2.30.2

From cc3c9af5c3868a5ce783338290c5cd4e56880a46 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Sat, 26 Mar 2022 09:35:06 +1300
Subject: [PATCH 2/2] HACK: run tap tests in contrib/basebackup_to_shell

---
 .cirrus.yml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.cirrus.yml b/.cirrus.yml
index 171bd29cf0..edbb8b2966 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -412,6 +412,8 @@ task:
 
   test_regress_parallel_script: |
     %T_C% perl src/tools/msvc/vcregress.pl check parallel
+  test_contrib_basebackup_to_shell_script: |
+    %T_C% perl src/tools/msvc/vcregress.pl taptest ./contrib/basebackup_to_shell
   startcreate_script: |
     rem paths to binaries need backslashes
     tmp_install\bin\pg_ctl.exe initdb -D tmp_check/db -l tmp_check/initdb.log --options=--no-sync
-- 
2.30.2

Reply via email to