Re: pgbench: improve --help and --version parsing

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: Andrei Korigodski <akorigod(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pgbench: improve --help and --version parsing
Date: 2018-07-24 01:55:08
Message-ID: 20180724015508.GC4035@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-07-24 01:58:03 Re: Missing pg_control crashes postmaster
Previous Message Michael Paquier 2018-07-24 01:43:08 Re: [report] memory leaks in COPY FROM on partitioned table