On Mon, Jul 23, 2018 at 07:47:44AM -0400, Fabien COELHO wrote:
>> I don't think that it is a bad idea to improve things the way you are
>
> For the record, this is not my patch, I'm merely reviewing it.
Of course, any input is welcome. It is nice to see that you took some
time to look at the patch.
>> doing, however you should extend program_version_ok() and
>> program_help_ok() in src/test/perl/TestLib.pm so as short options are
>> tested for two reasons:
>
> Interesting, I did not notice these functions before. I fully agree that
> manual testing is a pain for such a simple change.
>
> Do you mean something like the attached?
You basically have the idea, except that the number of tests in any TAP
files calling program_help_ok and program_version_ok needs to be
updated, and that the test is too verbose :)
> # Version
> -pgbench('-V', 0, [qr{^pgbench .PostgreSQL. }], [qr{^$}], 'pgbench version');
> +pgbench('-V', 0, [qr{^pgbench \(PostgreSQL\) \d+}], [qr{^$}], 'pgbench version');
This could go away.
> + is($stdout, $stdout2, "$cmd --help and -? have same output");
> + like($stdout, qr{Usage:}, "$cmd --help is about usage");
> + like($stdout, qr{\b$cmd\b}, "$cmd --help is about $cmd");
> + like($stdout, qr{--help}, "$cmd --help is about option --help");
> + like($stdout, qr{-\?}, "$cmd --help is about options -?");
> + like($stdout, qr{--version}, "$cmd --help is about options --version");
> + like($stdout, qr{-V}, "$cmd --help is about options -V");
I would keep things a bit more simple by not parsing the output as you
do, and it would be possible to reduce that to one single test with a
text block as well, say using qq().
Andrei, could you update your patch in this area please? That will ease
reviews and checks. Double-checking that the documentation gets the
correct call would not hurt as well.
--
Michael
