Unify PostgresNode's new() and get_new_node() methods

There is only one constructor now for PostgresNode, with the idiomatic
name 'new'. The method is not exported by the class, and must be called
as "PostgresNode->new('name',[args])". All the TAP tests that use
PostgresNode are modified accordingly. Third party scripts will need
adjusting, which is a fairly mechanical process (I just used a sed
script).

1 files changed, 27 insertions, 53 deletions

diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 92319452167..45c69a61baf 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -11,7 +11,7 @@ PostgresNode - class representing PostgreSQL server instance
 
   use PostgresNode;
 
-  my $node = PostgresNode->get_new_node('mynode');
+  my $node = PostgresNode->new('mynode');
 
   # Create a data directory with initdb
   $node->init();
@@ -61,9 +61,9 @@ PostgresNode - class representing PostgreSQL server instance
   my $ret = $node->backup_fs_cold('testbackup3')
 
   # Restore it to create a new independent node (not a replica)
-  my $replica = get_new_node('replica');
-  $replica->init_from_backup($node, 'testbackup');
-  $replica->start;
+  my $other_node = PostgresNode->new('mycopy');
+  $other_node->init_from_backup($node, 'testbackup');
+  $other_node->start;
 
   # Stop the server
   $node->stop('fast');
@@ -110,7 +110,6 @@ use Time::HiRes qw(usleep);
 use Scalar::Util qw(blessed);
 
 our @EXPORT = qw(
-  get_new_node
   get_free_port
 );
 
@@ -139,41 +138,6 @@ INIT
 
 =over
 
-=item PostgresNode::new($class, $name, $pghost, $pgport)
-
-Create a new PostgresNode instance. Does not initdb or start it.
-
-You should generally prefer to use get_new_node() instead since it takes care
-of finding port numbers, registering instances for cleanup, etc.
-
-=cut
-
-sub new
-{
-	my ($class, $name, $pghost, $pgport) = @_;
-	my $testname = basename($0);
-	$testname =~ s/\.[^.]+$//;
-	my $self = {
-		_port    => $pgport,
-		_host    => $pghost,
-		_basedir => "$TestLib::tmp_check/t_${testname}_${name}_data",
-		_name    => $name,
-		_logfile_generation => 0,
-		_logfile_base       => "$TestLib::log_path/${testname}_${name}",
-		_logfile            => "$TestLib::log_path/${testname}_${name}.log"
-	};
-
-	bless $self, $class;
-	mkdir $self->{_basedir}
-	  or
-	  BAIL_OUT("could not create data directory \"$self->{_basedir}\": $!");
-	$self->dump_info;
-
-	return $self;
-}
-
-=pod
-
 =item $node->port()
 
 Get the port number assigned to the host. This won't necessarily be a TCP port
@@ -1168,15 +1132,13 @@ sub _update_pid
 
 =pod
 
-=item PostgresNode->get_new_node(node_name, %params)
+=item PostgresNode->new(node_name, %params)
 
 Build a new object of class C<PostgresNode> (or of a subclass, if you have
 one), assigning a free port number.  Remembers the node, to prevent its port
 number from being reused for another node, and to ensure that it gets
 shut down when the test script exits.
 
-You should generally use this instead of C<PostgresNode::new(...)>.
-
 =over
 
 =item port => [1,65535]
@@ -1201,15 +1163,11 @@ not provided, Postgres binaries will be found in the caller's PATH.
 
 =back
 
-For backwards compatibility, it is also exported as a standalone function,
-which can only create objects of class C<PostgresNode>.
-
 =cut
 
-sub get_new_node
+sub new
 {
-	my $class = 'PostgresNode';
-	$class = shift if scalar(@_) % 2 != 1;
+	my $class = shift;
 	my ($name, %params) = @_;
 
 	# Select a port.
@@ -1244,14 +1202,30 @@ sub get_new_node
 		}
 	}
 
-	# Lock port number found by creating a new node
-	my $node = $class->new($name, $host, $port);
+	my $testname = basename($0);
+	$testname =~ s/\.[^.]+$//;
+	my $node = {
+		_port    => $port,
+		_host    => $host,
+		_basedir => "$TestLib::tmp_check/t_${testname}_${name}_data",
+		_name    => $name,
+		_logfile_generation => 0,
+		_logfile_base       => "$TestLib::log_path/${testname}_${name}",
+		_logfile            => "$TestLib::log_path/${testname}_${name}.log"
+	};
 
 	if ($params{install_path})
 	{
 		$node->{_install_path} = $params{install_path};
 	}
 
+	bless $node, $class;
+	mkdir $node->{_basedir}
+	  or
+	  BAIL_OUT("could not create data directory \"$node->{_basedir}\": $!");
+
+	$node->dump_info;
+
 	# Add node to list of nodes
 	push(@all_nodes, $node);
 
@@ -1322,7 +1296,7 @@ sub _set_pg_version
 # the remainder are# set. Then the PATH and (DY)LD_LIBRARY_PATH are adjusted
 # if the node's install path is set, and the copy environment is returned.
 #
-# The install path set in get_new_node needs to be a directory containing
+# The install path set in new() needs to be a directory containing
 # bin and lib subdirectories as in a standard PostgreSQL installation, so this
 # can't be used with installations where the bin and lib directories don't have
 # a common parent directory.
@@ -1407,7 +1381,7 @@ sub installed_command
 =item get_free_port()
 
 Locate an unprivileged (high) TCP port that's not currently bound to
-anything.  This is used by get_new_node, and is also exported for use
+anything.  This is used by new(), and is also exported for use
 by test cases that need to start other, non-Postgres servers.
 
 Ports assigned to existing PostgresNode objects are automatically