On Wed, Dec 2, 2015 at 12:01 PM, Alvaro Herrera
<[email protected]> wrote:
> Michael Paquier wrote:
>> On Wed, Dec 2, 2015 at 8:11 AM, Alvaro Herrera <[email protected]>
>> wrote:
>
>> > - It would be nice to have command_ok and command_fails in PostgresNode
>> > too; that would remove the need for setting $ENV{PGPORT} but it's
>> > possible to run commands outside a node too, so we'd need duplicates,
>> > which would be worse.
>>
>> I am fine to let it the way your patch does it. There are already many
>> changes.
>
> Idea: we can have a bare command_ok exported by TestLib just as
> currently, and instance method PostgresNode->command_ok that first sets
> local $ENV{PGPORT} and then calls the other one.
Hm. That would be cleaner and make the code more consistent. Now as
TestLib exports command_ok, command_like and command_fails, we would
get redefinition errors when compiling the code if those routines are
not named differently in PostgresNode. If you want to have the names
consistent, then I guess that the only way would be to remove those
routines from the export list of TestLib and call them directly as for
example TestLib::command_ok(). See for example the patch attached that
applies on top on your patch 2 that adds a set of routines in
PostgresNode with a slightly different name.
>> > Finally, I ran perltidy on all the files, which strangely changed stuff
>> > that I didn't expect it to change. I wonder if this is related to the
>> > perltidy version. Do you see further changes if you run perltidy on the
>> > patched tree?
>>
>> SimpleTee.pm shows some diffs, though it doesn't seem that this patch
>> should care about that. The rest is showing no diffs. And I have used
>> perltidy v20140711.
>
> Yes, the patch doesn't modify SimpleTee -- I just used "find" to indent
> perl files. What I don't understand is why doesn't my perltidy run
> match what was in master currently. It should be a no-op ...
Well I don't get it either :)
I did a run on top of your two patches and saw no differences.
--
Michael
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index bc4afce..96dd103 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -19,11 +19,10 @@ $node->init(hba_permit_replication => 0);
$node->start;
my $pgdata = $node->data_dir;
-$ENV{PGPORT} = $node->port;
-
-command_fails(['pg_basebackup'],
+$node->node_command_fails(
+ ['pg_basebackup'],
'pg_basebackup needs target directory specified');
-command_fails(
+$node->node_command_fails(
[ 'pg_basebackup', '-D', "$tempdir/backup" ],
'pg_basebackup fails because of hba');
@@ -38,7 +37,7 @@ if (open BADCHARS, ">>$tempdir/pgdata/FOO\xe0\xe0\xe0BAR")
$node->set_replication_conf();
system_or_bail 'pg_ctl', '-D', $pgdata, 'reload';
-command_fails(
+$node->node_command_fails(
[ 'pg_basebackup', '-D', "$tempdir/backup" ],
'pg_basebackup fails because of WAL configuration');
@@ -49,7 +48,7 @@ print CONF "wal_level = archive\n";
close CONF;
$node->restart;
-command_ok([ 'pg_basebackup', '-D', "$tempdir/backup" ],
+$node->node_command_ok([ 'pg_basebackup', '-D', "$tempdir/backup" ],
'pg_basebackup runs');
ok(-f "$tempdir/backup/PG_VERSION", 'backup was created');
@@ -58,34 +57,34 @@ is_deeply(
[ sort qw(. .. archive_status) ],
'no WAL files copied');
-command_ok(
+$node->node_command_ok(
[ 'pg_basebackup', '-D', "$tempdir/backup2", '--xlogdir',
"$tempdir/xlog2" ],
'separate xlog directory');
ok(-f "$tempdir/backup2/PG_VERSION", 'backup was created');
ok(-d "$tempdir/xlog2/", 'xlog directory was created');
-command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup", '-Ft' ],
+$node->node_command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup", '-Ft' ],
'tar format');
ok(-f "$tempdir/tarbackup/base.tar", 'backup tar was created');
-command_fails(
+$node->node_command_fails(
[ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-T=/foo" ],
'-T with empty old directory fails');
-command_fails(
+$node->node_command_fails(
[ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-T/foo=" ],
'-T with empty new directory fails');
-command_fails(
+$node->node_command_fails(
[ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp',
"-T/foo=/bar=/baz" ],
'-T with multiple = fails');
-command_fails(
+$node->node_command_fails(
[ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-Tfoo=/bar" ],
'-T with old directory not absolute fails');
-command_fails(
+$node->node_command_fails(
[ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-T/foo=bar" ],
'-T with new directory not absolute fails');
-command_fails(
+$node->node_command_fails(
[ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-Tfoo" ],
'-T with invalid format fails');
@@ -95,7 +94,7 @@ my $superlongpath = "$pgdata/$superlongname";
open FILE, ">$superlongpath" or die "unable to create file $superlongpath";
close FILE;
-command_fails([ 'pg_basebackup', '-D', "$tempdir/tarbackup_l1", '-Ft' ],
+$node->node_command_fails([ 'pg_basebackup', '-D', "$tempdir/tarbackup_l1", '-Ft' ],
'pg_basebackup tar with long name fails');
unlink "$pgdata/$superlongname";
@@ -116,17 +115,17 @@ SKIP:
$node->psql('postgres',
"CREATE TABLESPACE tblspc1 LOCATION '$shorter_tempdir/tblspc1';");
$node->psql('postgres', "CREATE TABLE test1 (a int) TABLESPACE tblspc1;");
- command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup2", '-Ft' ],
+ $node->node_command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup2", '-Ft' ],
'tar format with tablespaces');
ok(-f "$tempdir/tarbackup2/base.tar", 'backup tar was created');
my @tblspc_tars = glob "$tempdir/tarbackup2/[0-9]*.tar";
is(scalar(@tblspc_tars), 1, 'one tablespace tar was created');
- command_fails(
+ $node->node_command_fails(
[ 'pg_basebackup', '-D', "$tempdir/backup1", '-Fp' ],
'plain format with tablespaces fails without tablespace mapping');
- command_ok(
+ $node->node_command_ok(
[ 'pg_basebackup', '-D', "$tempdir/backup1", '-Fp',
"-T$shorter_tempdir/tblspc1=$tempdir/tbackup/tblspc1" ],
'plain format with tablespaces succeeds with tablespace mapping');
@@ -145,7 +144,7 @@ SKIP:
$node->psql('postgres', "DROP TABLESPACE tblspc1;");
$node->psql('postgres',
"CREATE TABLESPACE tblspc2 LOCATION '$shorter_tempdir/tbl=spc2';");
- command_ok(
+ $node->node_command_ok(
[ 'pg_basebackup', '-D', "$tempdir/backup3", '-Fp',
"-T$shorter_tempdir/tbl\\=spc2=$tempdir/tbackup/tbl\\=spc2" ],
'mapping tablespace with = sign in path');
@@ -156,12 +155,12 @@ SKIP:
mkdir "$tempdir/$superlongname";
$node->psql('postgres',
"CREATE TABLESPACE tblspc3 LOCATION '$tempdir/$superlongname';");
- command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup_l3", '-Ft' ],
+ $node->node_command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup_l3", '-Ft' ],
'pg_basebackup tar with long symlink target');
$node->psql('postgres', "DROP TABLESPACE tblspc3;");
}
-command_ok([ 'pg_basebackup', '-D', "$tempdir/backupR", '-R' ],
+$node->node_command_ok([ 'pg_basebackup', '-D', "$tempdir/backupR", '-R' ],
'pg_basebackup -R runs');
ok(-f "$tempdir/backupR/recovery.conf", 'recovery.conf was created');
my $recovery_conf = slurp_file "$tempdir/backupR/recovery.conf";
@@ -177,19 +176,19 @@ like(
qr/^primary_conninfo = '.*port=$ENV{PGPORT}.*'$/m,
'recovery.conf sets primary_conninfo');
-command_ok([ 'pg_basebackup', '-D', "$tempdir/backupxf", '-X', 'fetch' ],
+$node->node_command_ok([ 'pg_basebackup', '-D', "$tempdir/backupxf", '-X', 'fetch' ],
'pg_basebackup -X fetch runs');
ok(grep(/^[0-9A-F]{24}$/, slurp_dir("$tempdir/backupxf/pg_xlog")),
'WAL files copied');
-command_ok([ 'pg_basebackup', '-D', "$tempdir/backupxs", '-X', 'stream' ],
+$node->node_command_ok([ 'pg_basebackup', '-D', "$tempdir/backupxs", '-X', 'stream' ],
'pg_basebackup -X stream runs');
ok(grep(/^[0-9A-F]{24}$/, slurp_dir("$tempdir/backupxf/pg_xlog")),
'WAL files copied');
-command_fails(
+$node->node_command_fails(
[ 'pg_basebackup', '-D', "$tempdir/fail", '-S', 'slot1' ],
'pg_basebackup with replication slot fails without -X stream');
-command_fails(
+$node->node_command_fails(
[ 'pg_basebackup', '-D',
"$tempdir/backupxs_sl_fail", '-X',
'stream', '-S',
@@ -202,7 +201,7 @@ my $lsn = $node->psql('postgres',
q{SELECT restart_lsn FROM pg_replication_slots WHERE slot_name = 'slot1'}
);
is($lsn, '', 'restart LSN of new slot is null');
-command_ok(
+$node->node_command_ok(
[ 'pg_basebackup', '-D', "$tempdir/backupxs_sl", '-X',
'stream', '-S', 'slot1' ],
'pg_basebackup -X stream with replication slot runs');
@@ -211,7 +210,7 @@ $lsn = $node->psql('postgres',
);
like($lsn, qr!^0/[0-9A-Z]{7,8}$!, 'restart LSN of slot has advanced');
-command_ok(
+$node->node_command_ok(
[ 'pg_basebackup', '-D', "$tempdir/backupxs_sl_R", '-X',
'stream', '-S', 'slot1', '-R' ],
'pg_basebackup with replication slot and -R runs');
diff --git a/src/bin/scripts/t/010_clusterdb.pl b/src/bin/scripts/t/010_clusterdb.pl
index b1d4185..5d8c24e 100644
--- a/src/bin/scripts/t/010_clusterdb.pl
+++ b/src/bin/scripts/t/010_clusterdb.pl
@@ -13,14 +13,13 @@ my $node = get_new_node();
$node->init;
$node->start;
-$ENV{PGPORT} = $node->port;
-
$node->issues_sql_like(
['clusterdb'],
qr/statement: CLUSTER;/,
'SQL CLUSTER run');
-command_fails([ 'clusterdb', '-t', 'nonexistent' ],
+$node->node_command_fails(
+ [ 'clusterdb', '-t', 'nonexistent' ],
'fails with nonexistent table');
$node->psql('postgres',
diff --git a/src/bin/scripts/t/020_createdb.pl b/src/bin/scripts/t/020_createdb.pl
index 09efdc6..7fd5893 100644
--- a/src/bin/scripts/t/020_createdb.pl
+++ b/src/bin/scripts/t/020_createdb.pl
@@ -12,7 +12,6 @@ program_options_handling_ok('createdb');
my $node = get_new_node();
$node->init;
$node->start;
-$ENV{PGPORT} = $node->port;
$node->issues_sql_like(
[ 'createdb', 'foobar1' ],
@@ -23,4 +22,6 @@ $node->issues_sql_like(
qr/statement: CREATE DATABASE foobar2 ENCODING 'LATIN1'/,
'create database with encoding');
-command_fails([ 'createdb', 'foobar1' ], 'fails if database already exists');
+$node->node_command_fails(
+ [ 'createdb', 'foobar1' ],
+ 'fails if database already exists');
diff --git a/src/bin/scripts/t/030_createlang.pl b/src/bin/scripts/t/030_createlang.pl
index a323bbf..34d7cac 100644
--- a/src/bin/scripts/t/030_createlang.pl
+++ b/src/bin/scripts/t/030_createlang.pl
@@ -14,9 +14,8 @@ $node->init;
$node->start;
$ENV{PGDATABASE} = 'postgres';
-$ENV{PGPORT} = $node->port;
-command_fails([ 'createlang', 'plpgsql' ],
+$node->node_command_fails([ 'createlang', 'plpgsql' ],
'fails if language already exists');
$node->psql('postgres', 'DROP EXTENSION plpgsql');
@@ -25,4 +24,7 @@ $node->issues_sql_like(
qr/statement: CREATE EXTENSION "plpgsql"/,
'SQL CREATE EXTENSION run');
-command_like([ 'createlang', '--list' ], qr/plpgsql/, 'list output');
+$node->node_command_like(
+ [ 'createlang', '--list' ],
+ qr/plpgsql/,
+ 'list output');
diff --git a/src/bin/scripts/t/040_createuser.pl b/src/bin/scripts/t/040_createuser.pl
index 760b437..1642f45 100644
--- a/src/bin/scripts/t/040_createuser.pl
+++ b/src/bin/scripts/t/040_createuser.pl
@@ -13,8 +13,6 @@ my $node = get_new_node();
$node->init;
$node->start;
-$ENV{PGPORT} = $node->port;
-
$node->issues_sql_like(
[ 'createuser', 'user1' ],
qr/statement: CREATE ROLE user1 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT LOGIN;/,
@@ -32,4 +30,6 @@ $node->issues_sql_like(
qr/statement: CREATE ROLE user3 SUPERUSER CREATEDB CREATEROLE INHERIT LOGIN;/,
'create a superuser');
-command_fails([ 'createuser', 'user1' ], 'fails if role already exists');
+$node->node_command_fails(
+ [ 'createuser', 'user1' ],
+ 'fails if role already exists');
diff --git a/src/bin/scripts/t/050_dropdb.pl b/src/bin/scripts/t/050_dropdb.pl
index 50ee604..67ff243 100644
--- a/src/bin/scripts/t/050_dropdb.pl
+++ b/src/bin/scripts/t/050_dropdb.pl
@@ -12,7 +12,6 @@ program_options_handling_ok('dropdb');
my $node = get_new_node();
$node->init;
$node->start;
-$ENV{PGPORT} = $node->port;
$node->psql('postgres', 'CREATE DATABASE foobar1');
$node->issues_sql_like(
@@ -20,4 +19,6 @@ $node->issues_sql_like(
qr/statement: DROP DATABASE foobar1/,
'SQL DROP DATABASE run');
-command_fails([ 'dropdb', 'nonexistent' ], 'fails with nonexistent database');
+$node->node_command_fails(
+ [ 'dropdb', 'nonexistent' ],
+ 'fails with nonexistent database');
diff --git a/src/bin/scripts/t/060_droplang.pl b/src/bin/scripts/t/060_droplang.pl
index 43720fb..46a9eee 100644
--- a/src/bin/scripts/t/060_droplang.pl
+++ b/src/bin/scripts/t/060_droplang.pl
@@ -12,13 +12,12 @@ program_options_handling_ok('droplang');
my $node = get_new_node();
$node->init;
$node->start;
-$ENV{PGPORT} = $node->port;
$node->issues_sql_like(
[ 'droplang', 'plpgsql', 'postgres' ],
qr/statement: DROP EXTENSION "plpgsql"/,
'SQL DROP EXTENSION run');
-command_fails(
+$node->node_command_fails(
[ 'droplang', 'nonexistent', 'postgres' ],
'fails with nonexistent language');
diff --git a/src/bin/scripts/t/070_dropuser.pl b/src/bin/scripts/t/070_dropuser.pl
index 5a452c4..47913ce 100644
--- a/src/bin/scripts/t/070_dropuser.pl
+++ b/src/bin/scripts/t/070_dropuser.pl
@@ -12,7 +12,6 @@ program_options_handling_ok('dropuser');
my $node = get_new_node();
$node->init;
$node->start;
-$ENV{PGPORT} = $node->port;
$node->psql('postgres', 'CREATE ROLE foobar1');
$node->issues_sql_like(
@@ -20,4 +19,6 @@ $node->issues_sql_like(
qr/statement: DROP ROLE foobar1/,
'SQL DROP ROLE run');
-command_fails([ 'dropuser', 'nonexistent' ], 'fails with nonexistent user');
+$node->node_command_fails(
+ [ 'dropuser', 'nonexistent' ],
+ 'fails with nonexistent user');
diff --git a/src/bin/scripts/t/080_pg_isready.pl b/src/bin/scripts/t/080_pg_isready.pl
index ca512c1..5f107bc 100644
--- a/src/bin/scripts/t/080_pg_isready.pl
+++ b/src/bin/scripts/t/080_pg_isready.pl
@@ -15,6 +15,6 @@ my $node = get_new_node();
$node->init;
$node->start;
-$ENV{PGPORT} = $node->port;
-
-command_ok(['pg_isready'], 'succeeds with server running');
+$node->node_command_ok(
+ ['pg_isready'],
+ 'succeeds with server running');
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 76a935d..c1fd45f 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -423,6 +423,33 @@ sub poll_query_until
return 0;
}
+sub node_command_ok
+{
+ my ($self, $cmd, $test_name) = @_;
+
+ local $ENV{PGPORT} = $self->port;
+
+ TestLib::command_ok($cmd, $test_name);
+}
+
+sub node_command_fails
+{
+ my ($self, $cmd, $test_name) = @_;
+
+ local $ENV{PGPORT} = $self->port;
+
+ TestLib::command_fails($cmd, $test_name);
+}
+
+sub node_command_like
+{
+ my ($self, $cmd, $test_name) = @_;
+
+ local $ENV{PGPORT} = $self->port;
+
+ TestLib::command_like($cmd, $test_name);
+}
+
# Run a command on the node, then verify that $expected_sql appears in the
# server log file.
sub issues_sql_like
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
