Re: pg_createsubscriber TAP test wrapping makes command options hard to read.

Hi Michael,

Thanks for committing!

Michael Paquier <michael(at)paquier(dot)xyz> writes:

> On Tue, Jan 21, 2025 at 02:17:03PM +0000, Dagfinn Ilmari Mannsåker wrote:
>> Thanks again for reviewing this monster patch.
>
> +$node->command_checks_all(
> + [ 'pg_amcheck', '--exclude-user' => 'no_such_user', 'postgres' ],
>
> Extra error spotted here with s/--exclude-user/--username/.

I've attached a patch to make it check that it's actually complaining
about the role not existing, not an unrecognised option.

> The double --verbose for test 'run pg_createsubscriber on node S' is
> intentional, as mentioned by Euler. Fixed this one by making the two
> --verbose be side-by-side, and added a comment to document the
> intention.

Good call.

> Perhaps pg_resetwal/t/001_basic.pl should also use more long option
> names? Note that there are a few more under "Locale provider
> tests" in the initdb tests. I've left that as a follow-up thing,
> that was already a lot..

As I mentioned elsewhere in the thread, I left those alone since the
error message uses the short spelling even if the command had the long
one. We could add the long spelling to the preceding comments, like the
second attached patch.

> Found some more inconsistencies with the ordering of the options, like
> a few start commands with pg_ctl in the recovery tests. I've
> maintained some consistency here, even if your patch was OK.

Most invocations of pg_ctl actually have the action before any options:

❯ rg --multiline -tperl '(\W)pg_ctl\1,\s*(\W)\w+\2' | rg -cw pg_ctl
23

❯ rg --multiline -tperl '(\W)pg_ctl\1,\s*(\W)--\w+\2' | rg -cw pg_ctl
11

I've attached a third patch to make them consistently have the action
first.

- ilmari