Re: PSA: we lack TAP test coverage on NetBSD and OpenBSD
Fabien COELHO <coelho@cri.ensmp.fr>
From: Fabien COELHO <coelho@cri.ensmp.fr>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2019-01-17T09:46:34Z
Lists: pgsql-hackers
Hello Tom, > Although we've got a few NetBSD and OpenBSD buildfarm critters, > none of them are doing --enable-tap-tests. If they were, we'd > have noticed the pgbench regression tests falling over: > > [...] > > 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. Client application coverage is currently abysmal, especially "psql" despite the many script used for testing (39% of lines, 42% of functions!), pgbench is under 90%. Generally we really need more tests, not less. TAP tests are a good compromise because they are not always run, and ISTM sometimes (i.e. you asked for it) is enough. 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. Recently a patch updated the exit status of pgbench in many cases to distinguish between different kind errors, thus having non-regression in this area was shown to be a good idea. Moreover, knowing that the exit status on getopt errors is consistent across platform has value, and knowing that there is some variability is not uninteresting. -- Fabien.
Commits
-
Fix portability problem in pgbench.
- e6c3ba7fbfd5 12.0 landed
- 27d6bc68f98e 11.2 landed
-
Avoid assuming that we know the spelling of getopt_long's error messages.
- 434040159558 11.2 landed
- 251f57460f12 12.0 landed