Re: TAP output format in pg_regress

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: TAP output format in pg_regress
Date: 2022-08-18 11:24:20
Message-ID: 04784F1A-1EE1-48C9-B443-0ACC32D16F3A@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 18 Aug 2022, at 00:49, Andres Freund <andres(at)anarazel(dot)de> wrote:

>> while still be TAP compliant enough that running it with prove (with a tiny
>> Perl wrapper) works.
>
> Wonder if we could utilize that for making failures of 002_pg_upgrade.pl and
> 027_stream_regress.pl easier to understand, by reporting pg_regress' steps as
> part of the outer test. But that'd probably be at least somewhat hard, due to
> the embedded test count etc.

I have a feeling it might require some quite bespoke logic to tie it together
(which may not be worth it in the end) but I'll have a look.

>> A normal "make check" with this patch applied now looks like this:
>>
>> ...
>
> I'm happy with that compared to our current output.

Great! Once this thread has settled on a format, maybe it should go to a
separate -hackers thread to get more visibility and (hopefully) buy-in for such
a change.

One thing I haven't researched yet is if the Buildfarm or CFBot parses the
current output in any way or just check the exit status of the testrun.

>> The ending comment isn't currently shown when executed via prove as it has to
>> go to STDERR for that to work, and I'm not sure that's something we want to do
>> in the general case. I don't expect running the pg_regress tests via prove is
>> going to be the common case. How does the meson TAP ingestion handle
>> stdout/stderr?
>
> It'll parse stdout for tap output and log stderr output separately.

Then I think this patch will be compatible with that.

>> There is a slight reduction in information, for example around tests run
>> serially vs in a parallel group. This could perhaps be handled by marking the
>> test name in some way like "tablespace (serial) or prefixing with a symbol or
>> somerthing. I can take a stab at that in case we think that level of detail is
>> important to preserve.
>
> I think we could just indent the test "description" for tests in parallel
> groups:
>
> I.e. instead of
>
> ok 6 - varchar 68 ms
> ...
> ok 6 - varchar 68 ms
>
> that'd make it sufficiently clear, and is a bit more similar to the current
> format?

Thats a better option, done that way.

> I wonder if we should indent the test name so that the number doesn't cause
> wrapping?

The tricky part there is that we don't know beforehands how many tests will be
run. We could add a first pass over the schedule, which seems excessive for
this, or align with a fixed max such that 9999 number of tests is the maximum
number of tests which will print neatly aligned.

> And perhaps we can skip the - in that case?

The dash is listed as "Recommended" but not required in the TAP spec (including
up to v14) so we can skip it.

With these changes, the "worst case" output in terms of testnames alignment
would be something like this (assuming at most 9999 tests):

ok 211 stats 852 ms
# parallel group (2 tests): event_trigger oidjoins
ok 212 event_trigger 149 ms
not ok 213 oidjoins 194 ms
ok 214 fast_default 178 ms
1..214

I think that's fairly readable, and not that much different from today.

--
Daniel Gustafsson https://vmware.com/

Attachment Content-Type Size
v7-0001-Make-pg_regress-output-format-TAP-compliant.patch application/octet-stream 32.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavan Deolasee 2022-08-18 11:28:24 Re: Assertion failure on PG15 with modified test_shm_mq test
Previous Message Anant ngo 2022-08-18 10:42:45 Data caching