Date: Wed, 6 Oct 2021 15:28:24 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: More business with $Test::Builder::Level in the TAP tests
Message-ID: <YV1CCFwgM1RV1LeS@paquier.xyz>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="5gCOwa0tuJ/9q6pe"
Content-Disposition: inline
Archived-At: 
 <https://www.postgresql.org/message-id/YV1CCFwgM1RV1LeS%40paquier.xyz>
Precedence: bulk


--5gCOwa0tuJ/9q6pe
Content-Type: multipart/mixed; boundary="S2IQ2NndS2qoRtpK"
Content-Disposition: inline


--S2IQ2NndS2qoRtpK
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi all,

Following up with Peter E's recent commit 73aa5e0 to add some
forgotten level incrementations, I got to look again at what I did
wrong and why this stuff is useful.

I have gone through all the TAP tests and any code paths using
subroutines, to note that we could improve the locations of the
reports we get by adding more $Test::Builder::Level.  The context is
important, as some code paths use rather-long routines and also
argument values that allow to track easily which test path is being
taken (like pg_rewind), so there is no need to add anything in such
places.  The attached patch adds incrementations for the tests where
the debugging becomes much easier if there is a failure.

Thoughts?
--
Michael

--S2IQ2NndS2qoRtpK
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="tap-builder-level.patch"
Content-Transfer-Encoding: quoted-printable

diff --git a/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl b/src/bin=
/pg_archivecleanup/t/010_pg_archivecleanup.pl
index 8134c2a62e..8d689b9601 100644
--- a/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
+++ b/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
@@ -72,6 +72,8 @@ command_fails_like(
=20
 sub run_check
 {
+	local $Test::Builder::Level =3D $Test::Builder::Level + 1;
+
 	my ($suffix, $test_name) =3D @_;
=20
 	create_files();
diff --git a/src/bin/pg_ctl/t/004_logrotate.pl b/src/bin/pg_ctl/t/004_logro=
tate.pl
index aa0d64a4f7..13e91f3bc9 100644
--- a/src/bin/pg_ctl/t/004_logrotate.pl
+++ b/src/bin/pg_ctl/t/004_logrotate.pl
@@ -31,6 +31,8 @@ sub fetch_file_name
 # Check for a pattern in the logs associated to one format.
 sub check_log_pattern
 {
+	local $Test::Builder::Level =3D $Test::Builder::Level + 1;
+
 	my $format   =3D shift;
 	my $logfiles =3D shift;
 	my $pattern  =3D shift;
diff --git a/src/bin/pg_verifybackup/t/005_bad_manifest.pl b/src/bin/pg_ver=
ifybackup/t/005_bad_manifest.pl
index 4f5b8f5a49..1420cfb352 100644
--- a/src/bin/pg_verifybackup/t/005_bad_manifest.pl
+++ b/src/bin/pg_verifybackup/t/005_bad_manifest.pl
@@ -176,6 +176,8 @@ EOM
=20
 sub test_parse_error
 {
+	local $Test::Builder::Level =3D $Test::Builder::Level + 1;
+
 	my ($test_name, $manifest_contents) =3D @_;
=20
 	test_bad_manifest($test_name,
@@ -186,6 +188,8 @@ sub test_parse_error
=20
 sub test_fatal_error
 {
+	local $Test::Builder::Level =3D $Test::Builder::Level + 1;
+
 	my ($test_name, $manifest_contents) =3D @_;
=20
 	test_bad_manifest($test_name, qr/fatal: $test_name/, $manifest_contents);
@@ -194,6 +198,8 @@ sub test_fatal_error
=20
 sub test_bad_manifest
 {
+	local $Test::Builder::Level =3D $Test::Builder::Level + 1;
+
 	my ($test_name, $regexp, $manifest_contents) =3D @_;
=20
 	open(my $fh, '>', "$tempdir/backup_manifest") || die "open: $!";
diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_=
completion.pl
index 8695d22545..dbca56afad 100644
--- a/src/bin/psql/t/010_tab_completion.pl
+++ b/src/bin/psql/t/010_tab_completion.pl
@@ -127,6 +127,8 @@ sub check_completion
 # (won't work if we are inside a string literal!)
 sub clear_query
 {
+	local $Test::Builder::Level =3D $Test::Builder::Level + 1;
+
 	check_completion("\\r\n", qr/postgres=3D# /, "\\r works");
 	return;
 }
@@ -136,6 +138,8 @@ sub clear_query
 # than clear_query because we lose evidence in the history file)
 sub clear_line
 {
+	local $Test::Builder::Level =3D $Test::Builder::Level + 1;
+
 	check_completion("\025\n", qr/postgres=3D# /, "control-U works");
 	return;
 }
diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth=
=2Epl
index c484237d07..968be3952f 100644
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -221,6 +221,8 @@ sub test_access
 # As above, but test for an arbitrary query result.
 sub test_query
 {
+	local $Test::Builder::Level =3D $Test::Builder::Level + 1;
+
 	my ($node, $role, $query, $expected, $gssencmode, $test_name) =3D @_;
=20
 	# need to connect over TCP/IP for Kerberos
diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/00=
1_stream_rep.pl
index ac581c1c07..9916a36012 100644
--- a/src/test/recovery/t/001_stream_rep.pl
+++ b/src/test/recovery/t/001_stream_rep.pl
@@ -75,6 +75,8 @@ note "testing connection parameter \"target_session_attrs=
\"";
 # Expect to connect to $target_node (undef for failure) with given $status.
 sub test_target_session_attrs
 {
+	local $Test::Builder::Level =3D $Test::Builder::Level + 1;
+
 	my $node1       =3D shift;
 	my $node2       =3D shift;
 	my $target_node =3D shift;
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recover=
y/t/003_recovery_targets.pl
index 7bd500ed95..78ef60d3b2 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -14,6 +14,8 @@ use Time::HiRes qw(usleep);
 # count to reach $num_rows, yet not later than the recovery target.
 sub test_recovery_standby
 {
+	local $Test::Builder::Level =3D $Test::Builder::Level + 1;
+
 	my $test_name       =3D shift;
 	my $node_name       =3D shift;
 	my $node_primary    =3D shift;
diff --git a/src/test/recovery/t/007_sync_rep.pl b/src/test/recovery/t/007_=
sync_rep.pl
index 9d00e17f9f..3b031addf7 100644
--- a/src/test/recovery/t/007_sync_rep.pl
+++ b/src/test/recovery/t/007_sync_rep.pl
@@ -17,6 +17,8 @@ my $check_sql =3D
 # the configuration file is reloaded before the test.
 sub test_sync_state
 {
+	local $Test::Builder::Level =3D $Test::Builder::Level + 1;
+
 	my ($self, $expected, $msg, $setting) =3D @_;
=20
 	if (defined($setting))
diff --git a/src/test/recovery/t/009_twophase.pl b/src/test/recovery/t/009_=
twophase.pl
index 78d4ef5b54..66a256208c 100644
--- a/src/test/recovery/t/009_twophase.pl
+++ b/src/test/recovery/t/009_twophase.pl
@@ -14,6 +14,8 @@ my $psql_rc  =3D '';
=20
 sub configure_and_reload
 {
+	local $Test::Builder::Level =3D $Test::Builder::Level + 1;
+
 	my ($node, $parameter) =3D @_;
 	my $name =3D $node->name;
=20
diff --git a/src/test/recovery/t/018_wal_optimize.pl b/src/test/recovery/t/=
018_wal_optimize.pl
index 47cbc95955..3bedeffcae 100644
--- a/src/test/recovery/t/018_wal_optimize.pl
+++ b/src/test/recovery/t/018_wal_optimize.pl
@@ -18,6 +18,8 @@ use Test::More tests =3D> 38;
=20
 sub check_orphan_relfilenodes
 {
+	local $Test::Builder::Level =3D $Test::Builder::Level + 1;
+
 	my ($node, $test_name) =3D @_;
=20
 	my $db_oid =3D $node->safe_psql('postgres',
diff --git a/contrib/amcheck/t/001_verify_heapam.pl b/contrib/amcheck/t/001=
_verify_heapam.pl
index ba40f64b58..8e02a8db2a 100644
--- a/contrib/amcheck/t/001_verify_heapam.pl
+++ b/contrib/amcheck/t/001_verify_heapam.pl
@@ -209,6 +209,8 @@ sub corrupt_first_page
=20
 sub detects_heap_corruption
 {
+	local $Test::Builder::Level =3D $Test::Builder::Level + 1;
+
 	my ($function, $testname) =3D @_;
=20
 	detects_corruption(
@@ -224,6 +226,8 @@ sub detects_heap_corruption
=20
 sub detects_corruption
 {
+	local $Test::Builder::Level =3D $Test::Builder::Level + 1;
+
 	my ($function, $testname, @re) =3D @_;
=20
 	my $result =3D $node->safe_psql('postgres', qq(SELECT * FROM $function));
@@ -232,6 +236,8 @@ sub detects_corruption
=20
 sub detects_no_corruption
 {
+	local $Test::Builder::Level =3D $Test::Builder::Level + 1;
+
 	my ($function, $testname) =3D @_;
=20
 	my $result =3D $node->safe_psql('postgres', qq(SELECT * FROM $function));
@@ -247,6 +253,8 @@ sub detects_no_corruption
 # and should be unique.
 sub check_all_options_uncorrupted
 {
+	local $Test::Builder::Level =3D $Test::Builder::Level + 1;
+
 	my ($relname, $prefix) =3D @_;
=20
 	for my $stop (qw(true false))
diff --git a/contrib/test_decoding/t/001_repl_stats.pl b/contrib/test_decod=
ing/t/001_repl_stats.pl
index fdef6cb1ff..e8644e1cbc 100644
--- a/contrib/test_decoding/t/001_repl_stats.pl
+++ b/contrib/test_decoding/t/001_repl_stats.pl
@@ -19,6 +19,8 @@ $node->start;
 # Check that replication slot stats are expected.
 sub test_slot_stats
 {
+	local $Test::Builder::Level =3D $Test::Builder::Level + 1;
+
 	my ($node, $expected, $msg) =3D @_;
=20
 	my $result =3D $node->safe_psql(

--S2IQ2NndS2qoRtpK--

--5gCOwa0tuJ/9q6pe
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmFdQgcACgkQnvQgOdby
QH0RSA//YAfd1pW3HAaetCuV05Tx8U88wSFxhy+oRuRs21hLt9TER639JWBr0qjA
UWtp4OL6iRlZwhCz8ZbAMibQ/6GH+wnv5E97EtqgEVP6OJNyWCXNoWa58tCzz50+
6X31qLhReIxNI3qwt3Ecl1VSdbK3uLpIlglVmCnLpPPOIpwRx9PJe1cDO/QwbTIa
JAN2IZQkIcJe9kkyAyPLMelyiz+d2DySsjRPsJXeUxBj8qngaKpJW90GsSB+dwj7
sbiUMgvkM2x5cWRbbt6Eetkp8LcyF9+hha83NAj2SsVUU0yyV9o0ypOWkdTfI7al
cEV/k9h0LCE+1Ap3NpODurvLkG8WMh/6N6vEZFmIsBjhucHjdV8f7dufIjQ8N0js
0Ui/+CbRaQmQx/PVd4MvafYAqANPFiPWITXDAdRQHko1euAN9fFyRFLQA90FZoC8
GrDXVL0tpsYJGkBrNmiMQfcexkZBTwtH7Dps0AW4SB0kgbPjsrVPiss+XvWJkzmC
16Q1iO9a7GqyVA+cwPUtG8zltHmN97lYk2iEDAJyWw/+ZEMt2pNOPCOFaf3y//OB
+YQ8FjxzRwTnyb50z8OIi7oGoK/gDrVzxxjJiSvlTvTftcxYfmO2VNhTjRC10RG8
FnL8b3qMt6kFZqpwibbaFgZVMy4kXVh7FpZIlH70kPy/DIwueOg=
=raK2
-----END PGP SIGNATURE-----

--5gCOwa0tuJ/9q6pe--