Re: TAP output format in pg_regress

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, Nikolay Shaplov <dhyan(at)nataraj(dot)su>, Andres Freund <andres(at)anarazel(dot)de>, Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: TAP output format in pg_regress
Date: 2023-03-29 19:14:23
Message-ID: 424F65D5-CDB2-4120-BB59-1E562BBADD17@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 29 Mar 2023, at 09:08, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> wrote:
>
> On 28.03.23 15:56, Daniel Gustafsson wrote:
>>> On 28 Mar 2023, at 15:26, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
>>> I think the attached is a good candidate for going in, but I would like to see it
>>> for another spin in the CF bot first.
>> Another candidate due to a thinko which raised a compiler warning.
>
> This is incorrect:
>
> -echo "+++ tap install-check in $(subdir) +++" && \
> +echo "\# +++ tap install-check in $(subdir) +++" && \
>
> It actually prints the backslash.
>
> But this appears to be correct:
>
> pg_regress_check = \
> - echo "+++ regress check in $(subdir) +++" && \
> + echo "\# +++ regress check in $(subdir) +++" && \
>
> (Maybe because it's in a variable definition?)

Copy paste error, fixed.

> I'm confused why all the messages at the end lost their period ("All tests passed", "A copy of the test summary ...", etc.). As long as they are written as sentences, they should have a period.

Fixed. I also made sure that all messages printing the output directory wrap
it in quotes like how we print paths generally.

> There is a comment "Testnames are indented 8 spaces" but you made that into a macro now, which is currently defined to 3.

As explained below, this is removed. I've also removed a comment on NLS which
no longer makes sense.

> There is an unexplained #if 0.

Ugh, I clearly should've stayed on the couch yesterday.

> The additions of
>
> + free(difffilename);
> + difffilename = NULL;
> + free(logfilename);
> + logfilename = NULL;
>
> in the success case are not clear. The program is going to end soon anyway.

Fair enough, removed.

> On the indentation of the test names: If you look at the output of meson test verbose mode (e.g., meson test -C _build --suite regress --verbose), it reproduces the indentation in a weirdly backwards way, e.g.,
>
> ▶ 1/1 stats 981 ms OK
> ▶ 1/1 event_trigger 122 ms OK
> ▶ 1/1 oidjoins 172 ms OK
> ▶ 1/1 fast_default 137 ms OK
> ▶ 1/1 tablespace 285 ms OK
>
> Not sure by which logic it arrives at that, but you can clearly see 3 additional spaces for the single tests.

I'm not sure what meson does actually. It seems to strip the leading padding
and line up the testname, but then add the stripped padding on the right side?
Removing the padding for parallel tests solves it, so I have in the attached
moved to indicating parallel tests with a leading '+' and single tests with
'-'. Not sure if this is clear enough, but it's not worse than padding IMO.

> One thing I have noticed while playing around with this is that it's quite hard to tell casually whether a test run has failed or is failing, if you just keep half an eye on it in another Window. The display of "ok"/"not ok" as well as the summaries at the end are much less prominent than the previous "ok"/"FAILED" and the summary box at the end. I'm not sure what to do about that, or if it's just something to get used to.

meson already presents the results in a box, so if we bring back the === box it
gets quite verbose there. To some extent I think it is something to get used
to, but if there is discontent with what it looks like reported when more
hackers gets exposed to this we need to fix it.

--
Daniel Gustafsson

Attachment Content-Type Size
v20-0001-pg_regress-Emit-TAP-compliant-output.patch application/octet-stream 39.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2023-03-29 19:47:55 Re: Add pg_walinspect function with block info columns
Previous Message Daniel Verite 2023-03-29 18:39:00 Re: TAP tests for psql \g piped into program