Re: TAP output format in pg_regress

From: Nikolay Shaplov <dhyan(at)nataraj(dot)su>
To: Daniel Gustafsson <daniel(at)yesql(dot)se>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, 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>, Andres Freund <andres(at)anarazel(dot)de>
Subject: Re: TAP output format in pg_regress
Date: 2022-11-10 10:44:54
Message-ID: 3651868.BXJB8xsqyR@thinkpad-pgpro
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi! Do this patch still need reviewer, or reviewer field in commit-fest have
been left empty by mistake?

I am fan of TAP-tests, so I am quite happy that pg_regress output changed to
TAP tests...

I've checked new output, if is conform TAP specification. Checked that prove
consumes new pg_regress output well.

Did not found quick way to include prove TAP harness right into Makefile, so I
check dumped output, but it is not really important for now, I guess.

As for the code, I gave it a quick readthrough... And main issue I've stumbled
was here:

--------------
indent = 36 - (parallel ? 8 : 0) - floor(log10(testnumber) + 1);

status("ok %-5i %s%-*s %8.0f ms",
testnumber, (parallel ? " " : ""), indent,
testname,
runtime);
--------------

This peace of code has non-obvious logic (I can solve it without taking time
for deep thinking) and this code (almost similar) is repeated three times.
This is hard to support, and error prone solution.

I would suggest to add one _print_test_status function, that also accepts
status string and do this complex calculations and formatting only once (
"# SKIP (ignored)" should also added as %s, so we have only one print with
complex format string).

Then you will have to read check and fix it only once.

And may be it would be good to make this calculations more human-readable...

If this patch really needs reviewer and my way of thinking is acceptable,
please let me know, I will set myself as a reviewer and will dig further into
the code.

--
Nikolay Shaplov aka Nataraj
Fuzzing Engineer at Postgres Professional
Matrix IM: @dhyan:nataraj.su

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2022-11-10 11:17:57 Re: New docs chapter on Transaction Management and related changes
Previous Message sirisha chamarthi 2022-11-10 10:42:50 Re: Reviving lost replication slots