Re: PSA: we lack TAP test coverage on NetBSD and OpenBSD - Mailing list pgsql-hackers

Fabien COELHO <coelho@cri.ensmp.fr> writes:
>> I am, TBH, inclined to fix this by removing that test case rather
>> than teaching it another spelling to accept.  I think it's very
>> hard to make the case that tests like this one are anything but
>> a waste of developer and buildfarm time.  When they are also a
>> portability hazard, it's time to cut our losses.  (I also note
>> that this test has caused us problems before, cf 869aa40a2 and
>> 933851033.)

> I'd rather keep it by simply adding the "|unknown" alternative. 30 years 
> of programming have taught me that testing limit & error cases is useful, 
> although you never know when it will be proven so.

Sorry, I don't buy this line of argument.  Reasonable test design requires
making cost/benefit tradeoffs: the cost to run the test over and over,
and the cost to maintain the test itself (e.g. fix portability issues in
it) have to be balanced against the probability of it finding something
useful.  I judge that the chance of this particular test finding something
is small, and I've had quite enough of the maintenance costs.

Just to point up that we're still not clearly done with the maintenance
costs of supposing that we know how every version of getopt_long will
spell this error message, I note that my Linux box seems to have two
variants of it:

$ pgbench -z 
pgbench: invalid option -- 'z'
Try "pgbench --help" for more information.
$ pgbench --z
pgbench: unrecognized option '--z'
Try "pgbench --help" for more information.

of which the "invalid" alternative is also not in our list right now.
Who's to say how many more versions of getopt_long are out there,
or what the maintainers thereof might do in the future?

> I agree that some tests can be useless, but I do not think that it applies 
> to this one. This test also checks that under a bad option pgbench stops 
> with an appropriate 1 exit status.

It's possible that it's worth the trouble to check for exit status 1,
but I entirely fail to see the point of checking exactly what is the
spelling of a message that is issued by code not under our control.

Looking closer at the test case:

    [
        'bad option',
        '-h home -p 5432 -U calvin -d --bad-option',
        [ qr{(unrecognized|illegal) option}, qr{--help.*more information} ]
    ],

ISTM that just removing the first qr{} pattern, and checking only that
we get the text that *is* under our control, is a reasonable compromise
here.

            regards, tom lane