Subject: Readd use of TAP subtests

From: Peter Eisentraut <[email-masked]>
Date: Wed, 8 Dec 2021 14:26:35 +0100

Now that subtests in TAP are supported again, I want to correct the 
great historical injustice of 7912f9b7dc9e2d3f6cd81892ef6aa797578e9f06 
and put those subtests back.

Much more work like this is possible, of course.  I just wanted to get 
this out of the way since the code was already written and I've had this 
on my list for, uh, 7 years.

From d1e2df592b7eaa7952d621266d5ad5d6b80e503a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email-masked]>
Date: Wed, 8 Dec 2021 14:20:57 +0100
Subject: [PATCH] Readd use of TAP subtests

Since 405f32fc49609eb94fa39e7b5e7c1fe2bb2b73aa, Test::More must be new
enough to support subtests.

The present patch effectively reverts
7912f9b7dc9e2d3f6cd81892ef6aa797578e9f06.  Many more refactorings like
this are possible; this is just to get started.
---
 contrib/oid2name/t/001_basic.pl               |  2 +-
 contrib/vacuumlo/t/001_basic.pl               |  2 +-
 src/bin/initdb/t/001_initdb.pl                |  2 +-
 src/bin/pg_amcheck/t/001_basic.pl             |  2 +-
 src/bin/pg_amcheck/t/005_opclass_damage.pl    |  2 +-
 .../t/010_pg_archivecleanup.pl                |  2 +-
 src/bin/pg_basebackup/t/010_pg_basebackup.pl  |  2 +-
 src/bin/pg_basebackup/t/020_pg_receivewal.pl  |  2 +-
 src/bin/pg_basebackup/t/030_pg_recvlogical.pl |  2 +-
 src/bin/pg_checksums/t/001_basic.pl           |  2 +-
 src/bin/pg_checksums/t/002_actions.pl         |  2 +-
 src/bin/pg_config/t/001_pg_config.pl          |  2 +-
 .../pg_controldata/t/001_pg_controldata.pl    |  2 +-
 src/bin/pg_ctl/t/001_start_stop.pl            |  2 +-
 src/bin/pg_dump/t/001_basic.pl                |  2 +-
 src/bin/pg_resetwal/t/001_basic.pl            |  2 +-
 src/bin/pg_rewind/t/006_options.pl            |  2 +-
 src/bin/pg_test_fsync/t/001_basic.pl          |  2 +-
 src/bin/pg_test_timing/t/001_basic.pl         |  2 +-
 src/bin/pg_verifybackup/t/001_basic.pl        |  2 +-
 src/bin/pg_verifybackup/t/004_options.pl      |  2 +-
 src/bin/pg_verifybackup/t/006_encoding.pl     |  2 +-
 src/bin/pg_waldump/t/001_basic.pl             |  2 +-
 src/bin/psql/t/001_basic.pl                   |  2 +-
 src/bin/scripts/t/010_clusterdb.pl            |  2 +-
 src/bin/scripts/t/011_clusterdb_all.pl        |  2 +-
 src/bin/scripts/t/020_createdb.pl             |  2 +-
 src/bin/scripts/t/040_createuser.pl           |  2 +-
 src/bin/scripts/t/050_dropdb.pl               |  2 +-
 src/bin/scripts/t/070_dropuser.pl             |  2 +-
 src/bin/scripts/t/080_pg_isready.pl           |  2 +-
 src/bin/scripts/t/090_reindexdb.pl            |  2 +-
 src/bin/scripts/t/091_reindexdb_all.pl        |  2 +-
 src/bin/scripts/t/100_vacuumdb.pl             |  2 +-
 src/bin/scripts/t/101_vacuumdb_all.pl         |  2 +-
 src/bin/scripts/t/102_vacuumdb_stages.pl      |  2 +-
 src/test/perl/PostgreSQL/Test/Cluster.pm      | 16 +++--
 src/test/perl/PostgreSQL/Test/Utils.pm        | 68 +++++++++++--------
 src/test/ssl/t/001_ssltests.pl                |  2 +-
 39 files changed, 87 insertions(+), 71 deletions(-)

diff --git a/contrib/oid2name/t/001_basic.pl b/contrib/oid2name/t/001_basic.pl
index efedba0aa1..9d1d1e3533 100644
--- a/contrib/oid2name/t/001_basic.pl
+++ b/contrib/oid2name/t/001_basic.pl
@@ -5,7 +5,7 @@
 use warnings;
 
 use PostgreSQL::Test::Utils;
-use Test::More tests => 8;
+use Test::More tests => 3;
 
 #########################################
 # Basic checks
diff --git a/contrib/vacuumlo/t/001_basic.pl b/contrib/vacuumlo/t/001_basic.pl
index 951dad0d47..8acb4a9317 100644
--- a/contrib/vacuumlo/t/001_basic.pl
+++ b/contrib/vacuumlo/t/001_basic.pl
@@ -5,7 +5,7 @@
 use warnings;
 
 use PostgreSQL::Test::Utils;
-use Test::More tests => 8;
+use Test::More tests => 3;
 
 program_help_ok('vacuumlo');
 program_version_ok('vacuumlo');
diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl
index 6796d8520e..c621df8270 100644
--- a/src/bin/initdb/t/001_initdb.pl
+++ b/src/bin/initdb/t/001_initdb.pl
@@ -11,7 +11,7 @@
 use File::stat qw{lstat};
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 22;
+use Test::More tests => 15;
 
 my $tempdir = PostgreSQL::Test::Utils::tempdir;
 my $xlogdir = "$tempdir/pgxlog";
diff --git a/src/bin/pg_amcheck/t/001_basic.pl b/src/bin/pg_amcheck/t/001_basic.pl
index d44fe60a4c..3c5052b3a6 100644
--- a/src/bin/pg_amcheck/t/001_basic.pl
+++ b/src/bin/pg_amcheck/t/001_basic.pl
@@ -5,7 +5,7 @@
 use warnings;
 
 use PostgreSQL::Test::Utils;
-use Test::More tests => 8;
+use Test::More tests => 3;
 
 program_help_ok('pg_amcheck');
 program_version_ok('pg_amcheck');
diff --git a/src/bin/pg_amcheck/t/005_opclass_damage.pl b/src/bin/pg_amcheck/t/005_opclass_damage.pl
index 2f86f4f2a4..d045e81e65 100644
--- a/src/bin/pg_amcheck/t/005_opclass_damage.pl
+++ b/src/bin/pg_amcheck/t/005_opclass_damage.pl
@@ -8,7 +8,7 @@
 use warnings;
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 5;
+use Test::More tests => 3;
 
 my $node = PostgreSQL::Test::Cluster->new('test');
 $node->init;
diff --git a/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl b/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
index 6b3f486cfa..0531da1115 100644
--- a/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
+++ b/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
@@ -4,7 +4,7 @@
 use strict;
 use warnings;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 42;
+use Test::More tests => 37;
 
 program_help_ok('pg_archivecleanup');
 program_version_ok('pg_archivecleanup');
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 89f45b77a3..c7bd78d988 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -10,7 +10,7 @@
 use Fcntl qw(:seek);
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 110;
+use Test::More tests => 105;
 
 program_help_ok('pg_basebackup');
 program_version_ok('pg_basebackup');
diff --git a/src/bin/pg_basebackup/t/020_pg_receivewal.pl b/src/bin/pg_basebackup/t/020_pg_receivewal.pl
index 43599d832b..54cee6248d 100644
--- a/src/bin/pg_basebackup/t/020_pg_receivewal.pl
+++ b/src/bin/pg_basebackup/t/020_pg_receivewal.pl
@@ -5,7 +5,7 @@
 use warnings;
 use PostgreSQL::Test::Utils;
 use PostgreSQL::Test::Cluster;
-use Test::More tests => 42;
+use Test::More tests => 37;
 
 program_help_ok('pg_receivewal');
 program_version_ok('pg_receivewal');
diff --git a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
index 90da1662e3..ab566b116d 100644
--- a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
+++ b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
@@ -5,7 +5,7 @@
 use warnings;
 use PostgreSQL::Test::Utils;
 use PostgreSQL::Test::Cluster;
-use Test::More tests => 20;
+use Test::More tests => 15;
 
 program_help_ok('pg_recvlogical');
 program_version_ok('pg_recvlogical');
diff --git a/src/bin/pg_checksums/t/001_basic.pl b/src/bin/pg_checksums/t/001_basic.pl
index e9eb3197a6..507ffacef0 100644
--- a/src/bin/pg_checksums/t/001_basic.pl
+++ b/src/bin/pg_checksums/t/001_basic.pl
@@ -4,7 +4,7 @@
 use strict;
 use warnings;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 8;
+use Test::More tests => 3;
 
 program_help_ok('pg_checksums');
 program_version_ok('pg_checksums');
diff --git a/src/bin/pg_checksums/t/002_actions.pl b/src/bin/pg_checksums/t/002_actions.pl
index 20a5f27840..f34245108d 100644
--- a/src/bin/pg_checksums/t/002_actions.pl
+++ b/src/bin/pg_checksums/t/002_actions.pl
@@ -11,7 +11,7 @@
 use PostgreSQL::Test::Utils;
 
 use Fcntl qw(:seek);
-use Test::More tests => 66;
+use Test::More tests => 58;

 # Utility routine to create and check a table with corrupted checksums
diff --git a/src/bin/pg_config/t/001_pg_config.pl b/src/bin/pg_config/t/001_pg_config.pl
index 6c7f9b8602..ebf6aacc4a 100644
--- a/src/bin/pg_config/t/001_pg_config.pl
+++ b/src/bin/pg_config/t/001_pg_config.pl
@@ -4,7 +4,7 @@
 use strict;
 use warnings;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 20;
+use Test::More tests => 7;
 
 program_help_ok('pg_config');
 program_version_ok('pg_config');
diff --git a/src/bin/pg_controldata/t/001_pg_controldata.pl b/src/bin/pg_controldata/t/001_pg_controldata.pl
index ad7bacace5..fd3ecfaed7 100644
--- a/src/bin/pg_controldata/t/001_pg_controldata.pl
+++ b/src/bin/pg_controldata/t/001_pg_controldata.pl
@@ -5,7 +5,7 @@
 use warnings;
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 17;
+use Test::More tests => 10;
 
 program_help_ok('pg_controldata');
 program_version_ok('pg_controldata');
diff --git a/src/bin/pg_ctl/t/001_start_stop.pl b/src/bin/pg_ctl/t/001_start_stop.pl
index f95352bf94..9fb675a079 100644
--- a/src/bin/pg_ctl/t/001_start_stop.pl
+++ b/src/bin/pg_ctl/t/001_start_stop.pl
@@ -9,7 +9,7 @@
 use File::stat qw{lstat};
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 24;
+use Test::More tests => 17;
 
 my $tempdir       = PostgreSQL::Test::Utils::tempdir;
 my $tempdir_short = PostgreSQL::Test::Utils::tempdir_short;
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index 863f4da3d8..d31a72a621 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -7,7 +7,7 @@
 use Config;
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 82;
+use Test::More tests => 67;
 
 my $tempdir       = PostgreSQL::Test::Utils::tempdir;
 
diff --git a/src/bin/pg_resetwal/t/001_basic.pl b/src/bin/pg_resetwal/t/001_basic.pl
index 0f86aea68e..14aa98de40 100644
--- a/src/bin/pg_resetwal/t/001_basic.pl
+++ b/src/bin/pg_resetwal/t/001_basic.pl
@@ -6,7 +6,7 @@
 
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 12;
+use Test::More tests => 5;
 
 program_help_ok('pg_resetwal');
 program_version_ok('pg_resetwal');
diff --git a/src/bin/pg_rewind/t/006_options.pl b/src/bin/pg_rewind/t/006_options.pl
index 30c7bb46d2..ff08c80976 100644
--- a/src/bin/pg_rewind/t/006_options.pl
+++ b/src/bin/pg_rewind/t/006_options.pl
@@ -7,7 +7,7 @@
 use strict;
 use warnings;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 12;
+use Test::More tests => 7;
 
 program_help_ok('pg_rewind');
 program_version_ok('pg_rewind');
diff --git a/src/bin/pg_test_fsync/t/001_basic.pl b/src/bin/pg_test_fsync/t/001_basic.pl
index 8c71f1111e..973506238a 100644
--- a/src/bin/pg_test_fsync/t/001_basic.pl
+++ b/src/bin/pg_test_fsync/t/001_basic.pl
@@ -6,7 +6,7 @@
 
 use Config;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 12;
+use Test::More tests => 7;
 
 #########################################
 # Basic checks
diff --git a/src/bin/pg_test_timing/t/001_basic.pl b/src/bin/pg_test_timing/t/001_basic.pl
index 3e58926c96..1d95e2a6a6 100644
--- a/src/bin/pg_test_timing/t/001_basic.pl
+++ b/src/bin/pg_test_timing/t/001_basic.pl
@@ -6,7 +6,7 @@
 
 use Config;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 12;
+use Test::More tests => 7;
 
 #########################################
 # Basic checks
diff --git a/src/bin/pg_verifybackup/t/001_basic.pl b/src/bin/pg_verifybackup/t/001_basic.pl
index 33d6b38d33..9163f3e1b1 100644
--- a/src/bin/pg_verifybackup/t/001_basic.pl
+++ b/src/bin/pg_verifybackup/t/001_basic.pl
@@ -4,7 +4,7 @@
 use strict;
 use warnings;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 16;
+use Test::More tests => 11;
 
 my $tempdir = PostgreSQL::Test::Utils::tempdir;
 
diff --git a/src/bin/pg_verifybackup/t/004_options.pl b/src/bin/pg_verifybackup/t/004_options.pl
index 22b1444091..234a529e2a 100644
--- a/src/bin/pg_verifybackup/t/004_options.pl
+++ b/src/bin/pg_verifybackup/t/004_options.pl
@@ -10,7 +10,7 @@
 use File::Path qw(rmtree);
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 25;
+use Test::More tests => 19;
 
 # Start up the server and take a backup.
 my $primary = PostgreSQL::Test::Cluster->new('primary');
diff --git a/src/bin/pg_verifybackup/t/006_encoding.pl b/src/bin/pg_verifybackup/t/006_encoding.pl
index 21c4198b1c..d1b42edbe3 100644
--- a/src/bin/pg_verifybackup/t/006_encoding.pl
+++ b/src/bin/pg_verifybackup/t/006_encoding.pl
@@ -9,7 +9,7 @@
 use Config;
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 5;
+use Test::More tests => 3;
 
 my $primary = PostgreSQL::Test::Cluster->new('primary');
 $primary->init(allows_streaming => 1);
diff --git a/src/bin/pg_waldump/t/001_basic.pl b/src/bin/pg_waldump/t/001_basic.pl
index fdc968a5ee..277eae749c 100644
--- a/src/bin/pg_waldump/t/001_basic.pl
+++ b/src/bin/pg_waldump/t/001_basic.pl
@@ -4,7 +4,7 @@
 use strict;
 use warnings;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 8;
+use Test::More tests => 3;
 
 program_help_ok('pg_waldump');
 program_version_ok('pg_waldump');
diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl
index 6ca0bc75d0..bcc195dd69 100644
--- a/src/bin/psql/t/001_basic.pl
+++ b/src/bin/psql/t/001_basic.pl
@@ -6,7 +6,7 @@
 
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 25;
+use Test::More tests => 20;
 
 program_help_ok('psql');
 program_version_ok('psql');
diff --git a/src/bin/scripts/t/010_clusterdb.pl b/src/bin/scripts/t/010_clusterdb.pl
index 0ba4aa4876..02165aa6d5 100644
--- a/src/bin/scripts/t/010_clusterdb.pl
+++ b/src/bin/scripts/t/010_clusterdb.pl
@@ -6,7 +6,7 @@
 
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 14;
+use Test::More tests => 7;
 
 program_help_ok('clusterdb');
 program_version_ok('clusterdb');
diff --git a/src/bin/scripts/t/011_clusterdb_all.pl b/src/bin/scripts/t/011_clusterdb_all.pl
index d040b95cfb..b506bc9a10 100644
--- a/src/bin/scripts/t/011_clusterdb_all.pl
+++ b/src/bin/scripts/t/011_clusterdb_all.pl
@@ -6,7 +6,7 @@
 
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 2;
+use Test::More tests => 1;
 
 my $node = PostgreSQL::Test::Cluster->new('main');
 $node->init;
diff --git a/src/bin/scripts/t/020_createdb.pl b/src/bin/scripts/t/020_createdb.pl
index 6bcc59de08..cd849c2b1e 100644
--- a/src/bin/scripts/t/020_createdb.pl
+++ b/src/bin/scripts/t/020_createdb.pl
@@ -6,7 +6,7 @@
 
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 25;
+use Test::More tests => 17;
 
 program_help_ok('createdb');
 program_version_ok('createdb');
diff --git a/src/bin/scripts/t/040_createuser.pl b/src/bin/scripts/t/040_createuser.pl
index a865c01f5a..3ff38a4f55 100644
--- a/src/bin/scripts/t/040_createuser.pl
+++ b/src/bin/scripts/t/040_createuser.pl
@@ -6,7 +6,7 @@
 
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 17;
+use Test::More tests => 8;
 
 program_help_ok('createuser');
 program_version_ok('createuser');
diff --git a/src/bin/scripts/t/050_dropdb.pl b/src/bin/scripts/t/050_dropdb.pl
index 5c9342f290..6affaf083e 100644
--- a/src/bin/scripts/t/050_dropdb.pl
+++ b/src/bin/scripts/t/050_dropdb.pl
@@ -6,7 +6,7 @@
 
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 13;
+use Test::More tests => 6;
 
 program_help_ok('dropdb');
 program_version_ok('dropdb');
diff --git a/src/bin/scripts/t/070_dropuser.pl b/src/bin/scripts/t/070_dropuser.pl
index 5d6e75c903..4ce97d1a1d 100644
--- a/src/bin/scripts/t/070_dropuser.pl
+++ b/src/bin/scripts/t/070_dropuser.pl
@@ -6,7 +6,7 @@
 
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 11;
+use Test::More tests => 5;
 
 program_help_ok('dropuser');
 program_version_ok('dropuser');
diff --git a/src/bin/scripts/t/080_pg_isready.pl b/src/bin/scripts/t/080_pg_isready.pl
index 42be32decc..963c08df72 100644
--- a/src/bin/scripts/t/080_pg_isready.pl
+++ b/src/bin/scripts/t/080_pg_isready.pl
@@ -6,7 +6,7 @@
 
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 10;
+use Test::More tests => 5;
 
 program_help_ok('pg_isready');
 program_version_ok('pg_isready');
diff --git a/src/bin/scripts/t/090_reindexdb.pl b/src/bin/scripts/t/090_reindexdb.pl
index 5384b74ccd..fdf1fac52b 100644
--- a/src/bin/scripts/t/090_reindexdb.pl
+++ b/src/bin/scripts/t/090_reindexdb.pl
@@ -6,7 +6,7 @@
 
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 58;
+use Test::More tests => 37;
 
 program_help_ok('reindexdb');
 program_version_ok('reindexdb');
diff --git a/src/bin/scripts/t/091_reindexdb_all.pl b/src/bin/scripts/t/091_reindexdb_all.pl
index acb2418976..9c86c43390 100644
--- a/src/bin/scripts/t/091_reindexdb_all.pl
+++ b/src/bin/scripts/t/091_reindexdb_all.pl
@@ -5,7 +5,7 @@
 use warnings;
 
 use PostgreSQL::Test::Cluster;
-use Test::More tests => 2;
+use Test::More tests => 1;
 
 my $node = PostgreSQL::Test::Cluster->new('main');
 $node->init;
diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl
index 6937a35bc4..aa2832a27e 100644
--- a/src/bin/scripts/t/100_vacuumdb.pl
+++ b/src/bin/scripts/t/100_vacuumdb.pl
@@ -6,7 +6,7 @@
 
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 58;
+use Test::More tests => 36;
 
 program_help_ok('vacuumdb');
 program_version_ok('vacuumdb');
diff --git a/src/bin/scripts/t/101_vacuumdb_all.pl b/src/bin/scripts/t/101_vacuumdb_all.pl
index 3dfbfbfdb2..1432cec745 100644
--- a/src/bin/scripts/t/101_vacuumdb_all.pl
+++ b/src/bin/scripts/t/101_vacuumdb_all.pl
@@ -5,7 +5,7 @@
 use warnings;
 
 use PostgreSQL::Test::Cluster;
-use Test::More tests => 2;
+use Test::More tests => 1;
 
 my $node = PostgreSQL::Test::Cluster->new('main');
 $node->init;
diff --git a/src/bin/scripts/t/102_vacuumdb_stages.pl b/src/bin/scripts/t/102_vacuumdb_stages.pl
index f7bd45ba92..f8d29b5f51 100644
--- a/src/bin/scripts/t/102_vacuumdb_stages.pl
+++ b/src/bin/scripts/t/102_vacuumdb_stages.pl
@@ -5,7 +5,7 @@
 use warnings;
 
 use PostgreSQL::Test::Cluster;
-use Test::More tests => 4;
+use Test::More tests => 2;
 
 my $node = PostgreSQL::Test::Cluster->new('main');
 $node->init;
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 9467a199c8..ed2ac110ef 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2408,14 +2408,18 @@ sub issues_sql_like
 
 	my ($self, $cmd, $expected_sql, $test_name) = @_;
 
-	local %ENV = $self->_get_env();
+	subtest $test_name => sub {
+		plan tests => 2;
 
-	my $log_location = -s $self->logfile;
+		local %ENV = $self->_get_env();
 
-	my $result = PostgreSQL::Test::Utils::run_log($cmd);
-	ok($result, "@$cmd exit code 0");
-	my $log = PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
-	like($log, $expected_sql, "$test_name: SQL found in server log");
+		my $log_location = -s $self->logfile;
+
+		my $result = PostgreSQL::Test::Utils::run_log($cmd);
+		ok($result, "@$cmd exit code 0");
+		my $log = PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
+		like($log, $expected_sql, "$test_name: SQL found in server log");
+	};
 	return;
 }
 
diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm
index 378d3f7bc1..3b214e04e8 100644
--- a/src/test/perl/PostgreSQL/Test/Utils.pm
+++ b/src/test/perl/PostgreSQL/Test/Utils.pm
@@ -805,13 +805,16 @@ sub program_help_ok
 {
 	local $Test::Builder::Level = $Test::Builder::Level + 1;
 	my ($cmd) = @_;
-	my ($stdout, $stderr);
-	print("# Running: $cmd --help\n");
-	my $result = IPC::Run::run [ $cmd, '--help' ], '>', \$stdout, '2>',
-	  \$stderr;
-	ok($result, "$cmd --help exit code 0");
-	isnt($stdout, '', "$cmd --help goes to stdout");
-	is($stderr, '', "$cmd --help nothing to stderr");
+	subtest "$cmd --help" => sub {
+		plan tests => 3;
+		my ($stdout, $stderr);
+		print("# Running: $cmd --help\n");
+		my $result = IPC::Run::run [ $cmd, '--help' ], '>', \$stdout, '2>',
+		  \$stderr;
+		ok($result, "$cmd --help exit code 0");
+		isnt($stdout, '', "$cmd --help goes to stdout");
+		is($stderr, '', "$cmd --help nothing to stderr");
+	};
 	return;
 }
 
@@ -827,13 +830,16 @@ sub program_version_ok
 {
 	local $Test::Builder::Level = $Test::Builder::Level + 1;
 	my ($cmd) = @_;
-	my ($stdout, $stderr);
-	print("# Running: $cmd --version\n");
-	my $result = IPC::Run::run [ $cmd, '--version' ], '>', \$stdout, '2>',
-	  \$stderr;
-	ok($result, "$cmd --version exit code 0");
-	isnt($stdout, '', "$cmd --version goes to stdout");
-	is($stderr, '', "$cmd --version nothing to stderr");
+	subtest "$cmd --version" => sub {
+		plan tests => 3;
+		my ($stdout, $stderr);
+		print("# Running: $cmd --version\n");
+		my $result = IPC::Run::run [ $cmd, '--version' ], '>', \$stdout, '2>',
+		  \$stderr;
+		ok($result, "$cmd --version exit code 0");
+		isnt($stdout, '', "$cmd --version goes to stdout");
+		is($stderr, '', "$cmd --version nothing to stderr");
+	};
 	return;
 }
 
@@ -850,13 +856,16 @@ sub program_options_handling_ok
 {
 	local $Test::Builder::Level = $Test::Builder::Level + 1;
 	my ($cmd) = @_;
-	my ($stdout, $stderr);
-	print("# Running: $cmd --not-a-valid-option\n");
-	my $result = IPC::Run::run [ $cmd, '--not-a-valid-option' ], '>',
-	  \$stdout,
-	  '2>', \$stderr;
-	ok(!$result, "$cmd with invalid option nonzero exit code");
-	isnt($stderr, '', "$cmd with invalid option prints error message");
+	subtest "$cmd options handling" => sub {
+		plan tests => 2;
+		my ($stdout, $stderr);
+		print("# Running: $cmd --not-a-valid-option\n");
+		my $result = IPC::Run::run [ $cmd, '--not-a-valid-option' ], '>',
+		  \$stdout,
+		  '2>', \$stderr;
+		ok(!$result, "$cmd with invalid option nonzero exit code");
+		isnt($stderr, '', "$cmd with invalid option prints error message");
+	};
 	return;
 }
 
@@ -873,13 +882,16 @@ sub command_like
 {
 	local $Test::Builder::Level = $Test::Builder::Level + 1;
 	my ($cmd, $expected_stdout, $test_name) = @_;
-	my ($stdout, $stderr);
-	print("# Running: " . join(" ", @{$cmd}) . "\n");
-	my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
-	ok($result, "$test_name: exit code 0");
-	is($stderr, '', "$test_name: no stderr");
-	$stdout =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
-	like($stdout, $expected_stdout, "$test_name: matches");
+	subtest $test_name => sub {
+		plan tests => 3;
+		my ($stdout, $stderr);
+		print("# Running: " . join(" ", @{$cmd}) . "\n");
+		my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
+		ok($result, "$test_name: exit code 0");
+		is($stderr, '', "$test_name: no stderr");
+		$stdout =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
+		like($stdout, $expected_stdout, "$test_name: matches");
+	};
 	return;
 }
 
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 779ab66838..92d479c50f 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -21,7 +21,7 @@
 }
 else
 {
-	plan tests => 110;
+	plan tests => 106;
 }
 
 #### Some configuration
-- 
2.34.1

---

From: Dagfinn Ilmari Mannsåker <[email-masked]>
Date: Wed, 08 Dec 2021 13:49:06 +0000

> and put those subtests back.

The updated Test::More version requirement also gives us done_testing()
(added in 0.88), which saves us from the constant maintenance headache
of updating the test counts every time.  Do you fancy switching the
tests you're modifying anyway to that?

- ilmari

---

From: Daniel Gustafsson <[email-masked]>
Date: Wed, 8 Dec 2021 14:53:33 +0100

> On 8 Dec 2021, at 14:49, Dagfinn Ilmari Mannsåker <[email-masked]> wrote:
> 

We already call done_testing() in a number of tests, and have done so for a
number of years.  src/test/ssl/t/002_scram.pl is one example.

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

---

From: Andrew Dunstan <[email-masked]>
Date: Wed, 8 Dec 2021 08:53:59 -0500

On 12/8/21 08:26, Peter Eisentraut wrote:
> this out of the way since the code was already written and I've had
> this on my list for, uh, 7 years.

+many as long as we cover all the cases in Cluster.pm and Utils.pm. I
suspect they have acquired some new multi-test subs in the intervening 7
years :-)

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

---

From: Dagfinn Ilmari Mannsåker <[email-masked]>
Date: Wed, 08 Dec 2021 14:08:05 +0000

> number of years.  src/test/ssl/t/002_scram.pl is one example.

Reading the Test::More changelog more closely, it turns out that even
though we used to depend on version 0.87, that's effectively equivalent
0.88, because there was no stable 0.87 release, only 0.86 and
development releases 0.87_01 through _03.

Either way, I think we should be switching tests to done_testing()
whenever it would otherwise have to adjust the test count, to avoid
having to do that again and again and again going forward.

- ilmari

---

From: Andrew Dunstan <[email-masked]>
Date: Wed, 8 Dec 2021 09:33:06 -0500

> having to do that again and again and again going forward.

I'm not so sure. I don't think its necessarily a bad idea to have to
declare how many tests you're going to run. I appreciate it gets hard in
some cases, which is why we have now insisted on a Test::More version
that supports subtests. I suppose we could just take the attitude that
we're happy with however many tests it actually runs, and as long as
they all pass we're good. It just seems slightly sloppy.

cheers

andrew

---

From: Dagfinn Ilmari Mannsåker <[email-masked]>
Date: Wed, 08 Dec 2021 14:48:59 +0000

> they all pass we're good. It just seems slightly sloppy.

The point of done_testing() is to additionally assert that the test
script ran to completion, so you don't get silent failures if something
should end up calling exit(0) prematurely (a non-zero exit status is
considered a failure by the test harness).

The only cases where an explicit plan adds value is if you're running
tests in a loop and care about the number of iterations, or have a
callback with a test inside that you want to make sure gets called.  For
these, it's better to explicitly assert that the list you're iterating
over is of the right length, or increment a counter in the loop or
callback and assert that it has the expected value.  This has the added
benefit of the failure being coming from the relevant place and having a
helpful description, rather than a plan mismatch at the end which you
then have to hunt down the cause of.

- ilmari

---

From: Tom Lane <[email-masked]>
Date: Wed, 08 Dec 2021 10:25:32 -0500

> declare how many tests you're going to run.

I think the main point is to make sure that the test script reached an
intended exit point, rather than dying early someplace.  It's not apparent
to me why reaching a done_testing() call is a less reliable indicator of
that than executing some specific number of tests --- and I agree with
ilmari that maintaining the test count is a serious PITA.

			regards, tom lane

---

From: Tom Lane <[email-masked]>
Date: Wed, 08 Dec 2021 12:31:00 -0500

> then have to hunt down the cause of.

Yeah.  A different way of stating that is that the test count adds
security only if you re-derive its proper value from first principles
every time you modify the test script.  I don't know about you guys,
but the only way I've ever adjusted those numbers is to put in whatever
the error message said was right.  I don't see how that's adding
anything but make-work; it's certainly not doing much to help verify
the script's control flow.

A question that seems pretty relevant here is: what exactly is the
point of using the subtest feature, if we aren't especially interested
in its effect on the overall test count?  I can see that it'd have
value when you wanted to use skip_all to control a subset of a test
run, but I'm not detecting where is the value-added in the cases in
Peter's proposed patch.

			regards, tom lane

---

From: Peter Eisentraut <[email-masked]>
Date: Thu, 9 Dec 2021 16:51:17 +0100

> run, but I'm not detecting where is the value-added in the cases in

It's useful if you edit a test file and add (what would appear to be) N 
tests and want to update the number.

But I'm also OK with the done_testing() style, if there are no drawbacks 
to that.

Does that call into question why we raised the Test::More version to 
begin with?  Or were there other reasons?

---

From: Daniel Gustafsson <[email-masked]>
Date: Thu, 9 Dec 2021 21:54:36 +0100

> On 8 Dec 2021, at 16:25, Tom Lane <[email-masked]> wrote:

> ilmari that maintaining the test count is a serious PITA.

FWIW I agree with this and am in favor of using done_testing().