From: Michael Paquier <michael@paquier.xyz>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-committers@lists.postgresql.org, Andrew Dunstan <andrew@dunslane.net>
Subject: Re: pgsql: Support "postgres -C" with runtime-computed GUCs
Date: Fri, 17 Sep 2021 09:44:57 +0900
Message-ID: <YUPlCVfPi1urH/iN@paquier.xyz> (raw)
In-Reply-To: <YUPOfPqqbvuZeouS@paquier.xyz>
References: <E1mQglq-0007xB-Jw@gemulon.postgresql.org>
<1252480.1631829409@sss.pgh.pa.us>
<YUPOfPqqbvuZeouS@paquier.xyz>
On Fri, Sep 17, 2021 at 08:08:44AM +0900, Michael Paquier wrote:
> I am not completely sure what's going on here yet, so I'll just switch
> the test to be skipped when Msys is involved for now. That should be
> enough to bring back those machines to green.
So, after drinking down a coffee, I have remembered the difference
between the Msys and native perls when it comes to IPC::Run:
- Msys perl generates \r\n.
- Native perl generates \n.
We have already a couple of places where we filter that out, like
PostgresNode::psql or slurp_file. But we are missing some spots
before calling like() for outputs generated by IPC::Run. I have
tracked all those places with the attached, and I think that this
should take care of the failure seen here while preventing future
problems.
Any thoughts?
--
Michael
diff --git a/src/bin/pg_checksums/t/002_actions.pl b/src/bin/pg_checksums/t/002_actions.plindex 97492caab4..2cc8b68c75 100644--- a/src/bin/pg_checksums/t/002_actions.pl+++ b/src/bin/pg_checksums/t/002_actions.pl@@ -6,7 +6,6 @@
use strict;
use warnings;
-use Config;
use PostgresNode;
use TestLib;
@@ -182,18 +181,13 @@ command_fails(
# Test postgres -C for an offline cluster.
# Run-time GUCs are safe to query here. Note that a lock file is created,
# then unlinked, leading to an extra LOG entry showing in stderr.
-SKIP:-{- skip "unstable output generated with Msys", 3- if ($Config{osname} eq 'msys');- command_checks_all(- [ 'postgres', '-D', $pgdata, '-C', 'data_checksums' ],- 0,- [qr/^on$/],- # LOG entry when unlinking lock file.- [qr/database system is shut down/],- 'data_checksums=on is reported on an offline cluster');-}+command_checks_all(+ [ 'postgres', '-D', $pgdata, '-C', 'data_checksums' ],+ 0,+ [qr/^on$/],+ # LOG entry when unlinking lock file.+ [qr/database system is shut down/],+ 'data_checksums=on is reported on an offline cluster');
# Checks cannot happen with an online cluster
$node->start;
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pmindex cbab1587cc..bde429e476 100644--- a/src/test/perl/TestLib.pm+++ b/src/test/perl/TestLib.pm@@ -434,6 +434,8 @@ sub run_command
my ($cmd) = @_;
my ($stdout, $stderr);
my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
+ $stdout =~ s/\r\n/\n/g if $Config{osname} eq 'msys';+ $stderr =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
chomp($stdout);
chomp($stderr);
return ($stdout, $stderr);
@@ -878,6 +880,7 @@ sub command_like
my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
ok($result, "$test_name: exit code 0");
is($stderr, '', "$test_name: no stderr");
+ $stdout =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
like($stdout, $expected_stdout, "$test_name: matches");
return;
}
@@ -909,6 +912,7 @@ sub command_like_safe
$stderr = slurp_file($stderrfile);
ok($result, "$test_name: exit code 0");
is($stderr, '', "$test_name: no stderr");
+ $stdout =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
like($stdout, $expected_stdout, "$test_name: matches");
return;
}
@@ -930,6 +934,7 @@ sub command_fails_like
print("# Running: " . join(" ", @{$cmd}) . "\n");
my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
ok(!$result, "$test_name: exit code not 0");
+ $stderr =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
like($stderr, $expected_stderr, "$test_name: matches");
return;
}
@@ -981,12 +986,14 @@ sub command_checks_all
# check stdout
for my $re (@$out)
{
+ $stdout =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
like($stdout, $re, "$test_name stdout /$re/");
}
# check stderr
for my $re (@$err)
{
+ $stderr =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
like($stderr, $re, "$test_name stderr /$re/");
}
Attachments:fix-tap-msys-crlf.patch (text/plain, 2.9K) downloaddiff --git a/src/bin/pg_checksums/t/002_actions.pl b/src/bin/pg_checksums/t/002_actions.plindex 97492caab4..2cc8b68c75 100644--- a/src/bin/pg_checksums/t/002_actions.pl+++ b/src/bin/pg_checksums/t/002_actions.pl@@ -6,7 +6,6 @@
use strict;
use warnings;
-use Config;
use PostgresNode;
use TestLib;
@@ -182,18 +181,13 @@ command_fails(
# Test postgres -C for an offline cluster.
# Run-time GUCs are safe to query here. Note that a lock file is created,
# then unlinked, leading to an extra LOG entry showing in stderr.
-SKIP:-{- skip "unstable output generated with Msys", 3- if ($Config{osname} eq 'msys');- command_checks_all(- [ 'postgres', '-D', $pgdata, '-C', 'data_checksums' ],- 0,- [qr/^on$/],- # LOG entry when unlinking lock file.- [qr/database system is shut down/],- 'data_checksums=on is reported on an offline cluster');-}+command_checks_all(+ [ 'postgres', '-D', $pgdata, '-C', 'data_checksums' ],+ 0,+ [qr/^on$/],+ # LOG entry when unlinking lock file.+ [qr/database system is shut down/],+ 'data_checksums=on is reported on an offline cluster');
# Checks cannot happen with an online cluster
$node->start;
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pmindex cbab1587cc..bde429e476 100644--- a/src/test/perl/TestLib.pm+++ b/src/test/perl/TestLib.pm@@ -434,6 +434,8 @@ sub run_command
my ($cmd) = @_;
my ($stdout, $stderr);
my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
+ $stdout =~ s/\r\n/\n/g if $Config{osname} eq 'msys';+ $stderr =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
chomp($stdout);
chomp($stderr);
return ($stdout, $stderr);
@@ -878,6 +880,7 @@ sub command_like
my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
ok($result, "$test_name: exit code 0");
is($stderr, '', "$test_name: no stderr");
+ $stdout =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
like($stdout, $expected_stdout, "$test_name: matches");
return;
}
@@ -909,6 +912,7 @@ sub command_like_safe
$stderr = slurp_file($stderrfile);
ok($result, "$test_name: exit code 0");
is($stderr, '', "$test_name: no stderr");
+ $stdout =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
like($stdout, $expected_stdout, "$test_name: matches");
return;
}
@@ -930,6 +934,7 @@ sub command_fails_like
print("# Running: " . join(" ", @{$cmd}) . "\n");
my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
ok(!$result, "$test_name: exit code not 0");
+ $stderr =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
like($stderr, $expected_stderr, "$test_name: matches");
return;
}
@@ -981,12 +986,14 @@ sub command_checks_all
# check stdout
for my $re (@$out)
{
+ $stdout =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
like($stdout, $re, "$test_name stdout /$re/");
}
# check stderr
for my $re (@$err)
{
+ $stderr =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
like($stderr, $re, "$test_name stderr /$re/");
}
signature.asc (application/pgp-signature, 832B) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: michael@paquier.xyz, tgl@sss.pgh.pa.us, andrew@dunslane.net
Subject: Re: pgsql: Support "postgres -C" with runtime-computed GUCs
In-Reply-To: <YUPlCVfPi1urH/iN@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox