From c4429b2ad41850b8e3a360d1093be8a57014a156 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Fri, 19 Aug 2022 12:00:07 +1200 Subject: [PATCH 3/4] WIP: Stop using TCP in TAP tests on Windows. Since Unix-domain sockets are available on our minimum target Windows versions (10+), 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/authentication/t/003_peer.pl | 5 -- .../authentication/t/004_file_inclusion.pl | 5 -- src/test/perl/PostgreSQL/Test/Cluster.pm | 90 ++++--------------- src/test/perl/PostgreSQL/Test/Utils.pm | 9 +- 8 files changed, 19 insertions(+), 119 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index f31923333e..e4aed541d8 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -502,9 +502,6 @@ WINDOWS_ENVIRONMENT_BASE: &WINDOWS_ENVIRONMENT_BASE # 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/" sysinfo_script: | chcp 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 42d3d4c79b..45d105717a 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/003_peer.pl b/src/test/authentication/t/003_peer.pl index 26c34d05d3..d9d8616e30 100644 --- a/src/test/authentication/t/003_peer.pl +++ b/src/test/authentication/t/003_peer.pl @@ -10,11 +10,6 @@ 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/004_file_inclusion.pl b/src/test/authentication/t/004_file_inclusion.pl index c420f3ebca..db4fcd962b 100644 --- a/src/test/authentication/t/004_file_inclusion.pl +++ b/src/test/authentication/t/004_file_inclusion.pl @@ -11,11 +11,6 @@ use PostgreSQL::Test::Utils; use File::Basename qw(basename); use Test::More; use Data::Dumper; -if (!$use_unix_sockets) -{ - plan skip_all => - "authentication tests cannot run without Unix-domain sockets"; -} # Stores the number of lines created for each file. hba_rule and ident_rule # are used to respectively track pg_hba_file_rules.rule_number and diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 7411188dc8..fdaed41f7b 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -116,7 +116,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, $portdir); # the minimum version we believe to be compatible with this package without @@ -131,21 +131,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'; @@ -470,12 +460,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; } @@ -568,16 +552,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") @@ -796,15 +772,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}; @@ -1270,9 +1239,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(); } @@ -1280,17 +1247,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); @@ -1526,29 +1484,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; } $found = _reserve_port($port) if $found; } diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm index b139190cc8..631d8bd362 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.38.1