Cleanup some problems in new Perl test code

Noted by Tom Lane:
- PostgresNode had a BEGIN block which created files, contrary to
  perlmod suggestions to do that only on INIT blocks.
- Assign ports randomly rather than starting from 90600.

Noted by Noah Misch:
- Change use of no-longer-set PGPORT environment variable to $node->port
- Don't start a server in pg_controldata test
- PostgresNode was reading the PID file incorrectly; test the right
  thing, and chomp the line we read from the PID file.
- Remove an unused $devnull variable
- Use 'pg_ctl kill' instead of "kill" directly, for Windos portability.
- Make server log names more informative.

Author: Michael Paquier

diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 3e491a82e4a928dd32a0dd230034ecccdfe26f05..5991cf77f4adbf17ad71ce58946aa33e40343da3 100644 (file)
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -168,13 +168,14 @@ my $recovery_conf = slurp_file "$tempdir/backupR/recovery.conf";
 
 # using a character class for the final "'" here works around an apparent
 # bug in several version of the Msys DTK perl
+my $port = $node->port;
 like(
        $recovery_conf,
        qr/^standby_mode = 'on[']$/m,
        'recovery.conf sets standby_mode');
 like(
        $recovery_conf,
-       qr/^primary_conninfo = '.*port=$ENV{PGPORT}.*'$/m,
+       qr/^primary_conninfo = '.*port=$port.*'$/m,
        'recovery.conf sets primary_conninfo');
 
 $node->command_ok(

diff --git a/src/bin/pg_controldata/t/001_pg_controldata.pl b/src/bin/pg_controldata/t/001_pg_controldata.pl
index ae45f41339ce1a7e86664cea71baaeedba969317..073815aff686ba7d3a7dd62f75220b6ec1b3a9ec 100644 (file)
--- a/src/bin/pg_controldata/t/001_pg_controldata.pl
+++ b/src/bin/pg_controldata/t/001_pg_controldata.pl
@@ -13,7 +13,6 @@ command_fails([ 'pg_controldata', 'nonexistent' ],
 
 my $node = get_new_node();
 $node->init;
-$node->start;
 
 command_like([ 'pg_controldata', $node->data_dir ],
        qr/checkpoint/, 'pg_controldata produces output');

diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index aa7a00c9fa468e27009724ca2064fb6faf1c5ed7..b18c6bf33c65f8314add9cbae9dbde3442254053 100644 (file)
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -27,9 +27,8 @@ our @EXPORT = qw(
 
 our ($test_pghost, $last_port_assigned, @all_nodes);
 
-BEGIN
+INIT
 {
-
        # PGHOST is set once and for all through a single series of tests when
        # this module is loaded.
        $test_pghost =
@@ -38,11 +37,7 @@ BEGIN
        $ENV{PGDATABASE} = 'postgres';
 
        # Tracking of last port value assigned to accelerate free port lookup.
-       # XXX: Should this use PG_VERSION_NUM?
-       $last_port_assigned = 90600 % 16384 + 49152;
-
-       # Node tracking
-       @all_nodes = ();
+       $last_port_assigned = int(rand() * 16384) + 49152;
 }
 
 sub new
@@ -50,12 +45,14 @@ sub new
        my $class  = shift;
        my $pghost = shift;
        my $pgport = shift;
+       my $testname = basename($0);
+       $testname =~ s/\.[^.]+$//;
        my $self   = {
                _port     => $pgport,
                _host     => $pghost,
                _basedir  => TestLib::tempdir,
                _applname => "node_$pgport",
-               _logfile  => "$TestLib::log_path/node_$pgport.log" };
+               _logfile  => "$TestLib::log_path/${testname}_node_${pgport}.log" };
 
        bless $self, $class;
        $self->dump_info;
@@ -297,17 +294,16 @@ sub _update_pid
        # If we can open the PID file, read its first line and that's the PID we
        # want.  If the file cannot be opened, presumably the server is not
        # running; don't be noisy in that case.
-       open my $pidfile, $self->data_dir . "/postmaster.pid";
-       if (not defined $pidfile)
+       if (open my $pidfile, $self->data_dir . "/postmaster.pid")
        {
-               $self->{_pid} = undef;
-               print "# No postmaster PID\n";
+               chomp($self->{_pid} = <$pidfile>);
+               print "# Postmaster PID is $self->{_pid}\n";
+               close $pidfile;
                return;
        }
 
-       $self->{_pid} = <$pidfile>;
-       print "# Postmaster PID is $self->{_pid}\n";
-       close $pidfile;
+       $self->{_pid} = undef;
+       print "# No postmaster PID\n";
 }
 
 #
@@ -327,7 +323,6 @@ sub get_new_node
        {
                $port++;
                print "# Checking for port $port\n";
-               my $devnull = $TestLib::windows_os ? "nul" : "/dev/null";
                if (!TestLib::run_log([ 'pg_isready', '-p', $port ]))
                {
                        $found = 1;
@@ -360,7 +355,7 @@ sub DESTROY
        my $self = shift;
        return if not defined $self->{_pid};
        print "# signalling QUIT to $self->{_pid}\n";
-       kill 'QUIT', $self->{_pid};
+       TestLib::system_log('pg_ctl', 'kill', 'QUIT', $self->{_pid});
 }
 
 sub teardown_node