Re: TAP output format in pg_regress

Hi,

On 2022-07-05 00:06:04 +0200, Daniel Gustafsson wrote:
> > On 4 Jul 2022, at 16:27, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> wrote:
> >
> > On 29.06.22 21:50, Daniel Gustafsson wrote:
> >> Attached is a new version of this patch, which completes the TAP output format
> >> option such that all codepaths emitting output are TAP compliant. The verbose
> >> option is fixed to to not output extraneous newlines which the previous PoC
> >> did. The output it made to conform to the original TAP spec since v13/14 TAP
> >> parsers seem less common than those that can handle the original spec. Support
> >> for the new format additions should be quite simple to add should we want that.
> >> Running pg_regress --verbose should give the current format output.
> >> I did end up combining TAP and --verbose into a single patch, as the TAP format
> >> sort of depends on the verbose flag as TAP has no verbose mode. I can split it
> >> into two separate should a reviewer prefer that.
> >
> > I'm not sure what to make of all these options. I think providing a TAP output for pg_regress is a good idea. But then do we still need the old output? Is it worth maintaining two output formats that display exactly the same thing in slightly different ways?
>
> If we believe that TAP is good enough for human consumption and not just as
> input to test runners then we don't. Personally I think the traditional format
> is more pleasant to read than raw TAP output when running tests.

I think with a bit of care the tap output could be nearly the same
"quality". It might not be the absolute "purest" tap output, but who cares.

test tablespace ... ok 418 ms
parallel group (20 tests): oid char int2 varchar name int4 text pg_lsn regproc txid money boolean uuid float4 int8 float8 bit enum rangetypes numeric
boolean ... ok 34 ms
char ... ok 20 ms
name ... ok 26 ms

isn't that different from

ok 1 - tablespace 418ms
# parallel group (20 tests): oid char int2 varchar name int4 text pg_lsn regproc txid money boolean uuid float4 int8 float8 bit enum rangetypes numeric
ok 2 - boolean 34ms
ok 3 - char 20ms
ok 4 - name 26ms

or whatever.

For non-parallel tests I think we currently print the test name before running
the test, which obviously doesn't work well when needing to print the 'ok'
'not ok' first. We could just print
# non-parallel group tablespace
or such? That doesn't

I wonder if for parallel tests we should print the test number based on the
start of the test rather than the finish time? Hm, it also looks like it's
legal to just leave the test number out?

> The discussion on this was in 20220221164736(dot)rq3ornzjdkmwk2wo(at)alap3(dot)anarazel(dot)de
> where it was proposed that we could cut the boilerplate.

I think that was more about things like CREATE DATABASE etc. And I'm not sure
we need an option to keep showing those details.

> > I remember the last time I wanted to tweak the output of the parallel tests, people were very attached to the particular timing and spacing of the current output. So I'm not sure people will like this.
> >
> > The timing output is very popular. Where is that in the TAP output?
>
> As I mentioned in the mail upthread, TAP runners generally hide that info and
> only show a running total. That being said, I do agree with adding back so
> I'll do that in a new version of the patch.

FWIW, meson's testrunner shows the individual tap output when using -v.

One test where using -v is a problem is pg_dump's tests - the absurd number of
8169 tests makes showing that decidedly not fun.

> Having test output format parity with supported back branches seemed like a
> good idea to me at the time of writing at least.

That's of course nice, but it also kind of corners us into not evolving the
default format, which I don't think is warranted...

Greetings,

Andres Freund