0002-WIP-Stop-using-TCP-in-TAP-tests-on-Windows.patch

text/x-patch

Filename: 0002-WIP-Stop-using-TCP-in-TAP-tests-on-Windows.patch
Type: text/x-patch
Part: 1
Message: Re: Cleaning up historical portability baggage

Patch

Format: format-patch
Series: patch 0002
Subject: WIP: Stop using TCP in TAP tests on Windows.
File+
.cirrus.yml 0 3
src/bin/pg_ctl/t/001_start_stop.pl 3 10
src/test/authentication/t/001_password.pl 0 6
src/test/authentication/t/002_saslprep.pl 0 7
src/test/perl/PostgreSQL/Test/Cluster.pm 16 75
src/test/perl/PostgreSQL/Test/Utils.pm 1 8
From 9df705e98ef6197f3c9abd24a66ca2b299ba6d74 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Fri, 19 Aug 2022 12:00:07 +1200
Subject: [PATCH 2/2] WIP: Stop using TCP in TAP tests on Windows.

Since Unix-domain sockets are available on our minimum target Windows
version, we can remove a source of instability and a point of variation
between Unix and Windows.
---
 .cirrus.yml                               |  3 -
 src/bin/pg_ctl/t/001_start_stop.pl        | 13 +---
 src/test/authentication/t/001_password.pl |  6 --
 src/test/authentication/t/002_saslprep.pl |  7 --
 src/test/perl/PostgreSQL/Test/Cluster.pm  | 91 ++++-------------------
 src/test/perl/PostgreSQL/Test/Utils.pm    |  9 +--
 6 files changed, 20 insertions(+), 109 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 81eb8a9996..8890f660c9 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -361,9 +361,6 @@ task:
     # git's tar doesn't deal with drive letters, see
     # https://postgr.es/m/b6782dc3-a7b0-ed56-175f-f8f54cb08d67%40dunslane.net
     TAR: "c:/windows/system32/tar.exe"
-    # Avoids port conflicts between concurrent tap test runs
-    PG_TEST_USE_UNIX_SOCKETS: 1
-    PG_REGRESS_SOCK_DIR: "c:/cirrus/"
     # -m enables parallelism
     # verbosity:minimal + Summary reduce verbosity, while keeping a summary of
     #   errors/warnings
diff --git a/src/bin/pg_ctl/t/001_start_stop.pl b/src/bin/pg_ctl/t/001_start_stop.pl
index fdffd76d99..23f942a440 100644
--- a/src/bin/pg_ctl/t/001_start_stop.pl
+++ b/src/bin/pg_ctl/t/001_start_stop.pl
@@ -29,16 +29,9 @@ print $conf "port = $node_port\n";
 print $conf PostgreSQL::Test::Utils::slurp_file($ENV{TEMP_CONFIG})
   if defined $ENV{TEMP_CONFIG};
 
-if ($use_unix_sockets)
-{
-	print $conf "listen_addresses = ''\n";
-	$tempdir_short =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
-	print $conf "unix_socket_directories = '$tempdir_short'\n";
-}
-else
-{
-	print $conf "listen_addresses = '127.0.0.1'\n";
-}
+print $conf "listen_addresses = ''\n";
+$tempdir_short =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
+print $conf "unix_socket_directories = '$tempdir_short'\n";
 close $conf;
 my $ctlcmd = [
 	'pg_ctl', 'start', '-D', "$tempdir/data", '-l',
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
index 3e3079c824..3e1c1a070b 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -6,18 +6,12 @@
 # - Plain
 # - MD5-encrypted
 # - SCRAM-encrypted
-# This test can only run with Unix-domain sockets.
 
 use strict;
 use warnings;
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
 use Test::More;
-if (!$use_unix_sockets)
-{
-	plan skip_all =>
-	  "authentication tests cannot run without Unix-domain sockets";
-}
 
 # Delete pg_hba.conf from the given node, add a new entry to it
 # and then execute a reload to refresh it.
diff --git a/src/test/authentication/t/002_saslprep.pl b/src/test/authentication/t/002_saslprep.pl
index 5e87e21ee9..23849632c2 100644
--- a/src/test/authentication/t/002_saslprep.pl
+++ b/src/test/authentication/t/002_saslprep.pl
@@ -2,19 +2,12 @@
 # Copyright (c) 2021-2022, PostgreSQL Global Development Group
 
 # Test password normalization in SCRAM.
-#
-# This test can only run with Unix-domain sockets.
 
 use strict;
 use warnings;
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
 use Test::More;
-if (!$use_unix_sockets)
-{
-	plan skip_all =>
-	  "authentication tests cannot run without Unix-domain sockets";
-}
 
 # Delete pg_hba.conf from the given node, add a new entry to it
 # and then execute a reload to refresh it.
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 27fa607da4..6fdf73db2d 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -108,7 +108,7 @@ use PostgreSQL::Test::Utils ();
 use Time::HiRes qw(usleep);
 use Scalar::Util qw(blessed);
 
-our ($use_tcp, $test_localhost, $test_pghost, $last_host_assigned,
+our ($test_pghost,
 	$last_port_assigned, @all_nodes, $died);
 
 # the minimum version we believe to be compatible with this package without
@@ -120,21 +120,11 @@ INIT
 
 	# Set PGHOST for backward compatibility.  This doesn't work for own_host
 	# nodes, so prefer to not rely on this when writing new tests.
-	$use_tcp            = !$PostgreSQL::Test::Utils::use_unix_sockets;
-	$test_localhost     = "127.0.0.1";
-	$last_host_assigned = 1;
-	if ($use_tcp)
-	{
-		$test_pghost = $test_localhost;
-	}
-	else
-	{
-		# On windows, replace windows-style \ path separators with / when
-		# putting socket directories either in postgresql.conf or libpq
-		# connection strings, otherwise they are interpreted as escapes.
-		$test_pghost = PostgreSQL::Test::Utils::tempdir_short;
-		$test_pghost =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
-	}
+	# On windows, replace windows-style \ path separators with / when
+	# putting socket directories either in postgresql.conf or libpq
+	# connection strings, otherwise they are interpreted as escapes.
+	$test_pghost = PostgreSQL::Test::Utils::tempdir_short;
+	$test_pghost =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
 	$ENV{PGHOST}     = $test_pghost;
 	$ENV{PGDATABASE} = 'postgres';
 
@@ -425,12 +415,6 @@ sub set_replication_conf
 	open my $hba, '>>', "$pgdata/pg_hba.conf";
 	print $hba
 	  "\n# Allow replication (set up by PostgreSQL::Test::Cluster.pm)\n";
-	if ($PostgreSQL::Test::Utils::windows_os
-		&& !$PostgreSQL::Test::Utils::use_unix_sockets)
-	{
-		print $hba
-		  "host replication all $test_localhost/32 sspi include_realm=1 map=regress\n";
-	}
 	close $hba;
 	return;
 }
@@ -523,16 +507,8 @@ sub init
 	}
 
 	print $conf "port = $port\n";
-	if ($use_tcp)
-	{
-		print $conf "unix_socket_directories = ''\n";
-		print $conf "listen_addresses = '$host'\n";
-	}
-	else
-	{
-		print $conf "unix_socket_directories = '$host'\n";
-		print $conf "listen_addresses = ''\n";
-	}
+	print $conf "unix_socket_directories = '$host'\n";
+	print $conf "listen_addresses = ''\n";
 	close $conf;
 
 	chmod($self->group_access ? 0640 : 0600, "$pgdata/postgresql.conf")
@@ -751,15 +727,8 @@ sub init_from_backup
 		qq(
 port = $port
 ));
-	if ($use_tcp)
-	{
-		$self->append_conf('postgresql.conf', "listen_addresses = '$host'");
-	}
-	else
-	{
-		$self->append_conf('postgresql.conf',
-			"unix_socket_directories = '$host'");
-	}
+	$self->append_conf('postgresql.conf',
+		"unix_socket_directories = '$host'");
 	$self->enable_streaming($root_node) if $params{has_streaming};
 	$self->enable_restoring($root_node, $params{standby})
 	  if $params{has_restoring};
@@ -1225,9 +1194,7 @@ sub new
 	else
 	{
 		# When selecting a port, we look for an unassigned TCP port number,
-		# even if we intend to use only Unix-domain sockets.  This is clearly
-		# necessary on $use_tcp (Windows) configurations, and it seems like a
-		# good idea on Unixen as well.
+		# even if we intend to use only Unix-domain sockets.
 		$port = get_free_port();
 	}
 
@@ -1235,17 +1202,8 @@ sub new
 	my $host = $test_pghost;
 	if ($params{own_host})
 	{
-		if ($use_tcp)
-		{
-			$last_host_assigned++;
-			$last_host_assigned > 254 and BAIL_OUT("too many own_host nodes");
-			$host = '127.0.0.' . $last_host_assigned;
-		}
-		else
-		{
-			$host = "$test_pghost/$name"; # Assume $name =~ /^[-_a-zA-Z0-9]+$/
-			mkdir $host;
-		}
+		$host = "$test_pghost/$name"; # Assume $name =~ /^[-_a-zA-Z0-9]+$/
+		mkdir $host;
 	}
 
 	my $testname = basename($0);
@@ -1452,6 +1410,7 @@ start other, non-Postgres servers.
 Ports assigned to existing PostgreSQL::Test::Cluster objects are automatically
 excluded, even if those servers are not currently running.
 
+
 XXX A port available now may become unavailable by the time we start
 the desired service.
 
@@ -1481,29 +1440,11 @@ sub get_free_port
 		}
 
 		# Check to see if anything else is listening on this TCP port.
-		# Seek a port available for all possible listen_addresses values,
-		# so callers can harness this port for the widest range of purposes.
-		# The 0.0.0.0 test achieves that for MSYS, which automatically sets
-		# SO_EXCLUSIVEADDRUSE.  Testing 0.0.0.0 is insufficient for Windows
-		# native Perl (https://stackoverflow.com/a/14388707), so we also
-		# have to test individual addresses.  Doing that for 127.0.0/24
-		# addresses other than 127.0.0.1 might fail with EADDRNOTAVAIL on
-		# non-Linux, non-Windows kernels.
-		#
-		# Thus, 0.0.0.0 and individual 127.0.0/24 addresses are tested
-		# only on Windows and only when TCP usage is requested.
 		if ($found == 1)
 		{
-			foreach my $addr (qw(127.0.0.1),
-				($use_tcp && $PostgreSQL::Test::Utils::windows_os)
-				  ? qw(127.0.0.2 127.0.0.3 0.0.0.0)
-				  : ())
+			if (!can_bind(qw(127.0.0.1), $port))
 			{
-				if (!can_bind($addr, $port))
-				{
-					$found = 0;
-					last;
-				}
+				$found = 0;
 			}
 		}
 	}
diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm
index 1ca2cc5917..40968176fc 100644
--- a/src/test/perl/PostgreSQL/Test/Utils.pm
+++ b/src/test/perl/PostgreSQL/Test/Utils.pm
@@ -88,10 +88,9 @@ our @EXPORT = qw(
 
   $windows_os
   $is_msys2
-  $use_unix_sockets
 );
 
-our ($windows_os, $is_msys2, $use_unix_sockets, $timeout_default,
+our ($windows_os, $is_msys2, $timeout_default,
 	$tmp_check, $log_path, $test_logfile);
 
 BEGIN
@@ -153,12 +152,6 @@ BEGIN
 		Win32API::File->import(qw(createFile OsFHandleOpen CloseHandle));
 	}
 
-	# Specifies whether to use Unix sockets for test setups.  On
-	# Windows we don't use them by default since it's not universally
-	# supported, but it can be overridden if desired.
-	$use_unix_sockets =
-	  (!$windows_os || defined $ENV{PG_TEST_USE_UNIX_SOCKETS});
-
 	$timeout_default = $ENV{PG_TEST_TIMEOUT_DEFAULT};
 	$timeout_default = 180
 	  if not defined $timeout_default or $timeout_default eq '';
-- 
2.35.1