0002-Use-only-one-format-and-make-the-test-run-d-20250324.patch

From f26f88364a196dc9589ca451cb54f5e514e3422e Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: Mon, 24 Mar 2025 11:21:12 +0530
Subject: [PATCH 2/3] Use only one format and make the test run default

According to Alvaro (and I agree with him), the test should be run by
default. Otherwise we get to know about a bug only after buildfarm
animal where it's enabled reports a failure. Further testing only one
format may suffice; since all the formats have shown the same bugs till
now.

If we use --directory format we can use -j which reduces the time taken
by dump/restore test by about 12%.

This patch removes PG_TEST_EXTRA option as well as runs the test only in
directory format with parallelism enabled.

Note for committer: If we decide to accept this change, it should be
merged with the previous commit.
---
 doc/src/sgml/regress.sgml              | 12 ----
 src/bin/pg_upgrade/t/002_pg_upgrade.pl | 76 +++++++++-----------------
 2 files changed, 25 insertions(+), 63 deletions(-)

diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index 237b974b3ab..0e5e8e8f309 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -357,18 +357,6 @@ make check-world PG_TEST_EXTRA='kerberos ldap ssl load_balance libpq_encryption'
       </para>
      </listitem>
     </varlistentry>
-
-    <varlistentry>
-     <term><literal>regress_dump_test</literal></term>
-     <listitem>
-      <para>
-       When enabled, <filename>src/bin/pg_upgrade/t/002_pg_upgrade.pl</filename>
-       tests dump and restore of regression database left behind by the
-       regression run. Not enabled by default because it is time and resource
-       consuming.
-      </para>
-     </listitem>
-    </varlistentry>
    </variablelist>
 
    Tests for features that are not supported by the current build
diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
index d08eea6693f..cbd9831bf9e 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -268,16 +268,9 @@ else
 	# should be done in a separate TAP test, but doing it here saves us one full
 	# regression run.
 	#
-	# This step takes several extra seconds and some extra disk space, so
-	# requires an opt-in with the PG_TEST_EXTRA environment variable.
-	#
 	# Do this while the old cluster is running before it is shut down by the
 	# upgrade test.
-	if (   $ENV{PG_TEST_EXTRA}
-		&& $ENV{PG_TEST_EXTRA} =~ /\bregress_dump_test\b/)
-	{
-		test_regression_dump_restore($oldnode, %node_params);
-	}
+	test_regression_dump_restore($oldnode, %node_params);
 }
 
 # Initialize a new node for the upgrade.
@@ -590,53 +583,34 @@ sub test_regression_dump_restore
 	$dst_node->append_conf('postgresql.conf', 'autovacuum = off');
 	$dst_node->start;
 
-	# Test all formats one by one.
-	for my $format ('plain', 'tar', 'directory', 'custom')
-	{
-		my $dump_file = "$tempdir/regression_dump.$format";
-		my $restored_db = 'regression_' . $format;
-
-		# Use --create in dump and restore commands so that the restored
-		# database has the same configurable variable settings as the original
-		# database and the plain dumps taken for comparsion do not differ
-		# because of locale changes. Additionally this provides test coverage
-		# for --create option.
-		$src_node->command_ok(
-			[
-				'pg_dump', "-F$format", '--no-sync',
-				'-d', $src_node->connstr('regression'),
-				'--create', '-f', $dump_file
-			],
-			"pg_dump on source instance in $format format");
+	my $dump_file = "$tempdir/regression.dump";
 
-		my @restore_command;
-		if ($format eq 'plain')
-		{
-			# Restore dump in "plain" format using `psql`.
-			@restore_command = [ 'psql', '-d', 'postgres', '-f', $dump_file ];
-		}
-		else
-		{
-			@restore_command = [
-				'pg_restore', '--create',
-				'-d', 'postgres', $dump_file
-			];
-		}
-		$dst_node->command_ok(@restore_command,
-			"restored dump taken in $format format on destination instance");
+	# Use --create in dump and restore commands so that the restored database
+	# has the same configurable variable settings as the original database so
+	# that the plain dumps taken from both the database taken for comparisong do
+	# not differ because of locale changes. Additionally this provides test
+	# coverage for --create option.
+	#
+	# We use directory format which allows dumping and restoring in parallel to
+	# reduce the test's run time.
+	$src_node->command_ok(
+		[
+			'pg_dump', '-Fd', '-j2', '--no-sync',
+			'-d', $src_node->connstr('regression'),
+			'--create', '-f', $dump_file
+		],
+		"pg_dump on source instance succeeded");
 
-		my $dst_dump =
-		  get_dump_for_comparison($dst_node, 'regression',
-			'dest_dump.' . $format, 0);
+	$dst_node->command_ok(
+		[ 'pg_restore', '--create', '-j2', '-d', 'postgres', $dump_file ],
+		"restored dump to destination instance");
 
-		compare_files($src_dump, $dst_dump,
-			"dump outputs from original and restored regression database (using $format format) match"
-		);
+	my $dst_dump = get_dump_for_comparison($dst_node, 'regression',
+		'dest_dump', 0);
 
-		# Rename the restored database so that it is available for debugging in
-		# case the test fails.
-		$dst_node->safe_psql('postgres', "ALTER DATABASE regression RENAME TO $restored_db");
-	}
+	compare_files($src_dump, $dst_dump,
+			"dump outputs from original and restored regression database match"
+		);
 }
 
 # Dump database `db` from the given `node` in plain format and adjust it for
-- 
2.34.1