Improve TAP tests by replacing ok() with better Test::More functions

The TAP tests whose ok() calls are changed in this commit were relying
on perl operators, rather than equivalents available in Test::More.  For
example, rather than the following:
ok($data =~ qr/expr/m, "expr matching");
ok($data !~ qr/expr/m, "expr not matching");
The new test code uses this equivalent:
like($data, qr/expr/m, "expr matching");
unlike($data, qr/expr/m, "expr not matching");

A huge benefit of the new formulation is that it is possible to know
about the values we are checking if a failure happens, making debugging
easier, should the test runs happen in the buildfarm, in the CI or
locally.

This change leads to more test code overall as perltidy likes to make
the code pretty the way it is in this commit.

Author: Sadhuprasad Patro <b.sadhu@gmail.com>
Discussion: https://postgr.es/m/CAFF0-CHhwNx_Cv2uy7tKjODUbeOgPrJpW4Rpf1jqB16_1bU2sg@mail.gmail.com

6 files changed, 80 insertions, 51 deletions

diff --git a/src/test/subscription/t/001_rep_changes.pl b/src/test/subscription/t/001_rep_changes.pl
index ca55d8df50d..430c1246d14 100644
--- a/src/test/subscription/t/001_rep_changes.pl
+++ b/src/test/subscription/t/001_rep_changes.pl
@@ -364,14 +364,17 @@ $node_publisher->safe_psql('postgres', "DELETE FROM tab_full_pk WHERE a = 2");
 $node_publisher->wait_for_catchup('tap_sub');
 
 my $logfile = slurp_file($node_subscriber->logfile, $log_location);
-ok( $logfile =~
-	  qr/conflict detected on relation "public.tab_full_pk": conflict=update_missing.*\n.*DETAIL:.* Could not find the row to be updated.*\n.*Remote row \(1, quux\); replica identity \(a\)=\(1\)/m,
+like(
+	$logfile,
+	qr/conflict detected on relation "public.tab_full_pk": conflict=update_missing.*\n.*DETAIL:.* Could not find the row to be updated.*\n.*Remote row \(1, quux\); replica identity \(a\)=\(1\)/m,
 	'update target row is missing');
-ok( $logfile =~
-	  qr/conflict detected on relation "public.tab_full": conflict=update_missing.*\n.*DETAIL:.* Could not find the row to be updated.*\n.*Remote row \(26\); replica identity full \(25\)/m,
+like(
+	$logfile,
+	qr/conflict detected on relation "public.tab_full": conflict=update_missing.*\n.*DETAIL:.* Could not find the row to be updated.*\n.*Remote row \(26\); replica identity full \(25\)/m,
 	'update target row is missing');
-ok( $logfile =~
-	  qr/conflict detected on relation "public.tab_full_pk": conflict=delete_missing.*\n.*DETAIL:.* Could not find the row to be deleted.*\n.*Replica identity \(a\)=\(2\)/m,
+like(
+	$logfile,
+	qr/conflict detected on relation "public.tab_full_pk": conflict=delete_missing.*\n.*DETAIL:.* Could not find the row to be deleted.*\n.*Replica identity \(a\)=\(2\)/m,
 	'delete target row is missing');
 
 $node_subscriber->append_conf('postgresql.conf',
@@ -515,7 +518,9 @@ $node_publisher->safe_psql('postgres', "INSERT INTO tab_notrep VALUES (11)");
 $node_publisher->wait_for_catchup('tap_sub');
 
 $logfile = slurp_file($node_publisher->logfile, $log_location);
-ok($logfile =~ qr/skipped replication of an empty transaction with XID/,
+like(
+	$logfile,
+	qr/skipped replication of an empty transaction with XID/,
 	'empty transaction is skipped');
 
 $result =
@@ -588,8 +593,9 @@ CREATE TABLE skip_wal();
 CREATE PUBLICATION tap_pub2 FOR TABLE skip_wal;
 ROLLBACK;
 });
-ok( $reterr =~
-	  m/WARNING:  "wal_level" is insufficient to publish logical changes/,
+like(
+	$reterr,
+	qr/WARNING:  "wal_level" is insufficient to publish logical changes/,
 	'CREATE PUBLICATION while "wal_level=minimal"');
 
 done_testing();
diff --git a/src/test/subscription/t/007_ddl.pl b/src/test/subscription/t/007_ddl.pl
index 2a45fb13739..16cb7bca8f2 100644
--- a/src/test/subscription/t/007_ddl.pl
+++ b/src/test/subscription/t/007_ddl.pl
@@ -45,8 +45,9 @@ pass "subscription disable and drop in same transaction did not hang";
 my ($ret, $stdout, $stderr) = $node_subscriber->psql('postgres',
 	"CREATE SUBSCRIPTION mysub1 CONNECTION '$publisher_connstr' PUBLICATION mypub, non_existent_pub"
 );
-ok( $stderr =~
-	  m/WARNING:  publication "non_existent_pub" does not exist on the publisher/,
+like(
+	$stderr,
+	qr/WARNING:  publication "non_existent_pub" does not exist on the publisher/,
 	"Create subscription throws warning for non-existent publication");
 
 # Wait for initial table sync to finish.
@@ -56,16 +57,18 @@ $node_subscriber->wait_for_subscription_sync($node_publisher, 'mysub1');
 ($ret, $stdout, $stderr) = $node_subscriber->psql('postgres',
 	"ALTER SUBSCRIPTION mysub1 ADD PUBLICATION non_existent_pub1, non_existent_pub2"
 );
-ok( $stderr =~
-	  m/WARNING:  publications "non_existent_pub1", "non_existent_pub2" do not exist on the publisher/,
+like(
+	$stderr,
+	qr/WARNING:  publications "non_existent_pub1", "non_existent_pub2" do not exist on the publisher/,
 	"Alter subscription add publication throws warning for non-existent publications"
 );
 
 # Specifying non-existent publication along with set publication.
 ($ret, $stdout, $stderr) = $node_subscriber->psql('postgres',
 	"ALTER SUBSCRIPTION mysub1 SET PUBLICATION non_existent_pub");
-ok( $stderr =~
-	  m/WARNING:  publication "non_existent_pub" does not exist on the publisher/,
+like(
+	$stderr,
+	qr/WARNING:  publication "non_existent_pub" does not exist on the publisher/,
 	"Alter subscription set publication throws warning for non-existent publication"
 );
 
diff --git a/src/test/subscription/t/013_partition.pl b/src/test/subscription/t/013_partition.pl
index 763a91e75a3..3a5e92d094f 100644
--- a/src/test/subscription/t/013_partition.pl
+++ b/src/test/subscription/t/013_partition.pl
@@ -367,17 +367,21 @@ $node_publisher->wait_for_catchup('sub1');
 $node_publisher->wait_for_catchup('sub2');
 
 my $logfile = slurp_file($node_subscriber1->logfile(), $log_location);
-ok( $logfile =~
-	  qr/conflict detected on relation "public.tab1_2_2": conflict=update_missing.*\n.*DETAIL:.* Could not find the row to be updated.*\n.*Remote row \(null, 4, quux\); replica identity \(a\)=\(4\)/,
+like(
+	$logfile,
+	qr/conflict detected on relation "public.tab1_2_2": conflict=update_missing.*\n.*DETAIL:.* Could not find the row to be updated.*\n.*Remote row \(null, 4, quux\); replica identity \(a\)=\(4\)/,
 	'update target row is missing in tab1_2_2');
-ok( $logfile =~
-	  qr/conflict detected on relation "public.tab1_1": conflict=delete_missing.*\n.*DETAIL:.* Could not find the row to be deleted.*\n.*Replica identity \(a\)=\(1\)/,
+like(
+	$logfile,
+	qr/conflict detected on relation "public.tab1_1": conflict=delete_missing.*\n.*DETAIL:.* Could not find the row to be deleted.*\n.*Replica identity \(a\)=\(1\)/,
 	'delete target row is missing in tab1_1');
-ok( $logfile =~
-	  qr/conflict detected on relation "public.tab1_2_2": conflict=delete_missing.*\n.*DETAIL:.* Could not find the row to be deleted.*\n.*Replica identity \(a\)=\(4\)/,
+like(
+	$logfile,
+	qr/conflict detected on relation "public.tab1_2_2": conflict=delete_missing.*\n.*DETAIL:.* Could not find the row to be deleted.*\n.*Replica identity \(a\)=\(4\)/,
 	'delete target row is missing in tab1_2_2');
-ok( $logfile =~
-	  qr/conflict detected on relation "public.tab1_def": conflict=delete_missing.*\n.*DETAIL:.* Could not find the row to be deleted.*\n.*Replica identity \(a\)=\(10\)/,
+like(
+	$logfile,
+	qr/conflict detected on relation "public.tab1_def": conflict=delete_missing.*\n.*DETAIL:.* Could not find the row to be deleted.*\n.*Replica identity \(a\)=\(10\)/,
 	'delete target row is missing in tab1_def');
 
 # Tests for replication using root table identity and schema
@@ -780,11 +784,13 @@ $node_publisher->wait_for_catchup('sub_viaroot');
 $node_publisher->wait_for_catchup('sub2');
 
 $logfile = slurp_file($node_subscriber1->logfile(), $log_location);
-ok( $logfile =~
-	  qr/conflict detected on relation "public.tab2_1": conflict=update_missing.*\n.*DETAIL:.* Could not find the row to be updated.*\n.*Remote row \(pub_tab2, quux, 5\); replica identity \(a\)=\(5\)/,
+like(
+	$logfile,
+	qr/conflict detected on relation "public.tab2_1": conflict=update_missing.*\n.*DETAIL:.* Could not find the row to be updated.*\n.*Remote row \(pub_tab2, quux, 5\); replica identity \(a\)=\(5\)/,
 	'update target row is missing in tab2_1');
-ok( $logfile =~
-	  qr/conflict detected on relation "public.tab2_1": conflict=delete_missing.*\n.*DETAIL:.* Could not find the row to be deleted.*\n.*Replica identity \(a\)=\(1\)/,
+like(
+	$logfile,
+	qr/conflict detected on relation "public.tab2_1": conflict=delete_missing.*\n.*DETAIL:.* Could not find the row to be deleted.*\n.*Replica identity \(a\)=\(1\)/,
 	'delete target row is missing in tab2_1');
 
 # Enable the track_commit_timestamp to detect the conflict when attempting
@@ -801,8 +807,9 @@ $node_publisher->safe_psql('postgres',
 $node_publisher->wait_for_catchup('sub_viaroot');
 
 $logfile = slurp_file($node_subscriber1->logfile(), $log_location);
-ok( $logfile =~
-	  qr/conflict detected on relation "public.tab2_1": conflict=update_origin_differs.*\n.*DETAIL:.* Updating the row that was modified locally in transaction [0-9]+ at .*\n.*Existing local row \(yyy, null, 3\); remote row \(pub_tab2, quux, 3\); replica identity \(a\)=\(3\)/,
+like(
+	$logfile,
+	qr/conflict detected on relation "public.tab2_1": conflict=update_origin_differs.*\n.*DETAIL:.* Updating the row that was modified locally in transaction [0-9]+ at .*\n.*Existing local row \(yyy, null, 3\); remote row \(pub_tab2, quux, 3\); replica identity \(a\)=\(3\)/,
 	'updating a row that was modified by a different origin');
 
 # The remaining tests no longer test conflict detection.
diff --git a/src/test/subscription/t/027_nosuperuser.pl b/src/test/subscription/t/027_nosuperuser.pl
index 36af1c16e7f..691731743df 100644
--- a/src/test/subscription/t/027_nosuperuser.pl
+++ b/src/test/subscription/t/027_nosuperuser.pl
@@ -399,8 +399,9 @@ SKIP:
 	isnt($ret, 0,
 		"non zero exit for subscription whose owner is a non-superuser must specify password parameter of the connection string"
 	);
-	ok( $stderr =~
-		  m/DETAIL:  Non-superusers must provide a password in the connection string./,
+	like(
+		$stderr,
+		qr/DETAIL:  Non-superusers must provide a password in the connection string./,
 		'subscription whose owner is a non-superuser must specify password parameter of the connection string'
 	);
 
diff --git a/src/test/subscription/t/031_column_list.pl b/src/test/subscription/t/031_column_list.pl
index e859bcdf4eb..2c14270af21 100644
--- a/src/test/subscription/t/031_column_list.pl
+++ b/src/test/subscription/t/031_column_list.pl
@@ -1272,8 +1272,9 @@ my ($cmdret, $stdout, $stderr) = $node_subscriber->psql(
 	CREATE SUBSCRIPTION sub1 CONNECTION '$publisher_connstr' PUBLICATION pub_mix_1, pub_mix_2;
 ));
 
-ok( $stderr =~
-	  qr/cannot use different column lists for table "public.test_mix_1" in different publications/,
+like(
+	$stderr,
+	qr/cannot use different column lists for table "public.test_mix_1" in different publications/,
 	'different column lists detected');
 
 # TEST: If the column list is changed after creating the subscription, we
diff --git a/src/test/subscription/t/035_conflicts.pl b/src/test/subscription/t/035_conflicts.pl
index 741f5397478..a94bde16f53 100644
--- a/src/test/subscription/t/035_conflicts.pl
+++ b/src/test/subscription/t/035_conflicts.pl
@@ -224,8 +224,9 @@ ok( $node_B->poll_query_until(
 # Alter retain_dead_tuples for enabled subscription
 my ($cmdret, $stdout, $stderr) = $node_A->psql('postgres',
 	"ALTER SUBSCRIPTION $subname_AB SET (retain_dead_tuples = true)");
-ok( $stderr =~
-	  /ERROR:  cannot set option \"retain_dead_tuples\" for enabled subscription/,
+like(
+	$stderr,
+	qr/ERROR:  cannot set option \"retain_dead_tuples\" for enabled subscription/,
 	"altering retain_dead_tuples is not allowed for enabled subscription");
 
 # Disable the subscription
@@ -239,8 +240,9 @@ $node_A->poll_query_until('postgres',
 # Enable retain_dead_tuples for disabled subscription
 ($cmdret, $stdout, $stderr) = $node_A->psql('postgres',
 	"ALTER SUBSCRIPTION $subname_AB SET (retain_dead_tuples = true);");
-ok( $stderr =~
-	  /NOTICE:  deleted rows to detect conflicts would not be removed until the subscription is enabled/,
+like(
+	$stderr,
+	qr/NOTICE:  deleted rows to detect conflicts would not be removed until the subscription is enabled/,
 	"altering retain_dead_tuples is allowed for disabled subscription");
 
 # Re-enable the subscription
@@ -262,9 +264,11 @@ ok( $node_A->poll_query_until(
 
 ($cmdret, $stdout, $stderr) = $node_A->psql('postgres',
 	"ALTER SUBSCRIPTION $subname_AB SET (origin = any);");
-ok( $stderr =~
-	  /WARNING:  subscription "tap_sub_a_b" enabled retain_dead_tuples but might not reliably detect conflicts for changes from different origins/,
-	"warn of the possibility of receiving changes from origins other than the publisher");
+like(
+	$stderr,
+	qr/WARNING:  subscription "tap_sub_a_b" enabled retain_dead_tuples but might not reliably detect conflicts for changes from different origins/,
+	"warn of the possibility of receiving changes from origins other than the publisher"
+);
 
 # Reset the origin to none
 $node_A->psql('postgres',
@@ -302,8 +306,9 @@ $node_A->safe_psql('postgres', "DELETE FROM tab WHERE a = 1;");
 	'postgres', qq(VACUUM (verbose) public.tab;)
 );
 
-ok( $stderr =~
-	  qr/1 are dead but not yet removable/,
+like(
+	$stderr,
+	qr/1 are dead but not yet removable/,
 	'the deleted column is non-removable');
 
 # Ensure the DELETE is replayed on Node B
@@ -311,8 +316,9 @@ $node_A->wait_for_catchup($subname_BA);
 
 # Check the conflict detected on Node B
 my $logfile = slurp_file($node_B->logfile(), $log_location);
-ok( $logfile =~
-	  qr/conflict detected on relation "public.tab": conflict=delete_origin_differs.*
+like(
+	$logfile,
+	qr/conflict detected on relation "public.tab": conflict=delete_origin_differs.*
 .*DETAIL:.* Deleting the row that was modified locally in transaction [0-9]+ at .*
 .*Existing local row \(1, 3\); replica identity \(a\)=\(1\)/,
 	'delete target row was modified in tab');
@@ -324,8 +330,9 @@ $node_A->safe_psql(
 $node_B->wait_for_catchup($subname_AB);
 
 $logfile = slurp_file($node_A->logfile(), $log_location);
-ok( $logfile =~
-	  qr/conflict detected on relation "public.tab": conflict=update_deleted.*
+like(
+	$logfile,
+	qr/conflict detected on relation "public.tab": conflict=update_deleted.*
 .*DETAIL:.* The row to be updated was deleted locally in transaction [0-9]+ at .*
 .*Remote row \(1, 3\); replica identity \(a\)=\(1\)/,
 	'update target row was deleted in tab');
@@ -371,8 +378,9 @@ $node_A->safe_psql(
 $node_B->wait_for_catchup($subname_AB);
 
 $logfile = slurp_file($node_A->logfile(), $log_location);
-ok( $logfile =~
-	  qr/conflict detected on relation "public.tab": conflict=update_deleted.*
+like(
+	$logfile,
+	qr/conflict detected on relation "public.tab": conflict=update_deleted.*
 .*DETAIL:.* The row to be updated was deleted locally in transaction [0-9]+ at .*
 .*Remote row \(2, 4\); replica identity full \(2, 2\)/,
 	'update target row was deleted in tab');
@@ -502,7 +510,9 @@ if ($injection_points_supported != 0)
 	($cmdret, $stdout, $stderr) =
 	  $node_A->psql('postgres', qq(VACUUM (verbose) public.tab;));
 
-	ok($stderr =~ qr/1 are dead but not yet removable/,
+	like(
+		$stderr,
+		qr/1 are dead but not yet removable/,
 		'the deleted column is non-removable');
 
 	$log_location = -s $node_A->logfile;
@@ -527,8 +537,9 @@ if ($injection_points_supported != 0)
 	$node_B->wait_for_catchup($subname_AB);
 
 	$logfile = slurp_file($node_A->logfile(), $log_location);
-	ok( $logfile =~
-		  qr/conflict detected on relation "public.tab": conflict=update_deleted.*
+	like(
+		$logfile,
+		qr/conflict detected on relation "public.tab": conflict=update_deleted.*
 .*DETAIL:.* The row to be updated was deleted locally in transaction [0-9]+ at .*
 .*Remote row \(1, 2\); replica identity full \(1, 1\)/,
 		'update target row was deleted in tab');