pg_rewind tests

Started by Peter Eisentrautalmost 11 years ago7 messages
#1Peter Eisentraut
peter_e@gmx.net

There are some small issues with the pg_rewind tests.

This technique

check: all
$(prove_check) :: local
$(prove_check) :: remote

for passing arguments to "prove" does not work with the tools included
in Perl 5.8.

While sorting out the portability issues in the TAP framework during the
9.4 release cycle, we had set 5.8 as the oldest Perl version that is
supported. (It's the Perl version in RHEL 5.) I suggest using
environment variables instead, unless we want to change that.

Moreover,

if ($test_mode == "local")
...
elsif ($test_mode == "remote")

don't work, because those are numerical comparisons, not string
comparisons. So the remote branch is never actually run.

Finally, RewindTest.pm should use

use strict;
use warnings;

and the warnings caused by that should be addressed.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Michael Paquier
michael.paquier@gmail.com
In reply to: Peter Eisentraut (#1)
1 attachment(s)
Re: pg_rewind tests

On Tue, Mar 31, 2015 at 9:59 AM, Peter Eisentraut <peter_e@gmx.net> wrote:

There are some small issues with the pg_rewind tests.

This technique

check: all
$(prove_check) :: local
$(prove_check) :: remote

for passing arguments to "prove" does not work with the tools included
in Perl 5.8.

While sorting out the portability issues in the TAP framework during the
9.4 release cycle, we had set 5.8 as the oldest Perl version that is
supported. (It's the Perl version in RHEL 5.) I suggest using
environment variables instead, unless we want to change that.

That's good to know. And I think that by using environment variables it is
necessary to pass an additional flag to prove_check (see
PG_PROVE_TEST_MODE) in the patch attached because prove_check kicks several
instructions, a command mkdir to begin with.

Moreover,

if ($test_mode == "local")
...
elsif ($test_mode == "remote")

don't work, because those are numerical comparisons, not string
comparisons. So the remote branch is never actually run.

That's "eq" I guess.

Finally, RewindTest.pm should use

use strict;
use warnings;

and the warnings caused by that should be addressed.

All those things addressed give more or less the patch attached.

While looking at that I noticed two additional issues:
- In remote mode, the connection string to the promoted standby was
incorrect when running pg_rewind, leading to connection errors
- At least in my environment, a sleep of 1 after the standby promotion was
not sufficient to make the tests work.
Regards,
--
Michael

Attachments:

20150330_pgrewind_tap_fixes.patchtext/x-patch; charset=US-ASCII; name=20150330_pgrewind_tap_fixes.patchDownload
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 7c39d82..4108783 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -323,7 +323,7 @@ endef
 define prove_check
 $(MKDIR_P) tmp_check/log
 $(MAKE) -C $(top_builddir) DESTDIR='$(CURDIR)'/tmp_check/install install >'$(CURDIR)'/tmp_check/log/install.log 2>&1
-cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(CURDIR)/tmp_check/install$(bindir):$$PATH" $(call add_to_path,$(ld_library_path_var),$(CURDIR)/tmp_check/install$(libdir)) top_builddir='$(CURDIR)/$(top_builddir)' PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) t/*.pl
+cd $(srcdir) && TESTDIR='$(CURDIR)' PG_PROVE_TEST_MODE='$(PG_PROVE_TEST_MODE)' PATH="$(CURDIR)/tmp_check/install$(bindir):$$PATH" $(call add_to_path,$(ld_library_path_var),$(CURDIR)/tmp_check/install$(libdir)) top_builddir='$(CURDIR)/$(top_builddir)' PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) t/*.pl
 endef
 
 else
diff --git a/src/bin/pg_rewind/Makefile b/src/bin/pg_rewind/Makefile
index efd4988..2bda545 100644
--- a/src/bin/pg_rewind/Makefile
+++ b/src/bin/pg_rewind/Makefile
@@ -47,6 +47,8 @@ clean distclean maintainer-clean:
 	rm -f pg_rewind$(X) $(OBJS) xlogreader.c
 	rm -rf tmp_check regress_log
 
-check: all
-	$(prove_check) :: local
-	$(prove_check) :: remote
+check:
+	$(eval PG_PROVE_TEST_MODE = local)
+	$(prove_check)
+	$(eval PG_PROVE_TEST_MODE = remote)
+	$(prove_check)
diff --git a/src/bin/pg_rewind/RewindTest.pm b/src/bin/pg_rewind/RewindTest.pm
index 0f8f4ca..f748bd1 100644
--- a/src/bin/pg_rewind/RewindTest.pm
+++ b/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";
-$test_standby_datadir="$testroot/data_standby";
+our $test_master_datadir="$testroot/data_master";
+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";
-$connstr_standby="port=$port_standby";
+my $connstr_master="port=$port_master";
+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,14 @@ 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
+		printf "Port standby $port_standby\n";
+		printf "Port standby $test_master_datadir\n";
 		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');
diff --git a/src/bin/pg_rewind/t/001_basic.pl b/src/bin/pg_rewind/t/001_basic.pl
index 7198a3a..132892d 100644
--- a/src/bin/pg_rewind/t/001_basic.pl
+++ b/src/bin/pg_rewind/t/001_basic.pl
@@ -7,7 +7,7 @@ use RewindTest;
 
 my $testmode = shift;
 
-RewindTest::init_rewind_test('basic', $testmode);
+RewindTest::init_rewind_test('basic', $ENV{PG_PROVE_TEST_MODE});
 RewindTest::setup_cluster();
 
 # Create a test table and insert a row in master.
@@ -57,7 +57,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($ENV{PG_PROVE_TEST_MODE});
 
 check_query('SELECT * FROM tbl1',
 	qq(in master
diff --git a/src/bin/pg_rewind/t/002_databases.pl b/src/bin/pg_rewind/t/002_databases.pl
index 709c81e..61a5588 100644
--- a/src/bin/pg_rewind/t/002_databases.pl
+++ b/src/bin/pg_rewind/t/002_databases.pl
@@ -5,9 +5,7 @@ use Test::More tests => 2;
 
 use RewindTest;
 
-my $testmode = shift;
-
-RewindTest::init_rewind_test('databases', $testmode);
+RewindTest::init_rewind_test('databases', $ENV{PG_PROVE_TEST_MODE});
 RewindTest::setup_cluster();
 
 # Create a database in master.
@@ -25,7 +23,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($ENV{PG_PROVE_TEST_MODE});
 
 # Check that the correct databases are present after pg_rewind.
 check_query('SELECT datname FROM pg_database',
diff --git a/src/bin/pg_rewind/t/003_extrafiles.pl b/src/bin/pg_rewind/t/003_extrafiles.pl
index dd76fb8..975064b 100644
--- a/src/bin/pg_rewind/t/003_extrafiles.pl
+++ b/src/bin/pg_rewind/t/003_extrafiles.pl
@@ -11,9 +11,11 @@ use RewindTest;
 
 my $testmode = shift;
 
-RewindTest::init_rewind_test('extrafiles', $testmode);
+RewindTest::init_rewind_test('extrafiles', $ENV{PG_PROVE_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 +40,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($ENV{PG_PROVE_TEST_MODE});
 
 # List files in the data directory after rewind.
 my @paths;
#3Michael Paquier
michael.paquier@gmail.com
In reply to: Michael Paquier (#2)
1 attachment(s)
Re: pg_rewind tests

On Tue, Mar 31, 2015 at 4:37 PM, Michael Paquier <michael.paquier@gmail.com>
wrote:

While looking at that I noticed two additional issues:
- In remote mode, the connection string to the promoted standby was
incorrect when running pg_rewind, leading to connection errors
- At least in my environment, a sleep of 1 after the standby promotion was
not sufficient to make the tests work.

While working on another patch for TAP tests, I noticed that relying on
environment variables to run tests is a bad idea as well, as other tests do
not do it, so instead is a patch that refactors the tests so as they do not
use environment variables and so as it is not necessary to pass arguments
to prove.
The trick is to use sub-routines in each test, and invoke this subroutine
for both 'local' and 'remote'. This changes the order the tests are run,
but I guess that's not a big deal as long as the tests are run, and this
approach looks more solid to me as it makes pg_rewind's tests more
consistent with the rest. The log files of each test are still separated
the same way as before.
Regards,
--
Michael

Attachments:

0001-Refactor-TAP-tests-of-pg_rewind.patchtext/x-patch; charset=US-ASCII; name=0001-Refactor-TAP-tests-of-pg_rewind.patchDownload
From 1892bc680313cc5ab0044357842a04c853ce1cec Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@otacoo.com>
Date: Thu, 2 Apr 2015 10:46:47 +0900
Subject: [PATCH] Refactor TAP tests of pg_rewind

This removes the dependency on passing arguments to prove, something
incompatible with perl 5.8, the minimum requirement supported by the
in-core TAP tests.

Clean up at the same time a couple of issues:
- RewindTest.pm should use strict and warnings, fixing at the same
  time the multiple warnings reported.
- In remote mode, the connection string to the promoted standby was
  incorrect when running pg_rewind, leading to connection errors.
- a sleep of 1 after standby promotion may not be sufficient in some
  environments, leading to failures.
---
 src/bin/pg_rewind/Makefile            |   5 +-
 src/bin/pg_rewind/RewindTest.pm       |  24 +++++---
 src/bin/pg_rewind/t/001_basic.pl      | 105 ++++++++++++++++++---------------
 src/bin/pg_rewind/t/002_databases.pl  |  46 +++++++++------
 src/bin/pg_rewind/t/003_extrafiles.pl | 108 +++++++++++++++++++---------------
 5 files changed, 159 insertions(+), 129 deletions(-)

diff --git a/src/bin/pg_rewind/Makefile b/src/bin/pg_rewind/Makefile
index efd4988..e3400f5 100644
--- a/src/bin/pg_rewind/Makefile
+++ b/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
-	$(prove_check) :: local
-	$(prove_check) :: remote
+check:
+	$(prove_check)
diff --git a/src/bin/pg_rewind/RewindTest.pm b/src/bin/pg_rewind/RewindTest.pm
index 0f8f4ca..7a20e79 100644
--- a/src/bin/pg_rewind/RewindTest.pm
+++ b/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";
-$test_standby_datadir="$testroot/data_standby";
+our $test_master_datadir="$testroot/data_master";
+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";
-$connstr_standby="port=$port_standby";
+my $connstr_master="port=$port_master";
+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');
diff --git a/src/bin/pg_rewind/t/001_basic.pl b/src/bin/pg_rewind/t/001_basic.pl
index 7198a3a..f2424ab 100644
--- a/src/bin/pg_rewind/t/001_basic.pl
+++ b/src/bin/pg_rewind/t/001_basic.pl
@@ -1,80 +1,87 @@
 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::setup_cluster();
+	RewindTest::init_rewind_test('basic', $test_mode);
+	RewindTest::setup_cluster();
 
-# Create a test table and insert a row in master.
-master_psql("CREATE TABLE tbl1 (d text)");
-master_psql("INSERT INTO tbl1 VALUES ('in master')");
+	# Create a test table and insert a row in master.
+	master_psql("CREATE TABLE tbl1 (d text)");
+	master_psql("INSERT INTO tbl1 VALUES ('in master')");
 
-# This test table will be used to test truncation, i.e. the table
-# is extended in the old master after promotion
-master_psql("CREATE TABLE trunc_tbl (d text)");
-master_psql("INSERT INTO trunc_tbl VALUES ('in master')");
+	# This test table will be used to test truncation, i.e. the table
+	# is extended in the old master after promotion
+	master_psql("CREATE TABLE trunc_tbl (d text)");
+	master_psql("INSERT INTO trunc_tbl VALUES ('in master')");
 
-# This test table will be used to test the "copy-tail" case, i.e. the
-# table is truncated in the old master after promotion
-master_psql("CREATE TABLE tail_tbl (id integer, d text)");
-master_psql("INSERT INTO tail_tbl VALUES (0, 'in master')");
+	# This test table will be used to test the "copy-tail" case, i.e. the
+	# table is truncated in the old master after promotion
+	master_psql("CREATE TABLE tail_tbl (id integer, d text)");
+	master_psql("INSERT INTO tail_tbl VALUES (0, 'in master')");
 
+	master_psql("CHECKPOINT");
 
-master_psql("CHECKPOINT");
+	RewindTest::create_standby();
 
-RewindTest::create_standby();
+	# Insert additional data on master that will be replicated to standby
+	master_psql("INSERT INTO tbl1 values ('in master, before promotion')");
+	master_psql("INSERT INTO trunc_tbl values ('in master, before promotion')");
+	master_psql("INSERT INTO tail_tbl SELECT g, 'in master, before promotion: ' || g FROM generate_series(1, 10000) g");
 
-# Insert additional data on master that will be replicated to standby
-master_psql("INSERT INTO tbl1 values ('in master, before promotion')");
-master_psql("INSERT INTO trunc_tbl values ('in master, before promotion')");
-master_psql("INSERT INTO tail_tbl SELECT g, 'in master, before promotion: ' || g FROM generate_series(1, 10000) g");
+	master_psql('CHECKPOINT');
 
-master_psql('CHECKPOINT');
+	RewindTest::promote_standby();
 
-RewindTest::promote_standby();
+	# Insert a row in the old master. This causes the master and standby
+	# to have "diverged", it's no longer possible to just apply the
+	# standy's logs over master directory - you need to rewind.
+	master_psql("INSERT INTO tbl1 VALUES ('in master, after promotion')");
 
-# Insert a row in the old master. This causes the master and standby
-# to have "diverged", it's no longer possible to just apply the
-# standy's logs over master directory - you need to rewind.
-master_psql("INSERT INTO tbl1 VALUES ('in master, after promotion')");
+	# Also insert a new row in the standby, which won't be present in the
+	# old master.
+	standby_psql("INSERT INTO tbl1 VALUES ('in standby, after promotion')");
 
-# Also insert a new row in the standby, which won't be present in the
-# old master.
-standby_psql("INSERT INTO tbl1 VALUES ('in standby, after promotion')");
+	# Insert enough rows to trunc_tbl to extend the file. pg_rewind should
+	# truncate it back to the old size.
+	master_psql("INSERT INTO trunc_tbl SELECT 'in master, after promotion: ' || g FROM generate_series(1, 10000) g");
 
-# Insert enough rows to trunc_tbl to extend the file. pg_rewind should
-# truncate it back to the old size.
-master_psql("INSERT INTO trunc_tbl SELECT 'in master, after promotion: ' || g FROM generate_series(1, 10000) g");
+	# Truncate tail_tbl. pg_rewind should copy back the truncated part
+	# (We cannot use an actual TRUNCATE command here, as that creates a
+	# whole new relfilenode)
+	master_psql("DELETE FROM tail_tbl WHERE id > 10");
+	master_psql("VACUUM tail_tbl");
 
-# Truncate tail_tbl. pg_rewind should copy back the truncated part
-# (We cannot use an actual TRUNCATE command here, as that creates a
-# whole new relfilenode)
-master_psql("DELETE FROM tail_tbl WHERE id > 10");
-master_psql("VACUUM tail_tbl");
+	RewindTest::run_pg_rewind($test_mode);
 
-RewindTest::run_pg_rewind();
-
-check_query('SELECT * FROM tbl1',
-	qq(in master
+	check_query('SELECT * FROM tbl1',
+		qq(in master
 in master, before promotion
 in standby, after promotion
 ),
-	'table content');
+		'table content');
 
-check_query('SELECT * FROM trunc_tbl',
-	qq(in master
+	check_query('SELECT * FROM trunc_tbl',
+		qq(in master
 in master, before promotion
 ),
-	'truncation');
+		'truncation');
 
-check_query('SELECT count(*) FROM tail_tbl',
-	qq(10001
+	check_query('SELECT count(*) FROM tail_tbl',
+		qq(10001
 ),
-	'tail-copy');
+		'tail-copy');
+
+}
+
+# Run tests in supported modes
+run_test('local');
+run_test('remote');
 
 exit(0);
diff --git a/src/bin/pg_rewind/t/002_databases.pl b/src/bin/pg_rewind/t/002_databases.pl
index 709c81e..e211c65 100644
--- a/src/bin/pg_rewind/t/002_databases.pl
+++ b/src/bin/pg_rewind/t/002_databases.pl
@@ -1,41 +1,49 @@
 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::setup_cluster();
+	RewindTest::init_rewind_test('databases', $test_mode);
+	RewindTest::setup_cluster();
 
-# Create a database in master.
-master_psql('CREATE DATABASE inmaster');
+	# Create a database in master.
+	master_psql('CREATE DATABASE inmaster');
 
-RewindTest::create_standby();
+	RewindTest::create_standby();
 
-# Create another database, the creation is replicated to the standby
-master_psql('CREATE DATABASE beforepromotion');
+	# Create another database, the creation is replicated to the standby
+	master_psql('CREATE DATABASE beforepromotion');
 
-RewindTest::promote_standby();
+	RewindTest::promote_standby();
 
-# Create databases in the old master and the new promoted standby.
-master_psql('CREATE DATABASE master_afterpromotion');
-standby_psql('CREATE DATABASE standby_afterpromotion');
-# The clusters are now diverged.
+	# Create databases in the old master and the new promoted standby.
+	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',
-		   qq(template1
+	# Check that the correct databases are present after pg_rewind.
+	check_query('SELECT datname FROM pg_database',
+			   qq(template1
 template0
 postgres
 inmaster
 beforepromotion
 standby_afterpromotion
 ),
-		   'database names');
+			   'database names');
+
+}
+
+# Run tests in supported modes
+run_test('local');
+run_test('remote');
 
 exit(0);
diff --git a/src/bin/pg_rewind/t/003_extrafiles.pl b/src/bin/pg_rewind/t/003_extrafiles.pl
index dd76fb8..90ab274 100644
--- a/src/bin/pg_rewind/t/003_extrafiles.pl
+++ b/src/bin/pg_rewind/t/003_extrafiles.pl
@@ -3,59 +3,69 @@
 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);
-RewindTest::setup_cluster();
-
-# 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";
-append_to_file "$test_master_datadir/tst_both_dir/both_file2", "in both2";
-mkdir "$test_master_datadir/tst_both_dir/both_subdir/";
-append_to_file "$test_master_datadir/tst_both_dir/both_subdir/both_file3", "in both3";
-
-RewindTest::create_standby();
-
-# Create different subdirs and files in master and standby
-
-mkdir "$test_standby_datadir/tst_standby_dir";
-append_to_file "$test_standby_datadir/tst_standby_dir/standby_file1", "in standby1";
-append_to_file "$test_standby_datadir/tst_standby_dir/standby_file2", "in standby2";
-mkdir "$test_standby_datadir/tst_standby_dir/standby_subdir/";
-append_to_file "$test_standby_datadir/tst_standby_dir/standby_subdir/standby_file3", "in standby3";
-
-mkdir "$test_master_datadir/tst_master_dir";
-append_to_file "$test_master_datadir/tst_master_dir/master_file1", "in master1";
-append_to_file "$test_master_datadir/tst_master_dir/master_file2", "in master2";
-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();
-
-# List files in the data directory after rewind.
-my @paths;
-find(sub {push @paths, $File::Find::name if $File::Find::name =~ m/.*tst_.*/},
-	 $test_master_datadir);
-@paths = sort @paths;
-is_deeply(\@paths,
-		  ["$test_master_datadir/tst_both_dir",
-		   "$test_master_datadir/tst_both_dir/both_file1",
-		   "$test_master_datadir/tst_both_dir/both_file2",
-		   "$test_master_datadir/tst_both_dir/both_subdir",
-		   "$test_master_datadir/tst_both_dir/both_subdir/both_file3",
-		   "$test_master_datadir/tst_standby_dir",
-		   "$test_master_datadir/tst_standby_dir/standby_file1",
-		   "$test_master_datadir/tst_standby_dir/standby_file2",
-		   "$test_master_datadir/tst_standby_dir/standby_subdir",
-		   "$test_master_datadir/tst_standby_dir/standby_subdir/standby_file3"],
-		  "file lists match");
+
+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";
+	append_to_file "$test_master_datadir/tst_both_dir/both_file2", "in both2";
+	mkdir "$test_master_datadir/tst_both_dir/both_subdir/";
+	append_to_file "$test_master_datadir/tst_both_dir/both_subdir/both_file3", "in both3";
+
+	RewindTest::create_standby();
+
+	# Create different subdirs and files in master and standby
+
+	mkdir "$test_standby_datadir/tst_standby_dir";
+	append_to_file "$test_standby_datadir/tst_standby_dir/standby_file1", "in standby1";
+	append_to_file "$test_standby_datadir/tst_standby_dir/standby_file2", "in standby2";
+	mkdir "$test_standby_datadir/tst_standby_dir/standby_subdir/";
+	append_to_file "$test_standby_datadir/tst_standby_dir/standby_subdir/standby_file3", "in standby3";
+
+	mkdir "$test_master_datadir/tst_master_dir";
+	append_to_file "$test_master_datadir/tst_master_dir/master_file1", "in master1";
+	append_to_file "$test_master_datadir/tst_master_dir/master_file2", "in master2";
+	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($test_mode);
+
+	# List files in the data directory after rewind.
+	my @paths;
+	find(sub {push @paths, $File::Find::name if $File::Find::name =~ m/.*tst_.*/},
+		 $test_master_datadir);
+	@paths = sort @paths;
+	is_deeply(\@paths,
+			  ["$test_master_datadir/tst_both_dir",
+			   "$test_master_datadir/tst_both_dir/both_file1",
+			   "$test_master_datadir/tst_both_dir/both_file2",
+			   "$test_master_datadir/tst_both_dir/both_subdir",
+			   "$test_master_datadir/tst_both_dir/both_subdir/both_file3",
+			   "$test_master_datadir/tst_standby_dir",
+			   "$test_master_datadir/tst_standby_dir/standby_file1",
+			   "$test_master_datadir/tst_standby_dir/standby_file2",
+			   "$test_master_datadir/tst_standby_dir/standby_subdir",
+			   "$test_master_datadir/tst_standby_dir/standby_subdir/standby_file3"],
+			  "file lists match");
+}
+
+# Run tests in supported modes
+run_test('local');
+run_test('remote');
 
 exit(0);
-- 
2.3.5

#4Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Peter Eisentraut (#1)
Re: pg_rewind tests

Peter Eisentraut wrote:

There are some small issues with the pg_rewind tests.

Do these tests work in VPATH builds? I now get a failure in "make
check-world" (both with and without Michael's patch). "make check" in
src/bin/pg_rewind dies with the output below. If I change "./pg_rewind"
to "pg_rewind", tests pass, but I wonder if the full path to the
temp-install bindir should be used instead.

make -C ../../.. DESTDIR='/home/alvherre/Code/pgsql/build/master/src/bin/pg_rewind'/tmp_check/install install >'/home/alvherre/Code/pgsql/build/master/src/bin/pg_rewind'/tmp_check/log/install.log 2>&1
cd /pgsql/source/master/src/bin/pg_rewind && TESTDIR='/home/alvherre/Code/pgsql/build/master/src/bin/pg_rewind' PATH="/home/alvherre/Code/pgsql/build/master/src/bin/pg_rewind/tmp_check/install/pgsql/install/master/bin:$PATH" LD_LIBRARY_PATH="/home/alvherre/Code/pgsql/build/master/src/bin/pg_rewind/tmp_check/install/pgsql/install/master/lib" top_builddir='/home/alvherre/Code/pgsql/build/master/src/bin/pg_rewind/../../..' PGPORT='655432' prove -I /pgsql/source/master/src/test/perl/ --verbose t/*.pl :: local
t/001_basic.pl .......
1..4
file not found: ./pg_rewind at RewindTest.pm line 220.
# Looks like your test exited with 2 before it could output anything.
Dubious, test returned 2 (wstat 512, 0x200)
Failed 4/4 subtests
t/002_databases.pl ...
1..2
file not found: ./pg_rewind at RewindTest.pm line 220.
# Looks like your test exited with 2 before it could output anything.
Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/2 subtests
t/003_extrafiles.pl ..
1..2
file not found: ./pg_rewind at RewindTest.pm line 220.
# Looks like your test exited with 2 before it could output anything.
Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/2 subtests

Test Summary Report
-------------------
t/001_basic.pl (Wstat: 512 Tests: 0 Failed: 0)
Non-zero exit status: 2
Parse errors: Bad plan. You planned 4 tests but ran 0.
t/002_databases.pl (Wstat: 512 Tests: 0 Failed: 0)
Non-zero exit status: 2
Parse errors: Bad plan. You planned 2 tests but ran 0.
t/003_extrafiles.pl (Wstat: 512 Tests: 0 Failed: 0)
Non-zero exit status: 2
Parse errors: Bad plan. You planned 2 tests but ran 0.
Files=3, Tests=0, 26 wallclock secs ( 0.02 usr 0.00 sys + 4.41 cusr 0.66 csys = 5.09 CPU)
Result: FAIL
Makefile:51: recipe for target 'check' failed
make: *** [check] Error 1

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Alvaro Herrera (#4)
Re: pg_rewind tests

On 04/10/2015 04:01 PM, Alvaro Herrera wrote:

Do these tests work in VPATH builds? I now get a failure in "make
check-world" (both with and without Michael's patch). "make check" in
src/bin/pg_rewind dies with the output below. If I change "./pg_rewind"
to "pg_rewind", tests pass, but I wonder if the full path to the
temp-install bindir should be used instead.

Just "pg_rewind" seems correct, that's what all the other TAP tests do.
The Makefile target that calls the perl script sets PATH to point to the
temporary installation's bin dir. Fixed.

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Michael Paquier (#3)
Re: pg_rewind tests

On 04/02/2015 04:56 AM, Michael Paquier wrote:

On Tue, Mar 31, 2015 at 4:37 PM, Michael Paquier <michael.paquier@gmail.com>
wrote:

While looking at that I noticed two additional issues:
- In remote mode, the connection string to the promoted standby was
incorrect when running pg_rewind, leading to connection errors
- At least in my environment, a sleep of 1 after the standby promotion was
not sufficient to make the tests work.

While working on another patch for TAP tests, I noticed that relying on
environment variables to run tests is a bad idea as well, as other tests do
not do it, so instead is a patch that refactors the tests so as they do not
use environment variables and so as it is not necessary to pass arguments
to prove.
The trick is to use sub-routines in each test, and invoke this subroutine
for both 'local' and 'remote'. This changes the order the tests are run,
but I guess that's not a big deal as long as the tests are run, and this
approach looks more solid to me as it makes pg_rewind's tests more
consistent with the rest. The log files of each test are still separated
the same way as before.

Applied, thanks!

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Michael Paquier
michael.paquier@gmail.com
In reply to: Heikki Linnakangas (#6)
Re: pg_rewind tests

On Tue, Apr 14, 2015 at 12:32 AM, Heikki Linnakangas wrote:

Applied, thanks!

Thanks. Now, for Windows...
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers