Re: Rewriting the test of pg_upgrade as a TAP test - take three - remastered set

On Wed, May 18, 2022 at 01:03:15AM -0700, Noah Misch wrote:
> On Mon, May 16, 2022 at 02:30:00PM +0900, Michael Paquier wrote:
>> 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%3A35&stg=pg_upgrade-check

Yes, I saw that. This did not bother me much as the TAP tests run in
isolation, but I am fine to stick to your option and silence these.

> 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

So you are using EXTRA_REGRESS_OPTS, then, and a space is missing from
the first argument of the command used to make that work properly.

> 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.

Okay, added this one.

>> + 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.

Hmm. The CI has been passing for me with this name pattern in place,
as of https://github.com/michaelpq/postgres/tree/upgrade_tap_fixes.

Attached is an updated patch to address your concerns.
--
Michael