Hi all,

As promised on a recent thread, here is a second tentative to switch
pg_upgrade's test.sh into a TAP infrastructure.

This is a continuation of the following thread:
https://www.postgresql.org/message-id/CAB7nPqRdaN1A1YNjxNL9T1jUEWct8ttqq29dNv8W_o37%2Be8wfA%40mail.gmail.com

To begin with, compared to last version, I found and fixed a couple of
bugs, and I also implemented an interface which allows for tests of
pg_upgrade across major versions.  The patch set is as follows:
- 0001 refactors PostgresNode.pm so as a node's binary path is
registered when created and can be enforced by the test creating the
node via get_new_node.  That's useful for pg_upgrade because we want to
use different binary paths for the node to-be-upgraded and the new one.
This is also useful for test suites willing to work on cross-version
tests.
- 0002 begins the move to a TAP suite by introducing a base set of tests
for pg_upgrade.
- 0003 is the real meat, and removes test.sh in favor of a TAP script
aimed at supporting tests using the same version as well as
cross-version tests.

In order to do that, I have changed a bit the interface used for those
tests.  A use can enforce the following variables to run a test using a
past version, while the new version is the one of the tree where the TAP
script is kicked:
export oldsrc=...somewhere/postgresql (old version's source tree)
export oldbindir=...otherversion/bin  (old version's installed bin dir)
export oldlibdir=...otherversion/lib  (old version's installed lib dir)
make check

That is documented as well in TESTING in the patch.  While the patch is
working great when working on the same major version, there is still an
issue I am facing related to the loading of shared libraries in
src/test/regress/ for the old server's version after upgrading it. Even
by tweaking LD_LIBRARY_PATH I could not get the path load. The patch has
been tweaked so as the objects from regression tests that depend on
regress.so, autoinc.so and refint.so are not generated, allowing
pg_upgrade to work, which is definitely wrong.  I am sure that I missed
a trick here but it is Friday here ;p

Note also that the diff of the dump still needs to be eyeballed for two
reasons:
- hash index handling is different in v10~, as those exist now in the
dumps taken.
- PostgreSQL origin version string shows up.
We could apply as well some post-filters to allow the test to pass, but
that's one subject I would like to discuss on this thread.

Another topic that I would like to discuss is how this interface is fit
for the buildfarm code.  After hacking my stuff, I have looked at the
buildfarm code to notice that things like TestUpgradeXversion.pm do
*not* make use of test.sh, and that what I hacked is much similar to
what the buildfarm code is doing, which is itself roughly a copy of what
test.sh does.  Andrew, your feedback would be important here.

So, thoughts?
--
Michael
From 1294bb18426cea5c0764d0d07846fee291502b73 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Wed, 24 Jan 2018 16:19:53 +0900
Subject: [PATCH 1/3] Refactor PostgresNode.pm so as nodes can use custom
 binary path

This is a requirement for having a proper TAP test suite for pg_upgrade
where users have the possibility to manipulate nodes which use different
set of binaries during the upgrade processes.
---
 src/test/perl/PostgresNode.pm | 80 +++++++++++++++++++++++++++++++++----------
 1 file changed, 62 insertions(+), 18 deletions(-)

diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 1d5ac4ee35..c0892fb8a0 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -133,7 +133,7 @@ INIT
 
 =over
 
-=item PostgresNode::new($class, $name, $pghost, $pgport)
+=item PostgresNode::new($class, $name, $pghost, $pgport, $bindir)
 
 Create a new PostgresNode instance. Does not initdb or start it.
 
@@ -144,13 +144,14 @@ of finding port numbers, registering instances for cleanup, etc.
 
 sub new
 {
-	my ($class, $name, $pghost, $pgport) = @_;
+	my ($class, $name, $pghost, $pgport, $bindir) = @_;
 	my $testname = basename($0);
 	$testname =~ s/\.[^.]+$//;
 	my $self = {
 		_port    => $pgport,
 		_host    => $pghost,
 		_basedir => "$TestLib::tmp_check/t_${testname}_${name}_data",
+		_bindir  => $bindir,
 		_name    => $name,
 		_logfile => "$TestLib::log_path/${testname}_${name}.log" };
 
@@ -284,6 +285,20 @@ sub data_dir
 
 =pod
 
+=item $node->bin_dir()
+
+Returns the path to the binary directory used by this node.
+
+=cut
+
+sub bin_dir
+{
+	my ($self) = @_;
+	return $self->{_bindir};
+}
+
+=pod
+
 =item $node->archive_dir()
 
 If archiving is enabled, WAL files go here.
@@ -328,6 +343,7 @@ sub info
 	open my $fh, '>', \$_info or die;
 	print $fh "Name: " . $self->name . "\n";
 	print $fh "Data directory: " . $self->data_dir . "\n";
+	print $fh "Binary directory: " . $self->bin_dir . "\n";
 	print $fh "Backup directory: " . $self->backup_dir . "\n";
 	print $fh "Archive directory: " . $self->archive_dir . "\n";
 	print $fh "Connection string: " . $self->connstr . "\n";
@@ -401,6 +417,7 @@ sub init
 	my ($self, %params) = @_;
 	my $port   = $self->port;
 	my $pgdata = $self->data_dir;
+	my $bindir = $self->bin_dir;
 	my $host   = $self->host;
 
 	$params{allows_streaming} = 0 unless defined $params{allows_streaming};
@@ -409,8 +426,8 @@ sub init
 	mkdir $self->backup_dir;
 	mkdir $self->archive_dir;
 
-	TestLib::system_or_bail('initdb', '-D', $pgdata, '-A', 'trust', '-N',
-		@{ $params{extra} });
+	TestLib::system_or_bail("$bindir/initdb", '-D', $pgdata, '-A', 'trust',
+		'-N', @{ $params{extra} });
 	TestLib::system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata);
 
 	open my $conf, '>>', "$pgdata/postgresql.conf";
@@ -502,12 +519,13 @@ sub backup
 {
 	my ($self, $backup_name) = @_;
 	my $backup_path = $self->backup_dir . '/' . $backup_name;
+	my $bindir      = $self->bin_dir;
 	my $port        = $self->port;
 	my $name        = $self->name;
 
 	print "# Taking pg_basebackup $backup_name from node \"$name\"\n";
-	TestLib::system_or_bail('pg_basebackup', '-D', $backup_path, '-p', $port,
-		'--no-sync');
+	TestLib::system_or_bail("$bindir/pg_basebackup", '-D', $backup_path,
+		'-p', $port, '--no-sync');
 	print "# Backup finished\n";
 }
 
@@ -660,11 +678,12 @@ sub start
 	my ($self) = @_;
 	my $port   = $self->port;
 	my $pgdata = $self->data_dir;
+	my $bindir = $self->bin_dir;
 	my $name   = $self->name;
 	BAIL_OUT("node \"$name\" is already running") if defined $self->{_pid};
 	print("### Starting node \"$name\"\n");
-	my $ret = TestLib::system_log('pg_ctl', '-D', $self->data_dir, '-l',
-		$self->logfile, 'start');
+	my $ret = TestLib::system_log("$bindir/pg_ctl", '-D', $self->data_dir,
+		'-l', $self->logfile, 'start');
 
 	if ($ret != 0)
 	{
@@ -693,11 +712,13 @@ sub stop
 	my ($self, $mode) = @_;
 	my $port   = $self->port;
 	my $pgdata = $self->data_dir;
+	my $bindir = $self->bin_dir;
 	my $name   = $self->name;
 	$mode = 'fast' unless defined $mode;
 	return unless defined $self->{_pid};
 	print "### Stopping node \"$name\" using mode $mode\n";
-	TestLib::system_or_bail('pg_ctl', '-D', $pgdata, '-m', $mode, 'stop');
+	TestLib::system_or_bail("$bindir/pg_ctl", '-D', $pgdata, '-m', $mode,
+		'stop');
 	$self->_update_pid(0);
 }
 
@@ -714,9 +735,10 @@ sub reload
 	my ($self) = @_;
 	my $port   = $self->port;
 	my $pgdata = $self->data_dir;
+	my $bindir = $self->bin_dir;
 	my $name   = $self->name;
 	print "### Reloading node \"$name\"\n";
-	TestLib::system_or_bail('pg_ctl', '-D', $pgdata, 'reload');
+	TestLib::system_or_bail("$bindir/pg_ctl", '-D', $pgdata, 'reload');
 }
 
 =pod
@@ -732,10 +754,11 @@ sub restart
 	my ($self)  = @_;
 	my $port    = $self->port;
 	my $pgdata  = $self->data_dir;
+	my $bindir  = $self->bin_dir;
 	my $logfile = $self->logfile;
 	my $name    = $self->name;
 	print "### Restarting node \"$name\"\n";
-	TestLib::system_or_bail('pg_ctl', '-D', $pgdata, '-l', $logfile,
+	TestLib::system_or_bail("$bindir/pg_ctl", '-D', $pgdata, '-l', $logfile,
 		'restart');
 	$self->_update_pid(1);
 }
@@ -753,10 +776,11 @@ sub promote
 	my ($self)  = @_;
 	my $port    = $self->port;
 	my $pgdata  = $self->data_dir;
+	my $bindir  = $self->bin_dir;
 	my $logfile = $self->logfile;
 	my $name    = $self->name;
 	print "### Promoting node \"$name\"\n";
-	TestLib::system_or_bail('pg_ctl', '-D', $pgdata, '-l', $logfile,
+	TestLib::system_or_bail("$bindir/pg_ctl", '-D', $pgdata, '-l', $logfile,
 		'promote');
 }
 
@@ -860,13 +884,16 @@ sub _update_pid
 
 =pod
 
-=item PostgresNode->get_new_node(node_name)
+=item PostgresNode->get_new_node(node_name [, $bindir ])
 
 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.
 
+<$bindir> can be optionally defined to enforce from where this node should
+use the set of binaries controlling it in the tests.
+
 You should generally use this instead of C<PostgresNode::new(...)>.
 
 For backwards compatibility, it is also exported as a standalone function,
@@ -879,6 +906,7 @@ sub get_new_node
 	my $class = 'PostgresNode';
 	$class = shift if 1 < scalar @_;
 	my $name  = shift;
+	my $bindir = shift;
 	my $found = 0;
 	my $port  = $last_port_assigned;
 
@@ -921,8 +949,21 @@ sub get_new_node
 
 	print "# Found free port $port\n";
 
+	# Find binary directory for this new node.
+	if (!defined($bindir))
+	{
+		# If caller has not defined the binary directory, find it
+		# from pg_config defined in this environment's PATH.
+		my ($stdout, $stderr);
+		my $result = IPC::Run::run [ 'pg_config', '--bindir' ], '>',
+			\$stdout, '2>', \$stderr
+			or die "could not execute pg_config";
+		chomp($stdout);
+		$bindir = $stdout;
+	}
+
 	# Lock port number found by creating a new node
-	my $node = $class->new($name, $test_pghost, $port);
+	my $node = $class->new($name, $test_pghost, $port, $bindir);
 
 	# Add node to list of nodes
 	push(@all_nodes, $node);
@@ -1127,10 +1168,11 @@ sub psql
 
 	my $stdout            = $params{stdout};
 	my $stderr            = $params{stderr};
+	my $bindir            = $self->bin_dir;
 	my $timeout           = undef;
 	my $timeout_exception = 'psql timed out';
 	my @psql_params =
-	  ('psql', '-XAtq', '-d', $self->connstr($dbname), '-f', '-');
+	  ("$bindir/psql", '-XAtq', '-d', $self->connstr($dbname), '-f', '-');
 
 	# If the caller wants an array and hasn't passed stdout/stderr
 	# references, allocate temporary ones to capture them so we
@@ -1275,8 +1317,9 @@ sub poll_query_until
 	my ($self, $dbname, $query, $expected) = @_;
 
 	$expected = 't' unless defined($expected);    # default value
-
-	my $cmd = [ 'psql', '-XAt', '-c', $query, '-d', $self->connstr($dbname) ];
+	my $bindir = $self->bin_dir;
+	my $cmd = [ "$bindir/psql", '-XAt', '-c', $query, '-d',
+				$self->connstr($dbname) ];
 	my ($stdout, $stderr);
 	my $max_attempts = 180 * 10;
 	my $attempts     = 0;
@@ -1666,8 +1709,9 @@ sub pg_recvlogical_upto
 	die 'slot name must be specified' unless defined($slot_name);
 	die 'endpos must be specified'    unless defined($endpos);
 
+	my $bindir = $self->bin_dir;
 	my @cmd = (
-		'pg_recvlogical', '-S', $slot_name, '--dbname',
+		"$bindir/pg_recvlogical", '-S', $slot_name, '--dbname',
 		$self->connstr($dbname));
 	push @cmd, '--endpos', $endpos;
 	push @cmd, '-f', '-', '--no-loop', '--start';
-- 
2.15.1

From df74a70b886bb998ac37195b4d3067c41a012738 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Wed, 24 Jan 2018 16:22:00 +0900
Subject: [PATCH 2/3] Add basic TAP test setup for pg_upgrade

The plan is to convert the current pg_upgrade test to the TAP
framework.  This commit just puts a basic TAP test in place so that we
can see how the build farm behaves, since the build farm client has some
special knowledge of the pg_upgrade tests.
---
 src/bin/pg_upgrade/Makefile       | 7 ++++---
 src/bin/pg_upgrade/t/001_basic.pl | 9 +++++++++
 2 files changed, 13 insertions(+), 3 deletions(-)
 create mode 100644 src/bin/pg_upgrade/t/001_basic.pl

diff --git a/src/bin/pg_upgrade/Makefile b/src/bin/pg_upgrade/Makefile
index 1d6ee702c6..e5c98596a1 100644
--- a/src/bin/pg_upgrade/Makefile
+++ b/src/bin/pg_upgrade/Makefile
@@ -36,8 +36,9 @@ clean distclean maintainer-clean:
 	       pg_upgrade_dump_globals.sql \
 	       pg_upgrade_dump_*.custom pg_upgrade_*.log
 
-check: test.sh all
+check: test.sh
+	$(prove_check)
 	MAKE=$(MAKE) bindir=$(bindir) libdir=$(libdir) EXTRA_REGRESS_OPTS="$(EXTRA_REGRESS_OPTS)" $(SHELL) $< --install
 
-# installcheck is not supported because there's no meaningful way to test
-# pg_upgrade against a single already-running server
+installcheck:
+	$(prove_installcheck)
diff --git a/src/bin/pg_upgrade/t/001_basic.pl b/src/bin/pg_upgrade/t/001_basic.pl
new file mode 100644
index 0000000000..605a7f622f
--- /dev/null
+++ b/src/bin/pg_upgrade/t/001_basic.pl
@@ -0,0 +1,9 @@
+use strict;
+use warnings;
+
+use TestLib;
+use Test::More tests => 8;
+
+program_help_ok('pg_upgrade');
+program_version_ok('pg_upgrade');
+program_options_handling_ok('pg_upgrade');
-- 
2.15.1

From f903e01bbb65f1e38cd74b01ee99c4526e4c3b7f Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Fri, 26 Jan 2018 16:37:42 +0900
Subject: [PATCH 3/3] Replace pg_upgrade's test.sh by a TAP test

This replacement still allows tests across major versions, though the
interface to reach that has been changed a bit. See TESTING for more
details on the matter.
---
 doc/src/sgml/install-windows.sgml      |   1 -
 src/bin/pg_upgrade/Makefile            |   3 +-
 src/bin/pg_upgrade/TESTING             |   8 +-
 src/bin/pg_upgrade/t/002_pg_upgrade.pl | 213 ++++++++++++++++++++++++++
 src/bin/pg_upgrade/test.sh             | 264 ---------------------------------
 src/test/perl/PostgresNode.pm          |   7 +-
 src/tools/msvc/vcregress.pl            | 124 +---------------
 7 files changed, 222 insertions(+), 398 deletions(-)
 create mode 100644 src/bin/pg_upgrade/t/002_pg_upgrade.pl
 delete mode 100644 src/bin/pg_upgrade/test.sh

diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml
index 99e9c7b78e..62fc45e517 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -447,7 +447,6 @@ $ENV{CONFIG}="Debug";
 <userinput>vcregress isolationcheck</userinput>
 <userinput>vcregress bincheck</userinput>
 <userinput>vcregress recoverycheck</userinput>
-<userinput>vcregress upgradecheck</userinput>
 </screen>
 
    To change the schedule used (default is parallel), append it to the
diff --git a/src/bin/pg_upgrade/Makefile b/src/bin/pg_upgrade/Makefile
index e5c98596a1..bccab1d723 100644
--- a/src/bin/pg_upgrade/Makefile
+++ b/src/bin/pg_upgrade/Makefile
@@ -36,9 +36,8 @@ clean distclean maintainer-clean:
 	       pg_upgrade_dump_globals.sql \
 	       pg_upgrade_dump_*.custom pg_upgrade_*.log
 
-check: test.sh
+check:
 	$(prove_check)
-	MAKE=$(MAKE) bindir=$(bindir) libdir=$(libdir) EXTRA_REGRESS_OPTS="$(EXTRA_REGRESS_OPTS)" $(SHELL) $< --install
 
 installcheck:
 	$(prove_installcheck)
diff --git a/src/bin/pg_upgrade/TESTING b/src/bin/pg_upgrade/TESTING
index 6831f679f6..ee45f6a80f 100644
--- a/src/bin/pg_upgrade/TESTING
+++ b/src/bin/pg_upgrade/TESTING
@@ -9,14 +9,12 @@ an upgrade from the version in this source tree to a new instance of
 the same version.
 
 To test an upgrade from a different version, you must have a built
-source tree for the old version as well as this version, and you
-must have done "make install" for both versions.  Then do:
+source tree for the old version as well as this version.  Then do:
 
 export oldsrc=...somewhere/postgresql	(old version's source tree)
 export oldbindir=...otherversion/bin	(old version's installed bin dir)
-export bindir=...thisversion/bin	(this version's installed bin dir)
-export libdir=...thisversion/lib	(this version's installed lib dir)
-sh test.sh
+export oldlibdir=...otherversion/lib	(old version's installed lib dir)
+make check
 
 In this case, you will have to manually eyeball the resulting dump
 diff for version-specific differences, as explained below.
diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
new file mode 100644
index 0000000000..f2c45be284
--- /dev/null
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -0,0 +1,213 @@
+# Set of tests for pg_upgrade.
+use strict;
+use warnings;
+use Cwd;
+use Config;
+use File::Basename;
+use File::Copy;
+use IPC::Run;
+use PostgresNode;
+use TestLib;
+use Test::More tests => 4;
+
+# Generate a database with a name made of a range of ASCII characters.
+sub generate_db
+{
+	my ($node, $from_char, $to_char) = @_;
+
+	my $dbname = '';
+	for my $i ($from_char .. $to_char)
+	{
+		next if $i == 7 || $i == 10 || $i == 13;	# skip BEL, LF, and CR
+		$dbname = $dbname . sprintf('%c', $i);
+	}
+	$node->run_log([ 'createdb', '--port', $node->port, $dbname ]);
+}
+
+my $startdir = getcwd();
+
+# From now on, the test of pg_upgrade consists in setting up an instance
+# on which regression tests are run. This is the source instance used
+# for the upgrade. Then a new, fresh instance is created, and is used
+# as the target instance for the upgrade. Before running an upgrade a
+# logical dump of the old instance is taken, and a second logical dump
+# of the new instance is taken after the upgrade. The upgrade test
+# passes if there are no differences after running pg_upgrade.
+
+# Determine the version set to use depending on what the caller wants.
+# There are a couple of environment variables to scan for:
+# 1) oldsrc, which points to the code tree of the old instance's binaries
+#    which gets upgraded.
+# 2) oldbindir, which points to the binaries of the old instance to
+#    upgrade.
+# 2) oldlibdir, which points to the libraries of the old instance to
+#    upgrade.
+# If the caller has not defined any of them, default values are used,
+# pointing to the source tree where this script is located.
+my $regress_bin = undef;
+my $newsrc = "../../..";	# top of this code repository
+my $newlibdir;
+my $oldsrc = $newsrc;
+my $oldlibdir = undef;
+my $oldbindir = undef;
+my $dlpath = undef;
+
+my ($stdout, $stderr);
+my $result = IPC::Run::run [ 'pg_config', '--libdir' ], '>',
+	\$stdout, '2>', \$stderr
+	or die "could not execute pg_config";
+chomp($stdout);
+$newlibdir = $stdout;
+
+if (!defined($ENV{oldsrc}) &&
+	!defined($ENV{oldlibdir}) &&
+	!defined($ENV{oldbindir}))
+{
+	# No variables defined, so run the test using this version's
+	# tree for both the new and old instances.
+	$regress_bin = $ENV{PG_REGRESS};
+
+	# Calculate this build's old library directory
+	$oldlibdir = $newlibdir;
+
+	# And this build's old binary directory
+	$result = IPC::Run::run [ 'pg_config', '--bindir' ], '>',
+		\$stdout, '2>', \$stderr
+		or die "could not execute pg_config";
+	chomp($stdout);
+	$oldbindir = $stdout;
+
+	# In this case download path is in src/test/regress.
+	$dlpath = ".";
+}
+elsif (defined($ENV{oldsrc}) &&
+	defined($ENV{oldbindir}) &&
+	defined($ENV{oldlibdir}))
+{
+	# A run is wanted on an old version as base.
+	$oldsrc = $ENV{oldsrc};
+	$oldbindir = $ENV{oldbindir};
+	$oldlibdir = $ENV{oldlibdir};
+	# FIXME: this needs better tuning. Using "." or "$oldlibdir/postgresql"
+	# causes the regression tests to pass but pg_upgrade to fail afterwards.
+	$dlpath = "$oldlibdir";
+	$regress_bin = "$oldsrc/src/test/regress/pg_regress";
+}
+else
+{
+	# Not all variables are defined, so leave and die.
+	die "not all variables in oldsrc, oldlibdir and oldbindir are defined";
+}
+
+# Make sure the installed libraries come first in dynamic load paths.
+$ENV{LD_LIBRARY_PATH}="$newlibdir/postgresql";
+$ENV{DYLD_LIBRARY_PATH}="$newlibdir/postgresql";
+
+# Temporary location for dumps taken
+my $tempdir = TestLib::tempdir;
+
+# Initialize node to upgrade
+my $oldnode = get_new_node('old_node', $oldbindir);
+$oldbindir = $oldnode->bin_dir;	# could be default value
+$oldnode->init(extra => [ '--locale=C', '--encoding=LATIN1' ],
+			   pg_regress => $regress_bin);
+$oldnode->start;
+
+# Creating databases with names covering most ASCII bytes
+generate_db($oldnode, 1,  45);
+generate_db($oldnode, 46, 90);
+generate_db($oldnode, 91, 127);
+
+# Install manually regress.so, this got forgotten in the process.
+copy "$oldsrc/src/test/regress/regress.so",
+	"$oldlibdir/postgresql/regress.so"
+	unless (-e "$oldlibdir/regress.so");
+
+# Run regression tests on the old instance, using the binaries of this
+# instance. At the same time create a tablespace path needed for the
+# tests, similarly to what "make check" creates.
+chdir dirname($regress_bin);
+rmdir "testtablespace";
+mkdir "testtablespace";
+$oldnode->run_log([ "$oldbindir/createdb", '--port', $oldnode->port,
+					'regression' ]);
+
+$oldnode->command_ok([$regress_bin, '--schedule', './serial_schedule',
+		'--dlpath', "$dlpath", '--bindir', $oldnode->bin_dir,
+		'--use-existing', '--port', $oldnode->port],
+		'regression test run on old instance');
+
+# Before dumping, get rid of objects not existing in later versions. This
+# depends on the version of the old server used, and matters only if the
+# old and new source paths
+my $oldpgversion;
+($result, $oldpgversion, $stderr) =
+	$oldnode->psql('postgres', qq[SHOW server_version_num;]);
+my $fix_sql;
+if ($newsrc ne $oldsrc)
+{
+	if ($oldpgversion <= 80400)
+	{
+		$fix_sql = "DROP FUNCTION public.myfunc(integer); DROP FUNCTION public.oldstyle_length(integer, text);";
+	}
+	else
+	{
+		$fix_sql = "DROP FUNCTION public.oldstyle_length(integer, text);";
+	}
+	$oldnode->psql('postgres', $fix_sql);
+}
+
+# Take a dump before performing the upgrade as a base comparison. Note
+# that we need to use pg_dumpall from PATH here.
+$oldnode->command_ok(['pg_dumpall', '--no-sync', '--port', $oldnode->port,
+					  '-f', "$tempdir/dump1.sql"],
+					 'dump before running pg_upgrade');
+
+# After dumping, update references to the old source tree's regress.so and
+# such.
+if ($newsrc ne $oldsrc)
+{
+	if ($oldpgversion <= 80400)
+	{
+		$fix_sql = "UPDATE pg_proc SET probin = replace(probin::text, '$oldsrc', '$newsrc')::bytea WHERE probin LIKE '$oldsrc%';";
+	}
+	else
+	{
+		$fix_sql = "UPDATE pg_proc SET probin = replace(probin, '$oldsrc', '$newsrc') WHERE probin LIKE '$oldsrc%';";
+	}
+	$oldnode->psql('postgres', $fix_sql);
+
+	my $dump_data = slurp_file("$tempdir/dump1.sql");
+	$dump_data =~ s/$oldsrc/$newsrc/g;
+
+	open my $fh, ">", "$tempdir/dump1.sql" or die "could not open dump file";
+	print $fh $dump_data;
+	close $fh;
+}
+
+# Move back to current directory, all logs generated need to be located
+# at the origin.
+chdir $startdir;
+
+# Update the instance.
+$oldnode->stop;
+
+# Initialize a new node for the upgrade.
+my $newnode = get_new_node('new_node');
+$newnode->init(extra => [ '--locale=C', '--encoding=LATIN1' ]);
+
+# Time for the real run.
+chdir "$newsrc/src/test/regress";
+my $newbindir = $newnode->bin_dir;
+command_ok(['pg_upgrade', '-d', $oldnode->data_dir, '-D', $newnode->data_dir,
+	'-b', $oldnode->bin_dir, '-B', $newnode->bin_dir, '-p', $oldnode->port,
+	'-P', $newnode->port], 'run of pg_upgrade for new instance');
+$newnode->start;
+
+# Take a second dump on the upgraded instance.
+run_log(['pg_dumpall', '--no-sync', '--port', $newnode->port,
+		 '-f', "$tempdir/dump2.sql"]);
+
+# Compare the two dumps, there should be no differences.
+command_ok(['diff', '-q', "$tempdir/dump1.sql", "$tempdir/dump2.sql"],
+		   'Old and new dump checks after pg_upgrade');
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
deleted file mode 100644
index 39983abea1..0000000000
--- a/src/bin/pg_upgrade/test.sh
+++ /dev/null
@@ -1,264 +0,0 @@
-#!/bin/sh
-
-# src/bin/pg_upgrade/test.sh
-#
-# Test driver for pg_upgrade.  Initializes a new database cluster,
-# runs the regression tests (to put in some data), runs pg_dumpall,
-# runs pg_upgrade, runs pg_dumpall again, compares the dumps.
-#
-# Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
-# Portions Copyright (c) 1994, Regents of the University of California
-
-set -e
-
-: ${MAKE=make}
-
-# Guard against parallel make issues (see comments in pg_regress.c)
-unset MAKEFLAGS
-unset MAKELEVEL
-
-# Run a given "initdb" binary and overlay the regression testing
-# authentication configuration.
-standard_initdb() {
-	# To increase coverage of non-standard segment size without
-	# increase test runtime, run these tests with a lower setting.
-	"$1" -N --wal-segsize 1
-	if [ -n "$TEMP_CONFIG" -a -r "$TEMP_CONFIG" ]
-	then
-		cat "$TEMP_CONFIG" >> "$PGDATA/postgresql.conf"
-	fi
-	../../test/regress/pg_regress --config-auth "$PGDATA"
-}
-
-# Establish how the server will listen for connections
-testhost=`uname -s`
-
-case $testhost in
-	MINGW*)
-		LISTEN_ADDRESSES="localhost"
-		PGHOST=localhost
-		;;
-	*)
-		LISTEN_ADDRESSES=""
-		# Select a socket directory.  The algorithm is from the "configure"
-		# script; the outcome mimics pg_regress.c:make_temp_sockdir().
-		PGHOST=$PG_REGRESS_SOCK_DIR
-		if [ "x$PGHOST" = x ]; then
-			{
-				dir=`(umask 077 &&
-					  mktemp -d /tmp/pg_upgrade_check-XXXXXX) 2>/dev/null` &&
-				[ -d "$dir" ]
-			} ||
-			{
-				dir=/tmp/pg_upgrade_check-$$-$RANDOM
-				(umask 077 && mkdir "$dir")
-			} ||
-			{
-				echo "could not create socket temporary directory in \"/tmp\""
-				exit 1
-			}
-
-			PGHOST=$dir
-			trap 'rm -rf "$PGHOST"' 0
-			trap 'exit 3' 1 2 13 15
-		fi
-		;;
-esac
-
-POSTMASTER_OPTS="-F -c listen_addresses=$LISTEN_ADDRESSES -k \"$PGHOST\""
-export PGHOST
-
-# don't rely on $PWD here, as old shells don't set it
-temp_root=`pwd`/tmp_check
-
-if [ "$1" = '--install' ]; then
-	temp_install=$temp_root/install
-	bindir=$temp_install/$bindir
-	libdir=$temp_install/$libdir
-
-	"$MAKE" -s -C ../.. install DESTDIR="$temp_install"
-
-	# platform-specific magic to find the shared libraries; see pg_regress.c
-	LD_LIBRARY_PATH=$libdir:$LD_LIBRARY_PATH
-	export LD_LIBRARY_PATH
-	DYLD_LIBRARY_PATH=$libdir:$DYLD_LIBRARY_PATH
-	export DYLD_LIBRARY_PATH
-	LIBPATH=$libdir:$LIBPATH
-	export LIBPATH
-	SHLIB_PATH=$libdir:$SHLIB_PATH
-	export SHLIB_PATH
-	PATH=$libdir:$PATH
-
-	# We need to make it use psql from our temporary installation,
-	# because otherwise the installcheck run below would try to
-	# use psql from the proper installation directory, which might
-	# be outdated or missing. But don't override anything else that's
-	# already in EXTRA_REGRESS_OPTS.
-	EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --bindir='$bindir'"
-	export EXTRA_REGRESS_OPTS
-fi
-
-: ${oldbindir=$bindir}
-
-: ${oldsrc=../../..}
-oldsrc=`cd "$oldsrc" && pwd`
-newsrc=`cd ../../.. && pwd`
-
-PATH=$bindir:$PATH
-export PATH
-
-BASE_PGDATA=$temp_root/data
-PGDATA="$BASE_PGDATA.old"
-export PGDATA
-rm -rf "$BASE_PGDATA" "$PGDATA"
-
-logdir=`pwd`/log
-rm -rf "$logdir"
-mkdir "$logdir"
-
-# Clear out any environment vars that might cause libpq to connect to
-# the wrong postmaster (cf pg_regress.c)
-#
-# Some shells, such as NetBSD's, return non-zero from unset if the variable
-# is already unset. Since we are operating under 'set -e', this causes the
-# script to fail. To guard against this, set them all to an empty string first.
-PGDATABASE="";        unset PGDATABASE
-PGUSER="";            unset PGUSER
-PGSERVICE="";         unset PGSERVICE
-PGSSLMODE="";         unset PGSSLMODE
-PGREQUIRESSL="";      unset PGREQUIRESSL
-PGCONNECT_TIMEOUT=""; unset PGCONNECT_TIMEOUT
-PGHOSTADDR="";        unset PGHOSTADDR
-
-# Select a non-conflicting port number, similarly to pg_regress.c
-PG_VERSION_NUM=`grep '#define PG_VERSION_NUM' "$newsrc"/src/include/pg_config.h | awk '{print $3}'`
-PGPORT=`expr $PG_VERSION_NUM % 16384 + 49152`
-export PGPORT
-
-i=0
-while psql -X postgres </dev/null 2>/dev/null
-do
-	i=`expr $i + 1`
-	if [ $i -eq 16 ]
-	then
-		echo port $PGPORT apparently in use
-		exit 1
-	fi
-	PGPORT=`expr $PGPORT + 1`
-	export PGPORT
-done
-
-# buildfarm may try to override port via EXTRA_REGRESS_OPTS ...
-EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --port=$PGPORT"
-export EXTRA_REGRESS_OPTS
-
-# enable echo so the user can see what is being executed
-set -x
-
-standard_initdb "$oldbindir"/initdb
-"$oldbindir"/pg_ctl start -l "$logdir/postmaster1.log" -o "$POSTMASTER_OPTS" -w
-
-# Create databases with names covering the ASCII bytes other than NUL, BEL,
-# LF, or CR.  BEL would ring the terminal bell in the course of this test, and
-# it is not otherwise a special case.  PostgreSQL doesn't support the rest.
-dbname1=`awk 'BEGIN { for (i= 1; i < 46; i++)
-	if (i != 7 && i != 10 && i != 13) printf "%c", i }' </dev/null`
-# Exercise backslashes adjacent to double quotes, a Windows special case.
-dbname1='\"\'$dbname1'\\"\\\'
-dbname2=`awk 'BEGIN { for (i = 46; i <  91; i++) printf "%c", i }' </dev/null`
-dbname3=`awk 'BEGIN { for (i = 91; i < 128; i++) printf "%c", i }' </dev/null`
-createdb "$dbname1" || createdb_status=$?
-createdb "$dbname2" || createdb_status=$?
-createdb "$dbname3" || createdb_status=$?
-
-if "$MAKE" -C "$oldsrc" installcheck; then
-	oldpgversion=`psql -X -A -t -d regression -c "SHOW server_version_num"`
-
-	# before dumping, get rid of objects not existing in later versions
-	if [ "$newsrc" != "$oldsrc" ]; then
-		fix_sql=""
-		case $oldpgversion in
-			804??)
-				fix_sql="DROP FUNCTION public.myfunc(integer); DROP FUNCTION public.oldstyle_length(integer, text);"
-				;;
-			*)
-				fix_sql="DROP FUNCTION public.oldstyle_length(integer, text);"
-				;;
-		esac
-		psql -X -d regression -c "$fix_sql;" || psql_fix_sql_status=$?
-	fi
-
-	pg_dumpall --no-sync -f "$temp_root"/dump1.sql || pg_dumpall1_status=$?
-
-	if [ "$newsrc" != "$oldsrc" ]; then
-		# update references to old source tree's regress.so etc
-		fix_sql=""
-		case $oldpgversion in
-			804??)
-				fix_sql="UPDATE pg_proc SET probin = replace(probin::text, '$oldsrc', '$newsrc')::bytea WHERE probin LIKE '$oldsrc%';"
-				;;
-			*)
-				fix_sql="UPDATE pg_proc SET probin = replace(probin, '$oldsrc', '$newsrc') WHERE probin LIKE '$oldsrc%';"
-				;;
-		esac
-		psql -X -d regression -c "$fix_sql;" || psql_fix_sql_status=$?
-
-		mv "$temp_root"/dump1.sql "$temp_root"/dump1.sql.orig
-		sed "s;$oldsrc;$newsrc;g" "$temp_root"/dump1.sql.orig >"$temp_root"/dump1.sql
-	fi
-else
-	make_installcheck_status=$?
-fi
-"$oldbindir"/pg_ctl -m fast stop
-if [ -n "$createdb_status" ]; then
-	exit 1
-fi
-if [ -n "$make_installcheck_status" ]; then
-	exit 1
-fi
-if [ -n "$psql_fix_sql_status" ]; then
-	exit 1
-fi
-if [ -n "$pg_dumpall1_status" ]; then
-	echo "pg_dumpall of pre-upgrade database cluster failed"
-	exit 1
-fi
-
-PGDATA=$BASE_PGDATA
-
-standard_initdb 'initdb'
-
-pg_upgrade $PG_UPGRADE_OPTS -d "${PGDATA}.old" -D "${PGDATA}" -b "$oldbindir" -B "$bindir" -p "$PGPORT" -P "$PGPORT"
-
-pg_ctl start -l "$logdir/postmaster2.log" -o "$POSTMASTER_OPTS" -w
-
-case $testhost in
-	MINGW*)	cmd /c analyze_new_cluster.bat ;;
-	*)		sh ./analyze_new_cluster.sh ;;
-esac
-
-pg_dumpall --no-sync -f "$temp_root"/dump2.sql || pg_dumpall2_status=$?
-pg_ctl -m fast stop
-
-# no need to echo commands anymore
-set +x
-echo
-
-if [ -n "$pg_dumpall2_status" ]; then
-	echo "pg_dumpall of post-upgrade database cluster failed"
-	exit 1
-fi
-
-case $testhost in
-	MINGW*)	cmd /c delete_old_cluster.bat ;;
-	*)	    sh ./delete_old_cluster.sh ;;
-esac
-
-if diff "$temp_root"/dump1.sql "$temp_root"/dump2.sql >/dev/null; then
-	echo PASSED
-	exit 0
-else
-	echo "Files $temp_root/dump1.sql and $temp_root/dump2.sql differ"
-	echo "dumps were not identical"
-	exit 1
-fi
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index c0892fb8a0..2e957965ba 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -422,13 +422,14 @@ sub init
 
 	$params{allows_streaming} = 0 unless defined $params{allows_streaming};
 	$params{has_archiving}    = 0 unless defined $params{has_archiving};
+	$params{pg_regress} = $ENV{PG_REGRESS} unless defined $params{pg_regress};
 
 	mkdir $self->backup_dir;
 	mkdir $self->archive_dir;
 
 	TestLib::system_or_bail("$bindir/initdb", '-D', $pgdata, '-A', 'trust',
 		'-N', @{ $params{extra} });
-	TestLib::system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata);
+	TestLib::system_or_bail($params{pg_regress}, '--config-auth', $pgdata);
 
 	open my $conf, '>>', "$pgdata/postgresql.conf";
 	print $conf "\n# Added by PostgresNode.pm\n";
@@ -683,7 +684,7 @@ sub start
 	BAIL_OUT("node \"$name\" is already running") if defined $self->{_pid};
 	print("### Starting node \"$name\"\n");
 	my $ret = TestLib::system_log("$bindir/pg_ctl", '-D', $self->data_dir,
-		'-l', $self->logfile, 'start');
+		'-l', $self->logfile, '-w', 'start');
 
 	if ($ret != 0)
 	{
@@ -904,7 +905,7 @@ which can only create objects of class C<PostgresNode>.
 sub get_new_node
 {
 	my $class = 'PostgresNode';
-	$class = shift if 1 < scalar @_;
+	$class = shift if 2 < scalar @_;
 	my $name  = shift;
 	my $bindir = shift;
 	my $found = 0;
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 314f2c37d2..27dc9dd5c5 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -34,7 +34,7 @@ if (-e "src/tools/msvc/buildenv.pl")
 
 my $what = shift || "";
 if ($what =~
-/^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest)$/i
+/^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|bincheck|recoverycheck|taptest)$/i
   )
 {
 	$what = uc $what;
@@ -83,7 +83,6 @@ my %command = (
 	ISOLATIONCHECK => \&isolationcheck,
 	BINCHECK       => \&bincheck,
 	RECOVERYCHECK  => \&recoverycheck,
-	UPGRADECHECK   => \&upgradecheck,
 	TAPTEST        => \&taptest,);
 
 my $proc = $command{$what};
@@ -418,126 +417,6 @@ sub standard_initdb
 			$ENV{PGDATA}) == 0);
 }
 
-# This is similar to appendShellString().  Perl system(@args) bypasses
-# cmd.exe, so omit the caret escape layer.
-sub quote_system_arg
-{
-	my $arg = shift;
-
-	# Change N >= 0 backslashes before a double quote to 2N+1 backslashes.
-	$arg =~ s/(\\*)"/${\($1 . $1)}\\"/gs;
-
-	# Change N >= 1 backslashes at end of argument to 2N backslashes.
-	$arg =~ s/(\\+)$/${\($1 . $1)}/gs;
-
-	# Wrap the whole thing in unescaped double quotes.
-	return "\"$arg\"";
-}
-
-# Generate a database with a name made of a range of ASCII characters, useful
-# for testing pg_upgrade.
-sub generate_db
-{
-	my ($prefix, $from_char, $to_char, $suffix) = @_;
-
-	my $dbname = $prefix;
-	for my $i ($from_char .. $to_char)
-	{
-		next if $i == 7 || $i == 10 || $i == 13;    # skip BEL, LF, and CR
-		$dbname = $dbname . sprintf('%c', $i);
-	}
-	$dbname .= $suffix;
-
-	system('createdb', quote_system_arg($dbname));
-	my $status = $? >> 8;
-	exit $status if $status;
-}
-
-sub upgradecheck
-{
-	my $status;
-	my $cwd = getcwd();
-
-	# Much of this comes from the pg_upgrade test.sh script,
-	# but it only covers the --install case, and not the case
-	# where the old and new source or bin dirs are different.
-	# i.e. only this version to this version check. That's
-	# what pg_upgrade's "make check" does.
-
-	$ENV{PGHOST} = 'localhost';
-	$ENV{PGPORT} ||= 50432;
-	my $tmp_root = "$topdir/src/bin/pg_upgrade/tmp_check";
-	(mkdir $tmp_root || die $!) unless -d $tmp_root;
-	my $upg_tmp_install = "$tmp_root/install";    # unshared temp install
-	print "Setting up temp install\n\n";
-	Install($upg_tmp_install, "all", $config);
-
-	# Install does a chdir, so change back after that
-	chdir $cwd;
-	my ($bindir, $libdir, $oldsrc, $newsrc) =
-	  ("$upg_tmp_install/bin", "$upg_tmp_install/lib", $topdir, $topdir);
-	$ENV{PATH} = "$bindir;$ENV{PATH}";
-	my $data = "$tmp_root/data";
-	$ENV{PGDATA} = "$data.old";
-	my $logdir = "$topdir/src/bin/pg_upgrade/log";
-	(mkdir $logdir || die $!) unless -d $logdir;
-	print "\nRunning initdb on old cluster\n\n";
-	standard_initdb() or exit 1;
-	print "\nStarting old cluster\n\n";
-	my @args = ('pg_ctl', 'start', '-l', "$logdir/postmaster1.log");
-	system(@args) == 0 or exit 1;
-
-	print "\nCreating databases with names covering most ASCII bytes\n\n";
-	generate_db("\\\"\\", 1,  45,  "\\\\\"\\\\\\");
-	generate_db('',       46, 90,  '');
-	generate_db('',       91, 127, '');
-
-	print "\nSetting up data for upgrading\n\n";
-	installcheck();
-
-	# now we can chdir into the source dir
-	chdir "$topdir/src/bin/pg_upgrade";
-	print "\nDumping old cluster\n\n";
-	@args = ('pg_dumpall', '-f', "$tmp_root/dump1.sql");
-	system(@args) == 0 or exit 1;
-	print "\nStopping old cluster\n\n";
-	system("pg_ctl stop") == 0 or exit 1;
-	$ENV{PGDATA} = "$data";
-	print "\nSetting up new cluster\n\n";
-	standard_initdb() or exit 1;
-	print "\nRunning pg_upgrade\n\n";
-	@args = (
-		'pg_upgrade', '-d', "$data.old", '-D', $data, '-b',
-		$bindir,      '-B', $bindir);
-	system(@args) == 0 or exit 1;
-	print "\nStarting new cluster\n\n";
-	@args = ('pg_ctl', '-l', "$logdir/postmaster2.log", 'start');
-	system(@args) == 0 or exit 1;
-	print "\nSetting up stats on new cluster\n\n";
-	system(".\\analyze_new_cluster.bat") == 0 or exit 1;
-	print "\nDumping new cluster\n\n";
-	@args = ('pg_dumpall', '-f', "$tmp_root/dump2.sql");
-	system(@args) == 0 or exit 1;
-	print "\nStopping new cluster\n\n";
-	system("pg_ctl stop") == 0 or exit 1;
-	print "\nDeleting old cluster\n\n";
-	system(".\\delete_old_cluster.bat") == 0 or exit 1;
-	print "\nComparing old and new cluster dumps\n\n";
-
-	@args = ('diff', '-q', "$tmp_root/dump1.sql", "$tmp_root/dump2.sql");
-	system(@args);
-	$status = $?;
-	if (!$status)
-	{
-		print "PASSED\n";
-	}
-	else
-	{
-		print "dumps not identical!\n";
-		exit(1);
-	}
-}
-
 sub fetchRegressOpts
 {
 	my $handle;
@@ -647,7 +526,6 @@ sub usage
 	  "  plcheck        run tests of PL languages\n",
 	  "  recoverycheck  run recovery test suite\n",
 	  "  taptest        run an arbitrary TAP test set\n",
-	  "  upgradecheck   run tests of pg_upgrade\n",
 	  "\nOptions for <arg>: (used by check and installcheck)\n",
 	  "  serial         serial mode\n",
 	  "  parallel       parallel mode\n",
-- 
2.15.1

Attachment: signature.asc
Description: PGP signature

Reply via email to