From d3a92f3fc5e16caf03c1bc001ffa333b83483fb2 Mon Sep 17 00:00:00 2001 From: "Andrew A. Bille" Date: Wed, 23 Sep 2026 16:14:53 +0700 Subject: [PATCH] Fix TAP tests with recent IPC::Run on Windows Recent IPC::Run releases changed stream handling on Windows in ways that expose assumptions made by the TAP tests. Text output captured into scalars now needs text mode to be requested explicitly. Without that, CRLF is preserved and tests comparing textual output fail because of stray carriage returns. Add ipc_run_text_mode() so that tests can mark redirected output as text without depending directly on IPC::Run's binary(0) interface. There are also cases where leaving a standard stream implicit causes failures on Windows. In particular, psql can report "Bad file descriptor", and child programs invoked by pg_upgrade can fail when stdin is not provided explicitly. Request explicit stream endpoints where the tests depend on them. The full test suite passes on Windows with both IPC::Run 20260402.0 and IPC::Run 20231003.0. --- src/bin/pg_dump/t/010_dump_connstr.pl | 2 + src/bin/pg_rewind/t/RewindTest.pm | 5 +- src/interfaces/libpq/t/001_uri.pl | 4 +- .../modules/test_escape/t/001_test_escape.pl | 4 +- .../perl/PostgreSQL/Test/BackgroundPsql.pm | 6 +- src/test/perl/PostgreSQL/Test/Cluster.pm | 40 +++++++++--- src/test/perl/PostgreSQL/Test/Utils.pm | 61 +++++++++++++++---- src/test/recovery/t/021_row_visibility.pl | 8 +-- src/test/recovery/t/032_relfilenode_reuse.pl | 8 +-- 9 files changed, 101 insertions(+), 37 deletions(-) diff --git a/src/bin/pg_dump/t/010_dump_connstr.pl b/src/bin/pg_dump/t/010_dump_connstr.pl index bf2c3b6d00b..3cf54a4502f 100644 --- a/src/bin/pg_dump/t/010_dump_connstr.pl +++ b/src/bin/pg_dump/t/010_dump_connstr.pl @@ -247,6 +247,7 @@ $envar_node->run_log( local $ENV{PGPORT} = $envar_node->port; local $ENV{PGUSER} = $restore_super; $result = run_log([ 'psql', '--no-psqlrc', '--file' => $plain ], + '>' => sub { print STDOUT $_[0]; }, '2>' => \$stderr); } ok($result, @@ -286,6 +287,7 @@ $cmdline_node->run_log( '--no-psqlrc', '--file' => $plain, ], + '>' => sub { print STDOUT $_[0]; }, '2>' => \$stderr); } ok($result, diff --git a/src/bin/pg_rewind/t/RewindTest.pm b/src/bin/pg_rewind/t/RewindTest.pm index 32aeca80f13..3f3a336e684 100644 --- a/src/bin/pg_rewind/t/RewindTest.pm +++ b/src/bin/pg_rewind/t/RewindTest.pm @@ -101,9 +101,8 @@ sub check_query '--dbname' => $node_primary->connstr('postgres'), '--command' => $query ], - '>' => \$stdout, - '2>' => \$stderr; - + '>' => ipc_run_text_mode(), \$stdout, + '2>' => ipc_run_text_mode(), \$stderr; is($result, 1, "$test_name: psql exit code"); is($stderr, '', "$test_name: psql no stderr"); is($stdout, $expected_stdout, "$test_name: query result matches"); diff --git a/src/interfaces/libpq/t/001_uri.pl b/src/interfaces/libpq/t/001_uri.pl index 64f257ae046..ae1ce0c9240 100644 --- a/src/interfaces/libpq/t/001_uri.pl +++ b/src/interfaces/libpq/t/001_uri.pl @@ -269,8 +269,8 @@ sub test_uri my $cmd = [ 'libpq_uri_regress', $uri ]; $result{exit} = IPC::Run::run $cmd, - '>' => \$result{stdout}, - '2>' => \$result{stderr}; + '>' => ipc_run_text_mode(), \$result{stdout}, + '2>' => ipc_run_text_mode(), \$result{stderr}; chomp($result{stdout}); chomp($result{stderr}); diff --git a/src/test/modules/test_escape/t/001_test_escape.pl b/src/test/modules/test_escape/t/001_test_escape.pl index 3c6c968c07b..ea91a62e7ef 100644 --- a/src/test/modules/test_escape/t/001_test_escape.pl +++ b/src/test/modules/test_escape/t/001_test_escape.pl @@ -20,7 +20,9 @@ my $cmd = # There currently is no good other way to transport test results from a C # program that requires just the node being set-up... my ($stderr, $stdout); -my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr; +my $result = IPC::Run::run $cmd, + '>' => ipc_run_text_mode(), \$stdout, + '2>' => ipc_run_text_mode(), \$stderr; is($result, 1, "test_escape returns 0"); is($stderr, '', "test_escape stderr is empty"); diff --git a/src/test/perl/PostgreSQL/Test/BackgroundPsql.pm b/src/test/perl/PostgreSQL/Test/BackgroundPsql.pm index d7797225451..f4908eaac80 100644 --- a/src/test/perl/PostgreSQL/Test/BackgroundPsql.pm +++ b/src/test/perl/PostgreSQL/Test/BackgroundPsql.pm @@ -59,7 +59,7 @@ use warnings FATAL => 'all'; use Carp; use Config; use IPC::Run; -use PostgreSQL::Test::Utils qw(pump_until); +use PostgreSQL::Test::Utils qw(ipc_run_text_mode pump_until); use Test::More; =pod @@ -117,8 +117,8 @@ sub new { $run = IPC::Run::start $psql_params, '<' => \$psql->{stdin}, - '>' => \$psql->{stdout}, - '2>' => \$psql->{stderr}, + '>' => ipc_run_text_mode(), \$psql->{stdout}, + '2>' => ipc_run_text_mode(), \$psql->{stderr}, $psql->{timeout}; } diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 920d831be9e..02644d2d437 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -2271,8 +2271,30 @@ sub psql local $@; eval { my @ipcrun_opts = (\@psql_params, '<' => \$sql); - push @ipcrun_opts, '>' => $stdout if defined $stdout; - push @ipcrun_opts, '2>' => $stderr if defined $stderr; + + # psql output is text. Request text mode explicitly rather than + # relying on IPC::Run's platform-specific default. + if (defined $stdout) + { + push @ipcrun_opts, '>' => PostgreSQL::Test::Utils::ipc_run_text_mode(), $stdout; + } + elsif ($PostgreSQL::Test::Utils::windows_os) + { + # Preserve pass-through behavior explicitly on Windows. + push @ipcrun_opts, '>' => PostgreSQL::Test::Utils::ipc_run_text_mode(), + sub { print STDOUT $_[0]; }; + } + + if (defined $stderr) + { + push @ipcrun_opts, '2>' => PostgreSQL::Test::Utils::ipc_run_text_mode(), $stderr; + } + elsif ($PostgreSQL::Test::Utils::windows_os) + { + push @ipcrun_opts, '2>' => PostgreSQL::Test::Utils::ipc_run_text_mode(), + sub { print STDERR $_[0]; }; + } + push @ipcrun_opts, $timeout if defined $timeout; IPC::Run::run @ipcrun_opts; @@ -2789,8 +2811,8 @@ sub poll_query_until { my $result = IPC::Run::run $cmd, '<' => \$query, - '>' => \$stdout, - '2>' => \$stderr; + '>' => PostgreSQL::Test::Utils::ipc_run_text_mode(), \$stdout, + '2>' => PostgreSQL::Test::Utils::ipc_run_text_mode(), \$stderr; chomp($stdout); chomp($stderr); @@ -3811,7 +3833,11 @@ sub pg_recvlogical_upto { local $@; eval { - IPC::Run::run(\@cmd, '>' => \$stdout, '2>' => \$stderr, $timeout); + IPC::Run::run( + \@cmd, + '>' => PostgreSQL::Test::Utils::ipc_run_text_mode(), \$stdout, + '2>' => PostgreSQL::Test::Utils::ipc_run_text_mode(), \$stderr, + $timeout); $ret = $?; }; my $exc_save = $@; @@ -3931,8 +3957,8 @@ sub create_logical_slot_on_standby '--slot' => $slot_name, '--create-slot' ], - '>' => \$stdout, - '2>' => \$stderr); + '>' => PostgreSQL::Test::Utils::ipc_run_text_mode(), \$stdout, + '2>' => PostgreSQL::Test::Utils::ipc_run_text_mode(), \$stderr); # Arrange for the xl_running_xacts record for which pg_recvlogical is # waiting. diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm index d3e6abf7a68..0a7a30517ac 100644 --- a/src/test/perl/PostgreSQL/Test/Utils.pm +++ b/src/test/perl/PostgreSQL/Test/Utils.pm @@ -83,6 +83,7 @@ our @EXPORT = qw( scan_server_header system_or_bail system_log + ipc_run_text_mode run_log run_command pump_until @@ -421,6 +422,21 @@ sub system_or_bail =pod +=item ipc_run_text_mode() + +Return a filter suitable for C redirections that requests text mode. +Use this for command output that TAP tests treat as text rather than relying +on IPC::Run's platform-specific default. + +=cut + +sub ipc_run_text_mode +{ + return IPC::Run::binary(0); +} + +=pod + =item run_log(@cmd) Run the given command via C, noting it in the log. @@ -448,7 +464,9 @@ sub run_command { my ($cmd) = @_; my ($stdout, $stderr); - my $result = IPC::Run::run $cmd, '>' => \$stdout, '2>' => \$stderr; + my $result = IPC::Run::run $cmd, + '>' => ipc_run_text_mode(), \$stdout, + '2>' => ipc_run_text_mode(), \$stderr; chomp($stdout); chomp($stderr); return ($stdout, $stderr); @@ -1009,8 +1027,12 @@ sub command_ok local $Test::Builder::Level = $Test::Builder::Level + 1; my ($cmd, $test_name) = @_; my ($stdout, $stderr); + my $stdin = ''; print("# Running: " . join(" ", @{$cmd}) . "\n"); - my $result = IPC::Run::run $cmd, '>' => \$stdout, '2>' => \$stderr; + my $result = IPC::Run::run $cmd, + '<' => \$stdin, + '>' => ipc_run_text_mode(), \$stdout, + '2>' => ipc_run_text_mode(), \$stderr; ok($result, $test_name) or do { diag("---------- command failed ----------"); @@ -1033,7 +1055,9 @@ sub command_fails my ($cmd, $test_name) = @_; my ($stdout, $stderr); print("# Running: " . join(" ", @{$cmd}) . "\n"); - my $result = IPC::Run::run $cmd, '>' => \$stdout, '2>' => \$stderr; + my $result = IPC::Run::run $cmd, + '>' => ipc_run_text_mode(), \$stdout, + '2>' => ipc_run_text_mode(), \$stderr; ok(!$result, $test_name) or do { diag("-- command succeeded unexpectedly --"); @@ -1084,8 +1108,8 @@ sub program_help_ok my ($stdout, $stderr); print("# Running: $cmd --help\n"); my $result = IPC::Run::run [ $cmd, '--help' ], - '>' => \$stdout, - '2>' => \$stderr; + '>' => ipc_run_text_mode(), \$stdout, + '2>' => ipc_run_text_mode(), \$stderr; ok($result, "$cmd --help exit code 0"); isnt($stdout, '', "$cmd --help goes to stdout"); is($stderr, '', "$cmd --help nothing to stderr"); @@ -1116,8 +1140,8 @@ sub program_version_ok my ($stdout, $stderr); print("# Running: $cmd --version\n"); my $result = IPC::Run::run [ $cmd, '--version' ], - '>' => \$stdout, - '2>' => \$stderr; + '>' => ipc_run_text_mode(), \$stdout, + '2>' => ipc_run_text_mode(), \$stderr; ok($result, "$cmd --version exit code 0"); isnt($stdout, '', "$cmd --version goes to stdout"); is($stderr, '', "$cmd --version nothing to stderr"); @@ -1140,8 +1164,8 @@ sub program_options_handling_ok my ($stdout, $stderr); print("# Running: $cmd --not-a-valid-option\n"); my $result = IPC::Run::run [ $cmd, '--not-a-valid-option' ], - '>' => \$stdout, - '2>' => \$stderr; + '>' => ipc_run_text_mode(), \$stdout, + '2>' => ipc_run_text_mode(), \$stderr; ok(!$result, "$cmd with invalid option nonzero exit code"); isnt($stderr, '', "$cmd with invalid option prints error message"); return; @@ -1162,7 +1186,9 @@ sub command_like my ($cmd, $expected_stdout, $test_name) = @_; my ($stdout, $stderr); print("# Running: " . join(" ", @{$cmd}) . "\n"); - my $result = IPC::Run::run $cmd, '>' => \$stdout, '2>' => \$stderr; + my $result = IPC::Run::run $cmd, + '>' => ipc_run_text_mode(), \$stdout, + '2>' => ipc_run_text_mode(), \$stderr; ok($result, "$test_name: exit code 0"); is($stderr, '', "$test_name: no stderr"); like($stdout, $expected_stdout, "$test_name: matches"); @@ -1215,7 +1241,9 @@ sub command_fails_like my ($cmd, $expected_stderr, $test_name) = @_; my ($stdout, $stderr); print("# Running: " . join(" ", @{$cmd}) . "\n"); - my $result = IPC::Run::run $cmd, '>' => \$stdout, '2>' => \$stderr; + my $result = IPC::Run::run $cmd, + '>' => ipc_run_text_mode(), \$stdout, + '2>' => ipc_run_text_mode(), \$stderr; ok(!$result, "$test_name: exit code not 0"); like($stderr, $expected_stderr, "$test_name: matches"); return; @@ -1235,8 +1263,12 @@ sub command_ok_or_fails_like local $Test::Builder::Level = $Test::Builder::Level + 1; my ($cmd, $expected_stdout, $expected_stderr, $test_name) = @_; my ($stdout, $stderr); + my $stdin = ''; print("# Running: " . join(" ", @{$cmd}) . "\n"); - my $result = IPC::Run::run $cmd, '>' => \$stdout, '2>' => \$stderr; + my $result = IPC::Run::run $cmd, + '<' => \$stdin, + '>' => ipc_run_text_mode(), \$stdout, + '2>' => ipc_run_text_mode(), \$stderr; if (!$result) { like($stdout, $expected_stdout, "$test_name: stdout matches"); @@ -1277,7 +1309,10 @@ sub command_checks_all # run command my ($stdout, $stderr); print("# Running: " . join(" ", @{$cmd}) . "\n"); - IPC::Run::run($cmd, '>' => \$stdout, '2>' => \$stderr); + IPC::Run::run( + $cmd, + '>' => ipc_run_text_mode(), \$stdout, + '2>' => ipc_run_text_mode(), \$stderr); # See http://perldoc.perl.org/perlvar.html#%24CHILD_ERROR my $ret = $?; diff --git a/src/test/recovery/t/021_row_visibility.pl b/src/test/recovery/t/021_row_visibility.pl index 0a4d22b3698..f9ee5440222 100644 --- a/src/test/recovery/t/021_row_visibility.pl +++ b/src/test/recovery/t/021_row_visibility.pl @@ -44,8 +44,8 @@ $psql_primary{run} = IPC::Run::start( '--dbname' => $node_primary->connstr('postgres'), ], '<' => \$psql_primary{stdin}, - '>' => \$psql_primary{stdout}, - '2>' => \$psql_primary{stderr}, + '>' => ipc_run_text_mode(), \$psql_primary{stdout}, + '2>' => ipc_run_text_mode(), \$psql_primary{stderr}, $psql_timeout); my %psql_standby = ('stdin' => '', 'stdout' => '', 'stderr' => ''); @@ -56,8 +56,8 @@ $psql_standby{run} = IPC::Run::start( '--dbname' => $node_standby->connstr('postgres'), ], '<' => \$psql_standby{stdin}, - '>' => \$psql_standby{stdout}, - '2>' => \$psql_standby{stderr}, + '>' => ipc_run_text_mode(), \$psql_standby{stdout}, + '2>' => ipc_run_text_mode(), \$psql_standby{stderr}, $psql_timeout); # diff --git a/src/test/recovery/t/032_relfilenode_reuse.pl b/src/test/recovery/t/032_relfilenode_reuse.pl index d9e22e9bcaa..22277541bb2 100644 --- a/src/test/recovery/t/032_relfilenode_reuse.pl +++ b/src/test/recovery/t/032_relfilenode_reuse.pl @@ -42,8 +42,8 @@ $psql_primary{run} = IPC::Run::start( '--dbname' => $node_primary->connstr('postgres') ], '<' => \$psql_primary{stdin}, - '>' => \$psql_primary{stdout}, - '2>' => \$psql_primary{stderr}, + '>' => ipc_run_text_mode(), \$psql_primary{stdout}, + '2>' => ipc_run_text_mode(), \$psql_primary{stderr}, $psql_timeout); my %psql_standby = ('stdin' => '', 'stdout' => '', 'stderr' => ''); @@ -54,8 +54,8 @@ $psql_standby{run} = IPC::Run::start( '--dbname' => $node_standby->connstr('postgres') ], '<' => \$psql_standby{stdin}, - '>' => \$psql_standby{stdout}, - '2>' => \$psql_standby{stderr}, + '>' => ipc_run_text_mode(), \$psql_standby{stdout}, + '2>' => ipc_run_text_mode(), \$psql_standby{stderr}, $psql_timeout); -- 2.25.1