From: Nikolay Shaplov <dhyan@nataraj.su>
To: Daniel Gustafsson <daniel@yesql.se>, pgsql-hackers@lists.postgresql.org
Cc: Andrew Dunstan <andrew@dunslane.net>, Tom Lane <tgl@sss.pgh.pa.us>,
 Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
 PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>,
 Andres Freund <andres@anarazel.de>
Subject: Re: TAP output format in pg_regress
Date: Thu, 10 Nov 2022 13:44:54 +0300
Message-ID: <3651868.BXJB8xsqyR@thinkpad-pgpro>
Organization: Postgres Professional
In-Reply-To: <20221002010153.k5rvcmlqczksdujq@awork3.anarazel.de>
References: <5e6bc6a6-0a4e-109d-2b7d-c286dc58d609@enterprisedb.com>
 <664EA416-9C16-40F6-84B2-80E8542AFF43@yesql.se>
 <20221002010153.k5rvcmlqczksdujq@awork3.anarazel.de>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="nextPart10298581.XAHkF5tYk2";
 micalg="pgp-sha512"; protocol="application/pgp-signature"
Archived-At: 
 <https://www.postgresql.org/message-id/3651868.BXJB8xsqyR%40thinkpad-pgpro>
Precedence: bulk

--nextPart10298581.XAHkF5tYk2
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"; protected-headers="v1"
From: Nikolay Shaplov <dhyan@nataraj.su>
To: Daniel Gustafsson <daniel@yesql.se>, pgsql-hackers@lists.postgresql.org
Cc: Andrew Dunstan <andrew@dunslane.net>, Tom Lane <tgl@sss.pgh.pa.us>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>, Andres Freund <andres@anarazel.de>
Subject: Re: TAP output format in pg_regress
Date: Thu, 10 Nov 2022 13:44:54 +0300
Message-ID: <3651868.BXJB8xsqyR@thinkpad-pgpro>
Organization: Postgres Professional
In-Reply-To: <20221002010153.k5rvcmlqczksdujq@awork3.anarazel.de>
References: <5e6bc6a6-0a4e-109d-2b7d-c286dc58d609@enterprisedb.com> <664EA416-9C16-40F6-84B2-80E8542AFF43@yesql.se> <20221002010153.k5rvcmlqczksdujq@awork3.anarazel.de>

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

--nextPart10298581.XAHkF5tYk2
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: This is a digitally signed message part.
Content-Transfer-Encoding: 7Bit

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEE+sk3ebqQKlezKOi8PMbfuIHAGpgFAmNs1iYACgkQPMbfuIHA
Gpgh+wgAnbd42O2bk+TjWQ48hIO/m06GkQrZxdAU5tF6Ycg5YuT34KCN1cQAGYlD
LA07lt1p1BpisTbkvgGNXhHDmaFltdZeNpTy9dP0ZU0tFW2bkCgOsKQdrDzu2rSn
fG1hLQYabMHOpi2IX6eblyt+J6AisJ78lq3BbfhCXreyInNkpdqdGjZ09Bpl4hIh
d3LYVkHg4yXzdEDash4/dzlZszJ8L9ySqFY09WmQCJ+tgZIwWY9SfmwKpll5fDF9
lqM5TGONAKrE/gbL5Y1T+6xaGS0kDDFQkiVSwomUvmapScr0UjJgD9ziyBpoMsXw
iuw/QQPe2XH1rldUnJM33w0k+d0dPg==
=UHyJ
-----END PGP SIGNATURE-----

--nextPart10298581.XAHkF5tYk2--