Fix several issues with the TAP tests of pg_upgrade

michaelpq · michaelpq · commit 7dd3ee508432 · 2022-05-10T11:31:31.000+09:00
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
diff --git 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 @@ sub generate_db
 
 	# 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 @@ sub generate_db
 	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 @@ sub generate_db
 	# --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 @@ sub generate_db
 {
 	# 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 @@ sub generate_db
 }
 
 # Second dump from the upgraded instance.
-$newnode->run_log(
+$newnode->command_ok(
 	[
 		'pg_dumpall', '--no-sync',
 		'-d',         $newnode->connstr('postgres'),
