Refactor and fix TAP tests of pg_rewind

* Don't pass arguments to prove, since that's not supported on perl 5.8
which is the minimum version supported by the TAP tests. Refactor the
test files themselves to run the tests twice, in both local and remote mode.

* Use eq rather than == for string comparison. This thinko caused the remote
versions of the tests to never run.

* Add "use strict" and "use warnings", and fix warnings that that produced.

* Increase the delay after standby promotion, to make the tests more robust.

* In remote mode, the connection string to the promoted standby was
incorrect, leading to connection errors.

Patch by Michael Paquier, to address Peter Eisentraut's report.

src/bin/pg_rewind/Makefile

@@ -47,6 +47,5 @@ clean distclean maintainer-clean:
 	rm -f pg_rewind$(X) $(OBJS) xlogreader.c
 	rm -rf tmp_check regress_log
 
-check: all
+check:
-	$(prove_check) :: local
+	$(prove_check)
-	$(prove_check) :: remote

src/bin/pg_rewind/RewindTest.pm

@@ -29,6 +29,9 @@ package RewindTest;
 # master and standby servers. The data directories are also available
 # in paths $test_master_datadir and $test_standby_datadir
 
+use strict;
+use warnings;
+
 use TestLib;
 use Test::More;
 
@@ -58,8 +61,8 @@ our @EXPORT = qw(
 
 # Adjust these paths for your environment
 my $testroot = "./tmp_check";
-$test_master_datadir="$testroot/data_master";
+our $test_master_datadir="$testroot/data_master";
-$test_standby_datadir="$testroot/data_standby";
+our $test_standby_datadir="$testroot/data_standby";
 
 mkdir $testroot;
 
@@ -73,8 +76,8 @@ my $port_standby=$port_master + 1;
 my $log_path;
 my $tempdir_short;
 
-$connstr_master="port=$port_master";
+my $connstr_master="port=$port_master";
-$connstr_standby="port=$port_standby";
+my $connstr_standby="port=$port_standby";
 
 $ENV{PGDATABASE} = "postgres";
 
@@ -127,7 +130,8 @@ sub append_to_file
 
 sub init_rewind_test
 {
-	($testname, $test_mode) = @_;
+	my $testname = shift;
+	my $test_mode = shift;
 
 	$log_path="regress_log/pg_rewind_log_${testname}_${test_mode}";
 
@@ -195,11 +199,13 @@ sub promote_standby
 	# Now promote slave and insert some new data on master, this will put
 	# the master out-of-sync with the standby.
 	system_or_bail("pg_ctl -w -D $test_standby_datadir promote >>$log_path 2>&1");
-	sleep 1;
+	sleep 2;
 }
 
 sub run_pg_rewind
 {
+	my $test_mode = shift;
+
 	# Stop the master and be ready to perform the rewind
 	system_or_bail("pg_ctl -w -D $test_master_datadir stop -m fast >>$log_path 2>&1");
 
@@ -212,7 +218,7 @@ sub run_pg_rewind
 	# overwritten during the rewind.
 	copy("$test_master_datadir/postgresql.conf", "$testroot/master-postgresql.conf.tmp");
 	# Now run pg_rewind
-	if ($test_mode == "local")
+	if ($test_mode eq "local")
 	{
 		# Do rewind using a local pgdata as source
 		# Stop the master and be ready to perform the rewind
@@ -225,12 +231,12 @@ sub run_pg_rewind
 				'>>', $log_path, '2>&1');
 		ok ($result, 'pg_rewind local');
 	}
-	elsif ($test_mode == "remote")
+	elsif ($test_mode eq "remote")
 	{
 		# Do rewind using a remote connection as source
 		my $result =
 			run(['./pg_rewind',
-				 "--source-server=\"port=$port_standby dbname=postgres\"",
+				 "--source-server", "port=$port_standby dbname=postgres",
 				 "--target-pgdata=$test_master_datadir"],
 				'>>', $log_path, '2>&1');
 		ok ($result, 'pg_rewind remote');

src/bin/pg_rewind/t/001_basic.pl

@@ -1,13 +1,15 @@
 use strict;
 use warnings;
 use TestLib;
-use Test::More tests => 4;
+use Test::More tests => 8;
 
 use RewindTest;
 
-my $testmode = shift;
+sub run_test
+{
+	my $test_mode = shift;
 
-RewindTest::init_rewind_test('basic', $testmode);
+	RewindTest::init_rewind_test('basic', $test_mode);
 	RewindTest::setup_cluster();
 
 	# Create a test table and insert a row in master.
@@ -24,7 +26,6 @@ master_psql("INSERT INTO trunc_tbl VALUES ('in master')");
 	master_psql("CREATE TABLE tail_tbl (id integer, d text)");
 	master_psql("INSERT INTO tail_tbl VALUES (0, 'in master')");
 
-
 	master_psql("CHECKPOINT");
 
 	RewindTest::create_standby();
@@ -57,7 +58,7 @@ master_psql("INSERT INTO trunc_tbl SELECT 'in master, after promotion: ' || g FR
 	master_psql("DELETE FROM tail_tbl WHERE id > 10");
 	master_psql("VACUUM tail_tbl");
 
-RewindTest::run_pg_rewind();
+	RewindTest::run_pg_rewind($test_mode);
 
 	check_query('SELECT * FROM tbl1',
 		qq(in master
@@ -77,4 +78,10 @@ check_query('SELECT count(*) FROM tail_tbl',
 ),
 		'tail-copy');
 
+}
+
+# Run the test in both modes
+run_test('local');
+run_test('remote');
+
 exit(0);

src/bin/pg_rewind/t/002_databases.pl

@@ -1,13 +1,15 @@
 use strict;
 use warnings;
 use TestLib;
-use Test::More tests => 2;
+use Test::More tests => 4;
 
 use RewindTest;
 
-my $testmode = shift;
+sub run_test
+{
+	my $test_mode = shift;
 
-RewindTest::init_rewind_test('databases', $testmode);
+	RewindTest::init_rewind_test('databases', $test_mode);
 	RewindTest::setup_cluster();
 
 	# Create a database in master.
@@ -25,7 +27,7 @@ master_psql('CREATE DATABASE master_afterpromotion');
 	standby_psql('CREATE DATABASE standby_afterpromotion');
 	# The clusters are now diverged.
 
-RewindTest::run_pg_rewind();
+	RewindTest::run_pg_rewind($test_mode);
 
 	# Check that the correct databases are present after pg_rewind.
 	check_query('SELECT datname FROM pg_database',
@@ -38,4 +40,10 @@ standby_afterpromotion
 ),
 			   'database names');
 
+}
+
+# Run the test in both modes.
+run_test('local');
+run_test('remote');
+
 exit(0);

src/bin/pg_rewind/t/003_extrafiles.pl

@@ -3,17 +3,22 @@
 use strict;
 use warnings;
 use TestLib;
-use Test::More tests => 2;
+use Test::More tests => 4;
 
 use File::Find;
 
 use RewindTest;
 
-my $testmode = shift;
 
-RewindTest::init_rewind_test('extrafiles', $testmode);
+sub run_test
+{
+	my $test_mode = shift;
+
+	RewindTest::init_rewind_test('extrafiles', $test_mode);
 	RewindTest::setup_cluster();
 
+	my $test_master_datadir = $RewindTest::test_master_datadir;
+
 	# Create a subdir and files that will be present in both
 	mkdir "$test_master_datadir/tst_both_dir";
 	append_to_file "$test_master_datadir/tst_both_dir/both_file1", "in both1";
@@ -38,7 +43,7 @@ mkdir "$test_master_datadir/tst_master_dir/master_subdir/";
 	append_to_file "$test_master_datadir/tst_master_dir/master_subdir/master_file3", "in master3";
 
 	RewindTest::promote_standby();
-RewindTest::run_pg_rewind();
+	RewindTest::run_pg_rewind($test_mode);
 
 	# List files in the data directory after rewind.
 	my @paths;
@@ -57,5 +62,10 @@ is_deeply(\@paths,
 			   "$test_master_datadir/tst_standby_dir/standby_subdir",
 			   "$test_master_datadir/tst_standby_dir/standby_subdir/standby_file3"],
 			  "file lists match");
+}
+
+# Run the test in both modes.
+run_test('local');
+run_test('remote');
 
 exit(0);