Improve test coverage of pg_rewind

This includes new TAP tests for a couple of areas not covered yet and
some improvements:
- More coverage for --no-ensure-shutdown, the enforced recovery step and
--dry-run.
- Failures with option combinations and basic option checks.
- Removal of a duplicated comment.

Author: Alexey Kondratov, Michael Paquier
Discussion: https://postgr.es/m/20191007010651.GD14532@paquier.xyz

3 files changed, 106 insertions, 4 deletions

diff --git a/src/bin/pg_rewind/t/001_basic.pl b/src/bin/pg_rewind/t/001_basic.pl
index c3293e93df7..95d8ccfced1 100644
--- a/src/bin/pg_rewind/t/001_basic.pl
+++ b/src/bin/pg_rewind/t/001_basic.pl
@@ -1,7 +1,7 @@
 use strict;
 use warnings;
 use TestLib;
-use Test::More tests => 11;
+use Test::More tests => 15;
 
 use FindBin;
 use lib $FindBin::RealBin;
@@ -66,6 +66,71 @@ sub run_test
 	master_psql("DELETE FROM tail_tbl WHERE id > 10");
 	master_psql("VACUUM tail_tbl");
 
+	# Before running pg_rewind, do a couple of extra tests with several
+	# option combinations.  As the code paths taken by those tests
+	# do not change for the "local" and "remote" modes, just run them
+	# in "local" mode for simplicity's sake.
+	if ($test_mode eq 'local')
+	{
+		my $master_pgdata  = $node_master->data_dir;
+		my $standby_pgdata = $node_standby->data_dir;
+
+		# First check that pg_rewind fails if the target cluster is
+		# not stopped as it fails to start up for the forced recovery
+		# step.
+		command_fails(
+			[
+				'pg_rewind',       '--debug',
+				'--source-pgdata', $standby_pgdata,
+				'--target-pgdata', $master_pgdata,
+				'--no-sync'
+			],
+			'pg_rewind with running target');
+
+		# Again with --no-ensure-shutdown, which should equally fail.
+		# This time pg_rewind complains without attempting to perform
+		# recovery once.
+		command_fails(
+			[
+				'pg_rewind',       '--debug',
+				'--source-pgdata', $standby_pgdata,
+				'--target-pgdata', $master_pgdata,
+				'--no-sync',       '--no-ensure-shutdown'
+			],
+			'pg_rewind --no-ensure-shutdown with running target');
+
+		# Stop the target, and attempt to run with a local source
+		# still running.  This fails as pg_rewind requires to have
+		# a source cleanly stopped.
+		$node_master->stop;
+		command_fails(
+			[
+				'pg_rewind',       '--debug',
+				'--source-pgdata', $standby_pgdata,
+				'--target-pgdata', $master_pgdata,
+				'--no-sync',       '--no-ensure-shutdown'
+			],
+			'pg_rewind with unexpected running source');
+
+		# Stop the target cluster cleanly, and run again pg_rewind
+		# with --dry-run mode.  If anything gets generated in the data
+		# folder, the follow-up run of pg_rewind will most likely fail,
+		# so keep this test as the last one of this subset.
+		$node_standby->stop;
+		command_ok(
+			[
+				'pg_rewind',       '--debug',
+				'--source-pgdata', $standby_pgdata,
+				'--target-pgdata', $master_pgdata,
+				'--no-sync',       '--dry-run'
+			],
+			'pg_rewind --dry-run');
+
+		# Both clusters need to be alive moving forward.
+		$node_standby->start;
+		$node_master->start;
+	}
+
 	RewindTest::run_pg_rewind($test_mode);
 
 	check_query(
diff --git a/src/bin/pg_rewind/t/005_same_timeline.pl b/src/bin/pg_rewind/t/005_same_timeline.pl
index df469d39393..5464f4203a7 100644
--- a/src/bin/pg_rewind/t/005_same_timeline.pl
+++ b/src/bin/pg_rewind/t/005_same_timeline.pl
@@ -12,9 +12,6 @@ use lib $FindBin::RealBin;
 
 use RewindTest;
 
-# Test that running pg_rewind if the two clusters are on the same
-# timeline runs successfully.
-
 RewindTest::setup_cluster();
 RewindTest::start_master();
 RewindTest::create_standby();
diff --git a/src/bin/pg_rewind/t/006_options.pl b/src/bin/pg_rewind/t/006_options.pl
new file mode 100644
index 00000000000..1515696e663
--- /dev/null
+++ b/src/bin/pg_rewind/t/006_options.pl
@@ -0,0 +1,40 @@
+#
+# Test checking options of pg_rewind.
+#
+use strict;
+use warnings;
+use TestLib;
+use Test::More tests => 12;
+
+program_help_ok('pg_rewind');
+program_version_ok('pg_rewind');
+program_options_handling_ok('pg_rewind');
+
+my $primary_pgdata = TestLib::tempdir;
+my $standby_pgdata = TestLib::tempdir;
+command_fails(
+	[
+		'pg_rewind',       '--debug',
+		'--target-pgdata', $primary_pgdata,
+		'--source-pgdata', $standby_pgdata,
+		'extra_arg1'
+	],
+	'too many arguments');
+command_fails([ 'pg_rewind', '--target-pgdata', $primary_pgdata ],
+	'no source specified');
+command_fails(
+	[
+		'pg_rewind',       '--debug',
+		'--target-pgdata', $primary_pgdata,
+		'--source-pgdata', $standby_pgdata,
+		'--source-server', 'incorrect_source'
+	],
+	'both remote and local sources specified');
+command_fails(
+	[
+		'pg_rewind',       '--debug',
+		'--target-pgdata', $primary_pgdata,
+		'--source-pgdata', $standby_pgdata,
+		'--write-recovery-conf'
+	],
+	'no local source with --write-recovery-conf');