From 9821492ee417a5910a60f3d1f2ed24c062eab4e0 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Mon, 7 Dec 2015 19:25:31 -0300
Subject: [PATCH] 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
---
 src/bin/pg_basebackup/t/010_pg_basebackup.pl  |  3 +-
 .../pg_controldata/t/001_pg_controldata.pl    |  1 -
 src/test/perl/PostgresNode.pm                 | 29 ++++++++-----------
 3 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 3e491a82e4..5991cf77f4 100644
--- 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 ae45f41339..073815aff6 100644
--- 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 aa7a00c9fa..b18c6bf33c 100644
--- 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