Rework PostgresNode's psql method

This makes the psql() method much more capable: it captures both stdout
and stderr; it now returns the psql exit code rather than stdout; a
timeout can now be specified, as can ON_ERROR_STOP behavior; it gained a
new "on_error_die" (defaulting to off) parameter to raise an exception
if there's any problem.  Finally, additional parameters to psql can be
passed if there's need for further tweaking.

For convenience, a new safe_psql() method retains much of the old
behavior of psql(), except that it uses on_error_die on, so that
problems like syntax errors in SQL commands can be detected more easily.

Many existing TAP test files now use safe_psql, which is what is really
wanted.  A couple of ->psql() calls are now added in the commit_ts
tests, which verify that the right thing is happening on certain errors.
Some ->command_fails() calls in recovery tests that were verifying that
psql failed also became ->psql() calls now.

Author: Craig Ringer. Some tweaks by Álvaro Herrera
Reviewed-By: Michaël Paquier

3 files changed, 27 insertions, 23 deletions

diff --git a/src/test/modules/commit_ts/t/001_base.pl b/src/test/modules/commit_ts/t/001_base.pl
index 122b51557ad..f076a2739d4 100644
--- a/src/test/modules/commit_ts/t/001_base.pl
+++ b/src/test/modules/commit_ts/t/001_base.pl
@@ -13,17 +13,17 @@ $node->append_conf('postgresql.conf', 'track_commit_timestamp = on');
 $node->start;
 
 # Create a table, compare "now()" to the commit TS of its xmin
-$node->psql('postgres', 'create table t as select now from (select now(), pg_sleep(1)) f');
-my $true = $node->psql('postgres',
+$node->safe_psql('postgres', 'create table t as select now from (select now(), pg_sleep(1)) f');
+my $true = $node->safe_psql('postgres',
 	'select t.now - ts.* < \'1s\' from t, pg_class c, pg_xact_commit_timestamp(c.xmin) ts where relname = \'t\'');
 is($true, 't', 'commit TS is set');
-my $ts = $node->psql('postgres',
+my $ts = $node->safe_psql('postgres',
 	'select ts.* from pg_class, pg_xact_commit_timestamp(xmin) ts where relname = \'t\'');
 
 # Verify that we read the same TS after crash recovery
 $node->stop('immediate');
 $node->start;
 
-my $recovered_ts = $node->psql('postgres',
+my $recovered_ts = $node->safe_psql('postgres',
 	'select ts.* from pg_class, pg_xact_commit_timestamp(xmin) ts where relname = \'t\'');
 is($recovered_ts, $ts, 'commit TS remains after crash recovery');
diff --git a/src/test/modules/commit_ts/t/002_standby.pl b/src/test/modules/commit_ts/t/002_standby.pl
index 5cc2501c36c..9410c9c3d25 100644
--- a/src/test/modules/commit_ts/t/002_standby.pl
+++ b/src/test/modules/commit_ts/t/002_standby.pl
@@ -4,7 +4,7 @@ use strict;
 use warnings;
 
 use TestLib;
-use Test::More tests => 2;
+use Test::More tests => 4;
 use PostgresNode;
 
 my $bkplabel = 'backup';
@@ -25,31 +25,33 @@ $standby->start;
 
 for my $i (1 .. 10)
 {
-	$master->psql('postgres', "create table t$i()");
+	$master->safe_psql('postgres', "create table t$i()");
 }
-my $master_ts = $master->psql('postgres',
+my $master_ts = $master->safe_psql('postgres',
 	qq{SELECT ts.* FROM pg_class, pg_xact_commit_timestamp(xmin) AS ts WHERE relname = 't10'});
-my $master_lsn = $master->psql('postgres',
+my $master_lsn = $master->safe_psql('postgres',
 	'select pg_current_xlog_location()');
 $standby->poll_query_until('postgres',
 	qq{SELECT '$master_lsn'::pg_lsn <= pg_last_xlog_replay_location()})
 	or die "slave never caught up";
 
-my $standby_ts = $standby->psql('postgres',
+my $standby_ts = $standby->safe_psql('postgres',
 	qq{select ts.* from pg_class, pg_xact_commit_timestamp(xmin) ts where relname = 't10'});
 is($master_ts, $standby_ts, "standby gives same value as master");
 
 $master->append_conf('postgresql.conf', 'track_commit_timestamp = off');
 $master->restart;
-$master->psql('postgres', 'checkpoint');
-$master_lsn = $master->psql('postgres',
+$master->safe_psql('postgres', 'checkpoint');
+$master_lsn = $master->safe_psql('postgres',
 	'select pg_current_xlog_location()');
 $standby->poll_query_until('postgres',
 	qq{SELECT '$master_lsn'::pg_lsn <= pg_last_xlog_replay_location()})
 	or die "slave never caught up";
-$standby->psql('postgres', 'checkpoint');
+$standby->safe_psql('postgres', 'checkpoint');
 
 # This one should raise an error now
-$standby_ts = $standby->psql('postgres',
+my ($ret, $standby_ts_stdout, $standby_ts_stderr) = $standby->psql('postgres',
 	'select ts.* from pg_class, pg_xact_commit_timestamp(xmin) ts where relname = \'t10\'');
-is($standby_ts, '', "standby gives no value when master turned feature off");
+is($ret, 3, 'standby errors when master turned feature off');
+is($standby_ts_stdout, '', "standby gives no value when master turned feature off");
+like($standby_ts_stderr, qr/could not get commit timestamp data/, 'expected error when master turned feature off');
diff --git a/src/test/modules/commit_ts/t/003_standby_2.pl b/src/test/modules/commit_ts/t/003_standby_2.pl
index fadb6a237d9..138cc43dc2b 100644
--- a/src/test/modules/commit_ts/t/003_standby_2.pl
+++ b/src/test/modules/commit_ts/t/003_standby_2.pl
@@ -4,7 +4,7 @@ use strict;
 use warnings;
 
 use TestLib;
-use Test::More tests => 2;
+use Test::More tests => 4;
 use PostgresNode;
 
 my $bkplabel = 'backup';
@@ -24,23 +24,25 @@ $standby->start;
 
 for my $i (1 .. 10)
 {
-	$master->psql('postgres', "create table t$i()");
+	$master->safe_psql('postgres', "create table t$i()");
 }
 $master->append_conf('postgresql.conf', 'track_commit_timestamp = off');
 $master->restart;
-$master->psql('postgres', 'checkpoint');
-my $master_lsn = $master->psql('postgres',
+$master->safe_psql('postgres', 'checkpoint');
+my $master_lsn = $master->safe_psql('postgres',
 	'select pg_current_xlog_location()');
 $standby->poll_query_until('postgres',
 	qq{SELECT '$master_lsn'::pg_lsn <= pg_last_xlog_replay_location()})
 	or die "slave never caught up";
 
-$standby->psql('postgres', 'checkpoint');
+$standby->safe_psql('postgres', 'checkpoint');
 $standby->restart;
 
-my $standby_ts = $standby->psql('postgres',
+my ($psql_ret, $standby_ts_stdout, $standby_ts_stderr) = $standby->psql('postgres',
 	qq{SELECT ts.* FROM pg_class, pg_xact_commit_timestamp(xmin) AS ts WHERE relname = 't10'});
-is($standby_ts, '', "standby does not return a value after restart");
+is($psql_ret, 3, 'expect error when getting commit timestamp after restart');
+is($standby_ts_stdout, '', "standby does not return a value after restart");
+like($standby_ts_stderr, qr/could not get commit timestamp data/, 'expected err msg after restart');
 
 $master->append_conf('postgresql.conf', 'track_commit_timestamp = on');
 $master->restart;
@@ -50,7 +52,7 @@ $master->restart;
 system_or_bail('pg_ctl', '-w', '-D', $standby->data_dir, 'promote');
 $standby->poll_query_until('postgres', "SELECT pg_is_in_recovery() <> true");
 
-$standby->psql('postgres', "create table t11()");
-$standby_ts = $standby->psql('postgres',
+$standby->safe_psql('postgres', "create table t11()");
+my $standby_ts = $standby->safe_psql('postgres',
 	qq{SELECT ts.* FROM pg_class, pg_xact_commit_timestamp(xmin) AS ts WHERE relname = 't11'});
 isnt($standby_ts, '', "standby gives valid value ($standby_ts) after promotion");