public inbox for [email protected]
help / color / mirror / Atom feedFrom: Noah Misch <[email protected]>
To: Michael Paquier <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: Julien Rouhaud <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Cc: Postgres hackers <[email protected]>
Subject: Re: Rewriting the test of pg_upgrade as a TAP test - take three - remastered set
Date: Wed, 18 May 2022 01:03:15 -0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
On Mon, May 16, 2022 at 02:30:00PM +0900, Michael Paquier wrote:
> On Sat, May 14, 2022 at 01:27:28AM -0700, Noah Misch wrote:
> > Here, I requested the rationale for the differences you had just described.
> > You made a choice to stop testing one list of database names and start testing
> > a different list of database names. Why?
>
> Because the shape of the new names does not change the test coverage
> ("regression" prefix or the addition of the double quotes with
> backslashes for all the database names), while keeping the code a bit
> simpler. If you think that the older names are more adapted, I have
> no objections to use them, FWIW, which is something like the patch
> attached would achieve.
>
> This uses the same convention as vcregress.pl before 322becb, but not
> the one of test.sh where "regression" was appended to the database
> names.
I would have picked the test.sh names, both because test.sh was the senior
implementation and because doing so avoids warnings under
-DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS. See the warnings here:
https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=longfin&dt=2022-05-18%2000%3A59%3A...
More-notable line from that same log:
sh: /Users/buildfarm/bf-data/HEAD/pgsql.build/src/bin/pg_upgrade/../../../src/test/regress/pg_regress--port=5678: No such file or directory
Commit 7dd3ee5 adopted much of the 027_stream_regress.pl approach to running
pg_regress, but it didn't grab the "is($rc, 0, 'regression tests pass')"
needed to make defects like that report a failure.
> --- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
> +++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
> @@ -13,18 +13,16 @@ use Test::More;
> # Generate a database with a name made of a range of ASCII characters.
> sub generate_db
> {
> - my ($node, $from_char, $to_char) = @_;
> + my ($node, $prefix, $from_char, $to_char, $suffix) = @_;
>
> - my $dbname = '';
> + my $dbname = $prefix;
> for my $i ($from_char .. $to_char)
> {
> next if $i == 7 || $i == 10 || $i == 13; # skip BEL, LF, and CR
> $dbname = $dbname . sprintf('%c', $i);
> }
>
> - # Exercise backslashes adjacent to double quotes, a Windows special
> - # case.
> - $dbname = '\\"\\' . $dbname . '\\\\"\\\\\\';
> + $dbname .= $suffix;
> $node->command_ok([ 'createdb', $dbname ]);
> }
>
> @@ -79,10 +77,12 @@ else
> {
> # Default is to use pg_regress to set up the old instance.
>
> - # Create databases with names covering most ASCII bytes
> - generate_db($oldnode, 1, 45);
> - generate_db($oldnode, 46, 90);
> - generate_db($oldnode, 91, 127);
> + # Create databases with names covering most ASCII bytes. The
> + # first name exercises backslashes adjacent to double quotes, a
> + # Windows special case.
> + generate_db($oldnode, "\\\"\\", 1, 45, "\\\\\"\\\\\\");
> + generate_db($oldnode, '', 46, 90, '');
> + generate_db($oldnode, '', 91, 127, '');
Does this pass on Windows? I'm 65% confident that released IPC::Run can't
handle this input due to https://github.com/toddr/IPC-Run/issues/142. If it's
passing for you on Windows, then disregard.
view thread (122+ messages) latest in thread
reply
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Rewriting the test of pg_upgrade as a TAP test - take three - remastered set
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox