From 7dd3ee508432730d15c5d3032f37362f6b6e4dd8 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Tue, 10 May 2022 11:31:31 +0900
Subject: [PATCH] Fix several issues with the TAP tests of pg_upgrade

This commit addresses the following issues in the TAP tests of
pg_upgrade, introduced in 322becb:
- Remove --port and --host for commands that already rely on a node's
environment PGHOST and PGPORT.
- Switch from run_log() to command_ok(), as all the commands executed in
the tests should succeed.
- Change EXTRA_REGRESS_OPTS to make it count as a shell fragment (fixing
s/OPT/OPTS on a way), to be compatible with the various Makefiles using
it as well as 027_stream_regress.pl in the recovery tests.  The command
built for the execution the pg_regress command is reformatted, while on
it, to map with the recovery test doing the same thing (we should
refactor and consolidate that in the future, perhaps).
- Re-add the test for database names stressing the behavior of
backslashes with double quotes, mostly here for Windows.

Tests doable with the upgrade across different major versions still work
the same way.

Reported-by: Noah Misch
Discussion: https://postgr.es/m/20220502042718.GB1565149@rfd.leadboat.com
---
 src/bin/pg_upgrade/t/002_pg_upgrade.pl | 54 ++++++++++++--------------
 1 file changed, 24 insertions(+), 30 deletions(-)

diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
index 05bf161843..76b8dab4b7 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -21,9 +21,11 @@ sub generate_db
 		next if $i == 7 || $i == 10 || $i == 13;    # skip BEL, LF, and CR
 		$dbname = $dbname . sprintf('%c', $i);
 	}
-	$node->run_log(
-		[ 'createdb', '--host', $node->host, '--port', $node->port, $dbname ]
-	);
+
+	# Exercise backslashes adjacent to double quotes, a Windows special
+	# case.
+	$dbname = '\\"\\' . $dbname . '\\\\"\\\\\\';
+	$node->command_ok([ 'createdb', $dbname ]);
 }
 
 # The test of pg_upgrade requires two clusters, an old one and a new one
@@ -70,12 +72,7 @@ if (defined($ENV{olddump}))
 
 	# Load the dump using the "postgres" database as "regression" does
 	# not exist yet, and we are done here.
-	$oldnode->command_ok(
-		[
-			'psql',   '-X',           '-f',     $olddumpfile,
-			'--port', $oldnode->port, '--host', $oldnode->host,
-			'postgres'
-		]);
+	$oldnode->command_ok([ 'psql', '-X', '-f', $olddumpfile, 'postgres' ]);
 }
 else
 {
@@ -87,8 +84,7 @@ else
 	generate_db($oldnode, 91, 127);
 
 	# Grab any regression options that may be passed down by caller.
-	my $extra_opts_val = $ENV{EXTRA_REGRESS_OPT} || "";
-	my @extra_opts     = split(/\s+/, $extra_opts_val);
+	my $extra_opts = $ENV{EXTRA_REGRESS_OPTS} || "";
 
 	# --dlpath is needed to be able to find the location of regress.so
 	# and any libraries the regression tests require.
@@ -100,19 +96,19 @@ else
 	# --inputdir points to the path of the input files.
 	my $inputdir = "$srcdir/src/test/regress";
 
-	my @regress_command = [
-		$ENV{PG_REGRESS},                             @extra_opts,
-		'--dlpath',                                   $dlpath,
-		'--max-concurrent-tests',                     '20',
-		'--bindir=',                                  '--host',
-		$oldnode->host,                               '--port',
-		$oldnode->port,                               '--schedule',
-		"$srcdir/src/test/regress/parallel_schedule", '--outputdir',
-		$outputdir,                                   '--inputdir',
-		$inputdir
-	];
-
-	my $rc = run_log(@regress_command);
+	my $rc =
+	  system($ENV{PG_REGRESS}
+		  . "$extra_opts "
+		  . "--dlpath=\"$dlpath\" "
+		  . "--bindir= "
+		  . "--host="
+		  . $oldnode->host . " "
+		  . "--port="
+		  . $oldnode->port . " "
+		  . "--schedule=$srcdir/src/test/regress/parallel_schedule "
+		  . "--max-concurrent-tests=20 "
+		  . "--inputdir=\"$inputdir\" "
+		  . "--outputdir=\"$outputdir\"");
 	if ($rc != 0)
 	{
 		# Dump out the regression diffs file, if there is one
@@ -133,12 +129,10 @@ if (defined($ENV{oldinstall}))
 {
 	# Note that upgrade_adapt.sql from the new version is used, to
 	# cope with an upgrade to this version.
-	$oldnode->run_log(
+	$oldnode->command_ok(
 		[
-			'psql',   '-X',
-			'-f',     "$srcdir/src/bin/pg_upgrade/upgrade_adapt.sql",
-			'--port', $oldnode->port,
-			'--host', $oldnode->host,
+			'psql', '-X',
+			'-f',   "$srcdir/src/bin/pg_upgrade/upgrade_adapt.sql",
 			'regression'
 		]);
 }
@@ -232,7 +226,7 @@ if (-d $log_path)
 }
 
 # Second dump from the upgraded instance.
-$newnode->run_log(
+$newnode->command_ok(
 	[
 		'pg_dumpall', '--no-sync',
 		'-d',         $newnode->connstr('postgres'),