Subject: Re: Implement generalized sub routine find_in_log for tap test

From: Andrew Dunstan <[email-masked]>
Date: Sat, 27 May 2023 08:01:58 -0400

On 2023-05-26 Fr 20:35, vignesh C wrote:
> On Fri, 26 May 2023 at 04:09, Michael Paquier<[email-masked]>  wrote:
>> On Thu, May 25, 2023 at 06:34:20PM +0100, Dagfinn Ilmari Mannsåker wrote:
>>> However, none of the other functions in ::Utils know anything about node
>>> objects, which makes me think it should be a method on the node itself
>>> (i.e. in PostgreSQL::Test::Cluster) instead.  Also, I think log_contains
>>> would be a better name, since it just returns a boolean.  The name
>>> find_in_log makes me think it would return the log lines matching the
>>> pattern, or the position of the match in the file.
>>>
>>> In that case, the slurp_file() call would have to be fully qualified,
>>> since ::Cluster uses an empty import list to avoid polluting the method
>>> namespace with imported functions.
>> Hmm.  connect_ok() and connect_fails() in Cluster.pm have a similar
>> log comparison logic, feeding from the offset of a log file.  Couldn't
>> you use the same code across the board for everything?  Note that this
>> stuff is parameterized so as it is possible to check if patterns match
>> or do not match, for multiple patterns.  It seems to me that we could
>> use the new log finding routine there as well, so how about extending
>> it a bit more?  You would need, at least:
>> - One parameter for log entries matching.
>> - One parameter for log entries not matching.
> I felt adding these to log_contains was making the function slightly
> complex with multiple checks. I was not able to make it simple with
> the approach I tried. How about having a common function
> check_connect_log_contents which has the common log contents check for
> connect_ok and connect_fails function like the v2-0002 patch attached.

+    $offset = 0 unless defined $offset;

This is unnecessary, as slurp_file() handles it appropriately, and in 
fact doing this is slightly inefficient, as it will cause slurp_file to 
do a redundant seek.

FYI there's a simpler way to say it if we wanted to:

     $offset //= 0;

cheers

andrew

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

---

From: vignesh C <[email-masked]>
Date: Mon, 29 May 2023 07:49:52 +0530

On Sat, 27 May 2023 at 17:32, Andrew Dunstan <[email-masked]> wrote:
> On Fri, 26 May 2023 at 04:09, Michael Paquier <[email-masked]> wrote:
>
> This is unnecessary, as slurp_file() handles it appropriately, and in fact doing this is slightly inefficient, as it will cause slurp_file to do a redundant seek.
>

Thanks for the comment, the attached v3 version patch has the changes
for the same.

Regards,
Vignesh
From 02cda5bc291d2e951b971081981a336e22c74ec1 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email-masked]>
Date: Sat, 27 May 2023 05:36:34 +0530
Subject: [PATCH v3 2/2] Move common connection log content verification code
 to a common function.

Move common connection log content verification code to a common
function.
---
 src/test/perl/PostgreSQL/Test/Cluster.pm | 122 +++++++++++------------
 1 file changed, 60 insertions(+), 62 deletions(-)

diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 4df7dd4dec..912892e28b 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2168,21 +2168,19 @@ sub pgbench
 
 =pod
 
-=item $node->connect_ok($connstr, $test_name, %params)
+=item $node->check_connect_log_contents($offset, $test_name, %parameters)
 
-Attempt a connection with a custom connection string.  This is expected
-to succeed.
+Check connection log contents.
 
 =over
 
-=item sql => B<value>
+=item $test_name
 
-If this parameter is set, this query is used for the connection attempt
-instead of the default.
+Name of test for error messages.
 
-=item expected_stdout => B<value>
+=item $offset
 
-If this regular expression is set, matches it with the output generated.
+Offset of the log file.
 
 =item log_like => [ qr/required message/ ]
 
@@ -2200,6 +2198,58 @@ passed to C<Test::More::unlike()>.
 
 =cut
 
+sub check_connect_log_contents
+{
+	my ($self, $test_name, $offset, %params) = @_;
+
+	my (@log_like, @log_unlike);
+	if (defined($params{log_like}))
+	{
+		@log_like = @{ $params{log_like} };
+	}
+	if (defined($params{log_unlike}))
+	{
+		@log_unlike = @{ $params{log_unlike} };
+	}
+
+	if (@log_like or @log_unlike)
+	{
+		my $log_contents =
+		  PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset);
+
+		while (my $regex = shift @log_like)
+		{
+			like($log_contents, $regex, "$test_name: log matches");
+		}
+		while (my $regex = shift @log_unlike)
+		{
+			unlike($log_contents, $regex, "$test_name: log does not match");
+		}
+	}
+}
+
+=pod
+
+=item $node->connect_ok($connstr, $test_name, %params)
+
+Attempt a connection with a custom connection string.  This is expected
+to succeed.
+
+=over
+
+=item sql => B<value>
+
+If this parameter is set, this query is used for the connection attempt
+instead of the default.
+
+=item expected_stdout => B<value>
+
+If this regular expression is set, matches it with the output generated.
+
+=back
+
+=cut
+
 sub connect_ok
 {
 	local $Test::Builder::Level = $Test::Builder::Level + 1;
@@ -2215,16 +2265,6 @@ sub connect_ok
 		$sql = "SELECT \$\$connected with $connstr\$\$";
 	}
 
-	my (@log_like, @log_unlike);
-	if (defined($params{log_like}))
-	{
-		@log_like = @{ $params{log_like} };
-	}
-	if (defined($params{log_unlike}))
-	{
-		@log_unlike = @{ $params{log_unlike} };
-	}
-
 	my $log_location = -s $self->logfile;
 
 	# Never prompt for a password, any callers of this routine should
@@ -2245,20 +2285,7 @@ sub connect_ok
 
 	is($stderr, "", "$test_name: no stderr");
 
-	if (@log_like or @log_unlike)
-	{
-		my $log_contents =
-		  PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
-
-		while (my $regex = shift @log_like)
-		{
-			like($log_contents, $regex, "$test_name: log matches");
-		}
-		while (my $regex = shift @log_unlike)
-		{
-			unlike($log_contents, $regex, "$test_name: log does not match");
-		}
-	}
+	$self->check_connect_log_contents($test_name, $log_location, %params);
 }
 
 =pod
@@ -2274,12 +2301,6 @@ to fail.
 
 If this regular expression is set, matches it with the output generated.
 
-=item log_like => [ qr/required message/ ]
-
-=item log_unlike => [ qr/prohibited message/ ]
-
-See C<connect_ok(...)>, above.
-
 =back
 
 =cut
@@ -2289,16 +2310,6 @@ sub connect_fails
 	local $Test::Builder::Level = $Test::Builder::Level + 1;
 	my ($self, $connstr, $test_name, %params) = @_;
 
-	my (@log_like, @log_unlike);
-	if (defined($params{log_like}))
-	{
-		@log_like = @{ $params{log_like} };
-	}
-	if (defined($params{log_unlike}))
-	{
-		@log_unlike = @{ $params{log_unlike} };
-	}
-
 	my $log_location = -s $self->logfile;
 
 	# Never prompt for a password, any callers of this routine should
@@ -2316,20 +2327,7 @@ sub connect_fails
 		like($stderr, $params{expected_stderr}, "$test_name: matches");
 	}
 
-	if (@log_like or @log_unlike)
-	{
-		my $log_contents =
-		  PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
-
-		while (my $regex = shift @log_like)
-		{
-			like($log_contents, $regex, "$test_name: log matches");
-		}
-		while (my $regex = shift @log_unlike)
-		{
-			unlike($log_contents, $regex, "$test_name: log does not match");
-		}
-	}
+	$self->check_connect_log_contents($test_name, $log_location, %params);
 }
 
 =pod
-- 
2.34.1

From 12dea35bbb20d2ee09a3dfea1f67895d08110096 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email-masked]>
Date: Fri, 26 May 2023 20:07:30 +0530
Subject: [PATCH v3 1/2] Remove duplicate find_in_log sub routines from tap
 tests.

Remove duplicate find_in_log sub routines from tap tests.
---
 src/test/authentication/t/003_peer.pl         | 17 ++--------
 src/test/perl/PostgreSQL/Test/Cluster.pm      | 15 +++++++++
 src/test/recovery/t/019_replslot_limit.pl     | 33 +++++--------------
 src/test/recovery/t/033_replay_tsp_drops.pl   | 15 ++-------
 .../t/035_standby_logical_decoding.pl         | 26 +++------------
 5 files changed, 32 insertions(+), 74 deletions(-)

diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index 3272e52cae..2a035c2d0d 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -69,17 +69,6 @@ sub test_role
 	}
 }
 
-# Find $pattern in log file of $node.
-sub find_in_log
-{
-	my ($node, $offset, $pattern) = @_;
-
-	my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $offset);
-	return 0 if (length($log) <= 0);
-
-	return $log =~ m/$pattern/;
-}
-
 my $node = PostgreSQL::Test::Cluster->new('node');
 $node->init;
 $node->append_conf('postgresql.conf', "log_connections = on\n");
@@ -91,9 +80,9 @@ reset_pg_hba($node, 'peer');
 # Check if peer authentication is supported on this platform.
 my $log_offset = -s $node->logfile;
 $node->psql('postgres');
-if (find_in_log(
-		$node, $log_offset,
-		qr/peer authentication is not supported on this platform/))
+if ($node->log_contains(
+		qr/peer authentication is not supported on this platform/),
+		$log_offset,)
 {
 	plan skip_all => 'peer authentication is not supported on this platform';
 }
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index baea0fcd1c..4df7dd4dec 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2536,6 +2536,21 @@ sub log_content
 }

+=pod
+
+=item log_contains(pattern, offset)
+
+Find pattern in logfile of node after offset byte.
+
+=cut
+
+sub log_contains
+{
+	my ($self, $pattern, $offset) = @_;
+
+	return PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset) =~ m/$pattern/;
+}
+
 =pod
 
 =item $node->run_log(...)
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index a1aba16e14..95acf9e357 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -161,8 +161,7 @@ $node_primary->wait_for_catchup($node_standby);
 
 $node_standby->stop;
 
-ok( !find_in_log(
-		$node_standby,
+ok( !$node_standby->log_contains(
 		"requested WAL segment [0-9A-F]+ has already been removed"),
 	'check that required WAL segments are still available');
 
@@ -184,8 +183,8 @@ $node_primary->safe_psql('postgres', "CHECKPOINT;");
 my $invalidated = 0;
 for (my $i = 0; $i < 10 * $PostgreSQL::Test::Utils::timeout_default; $i++)
 {
-	if (find_in_log(
-			$node_primary, 'invalidating obsolete replication slot "rep1"',
+	if ($node_primary->log_contains(
+			'invalidating obsolete replication slot "rep1"',
 			$logstart))
 	{
 		$invalidated = 1;
@@ -207,7 +206,7 @@ is($result, "rep1|f|t|lost|",
 my $checkpoint_ended = 0;
 for (my $i = 0; $i < 10 * $PostgreSQL::Test::Utils::timeout_default; $i++)
 {
-	if (find_in_log($node_primary, "checkpoint complete: ", $logstart))
+	if ($node_primary->log_contains("checkpoint complete: ", $logstart))
 	{
 		$checkpoint_ended = 1;
 		last;
@@ -237,8 +236,7 @@ $node_standby->start;
 my $failed = 0;
 for (my $i = 0; $i < 10 * $PostgreSQL::Test::Utils::timeout_default; $i++)
 {
-	if (find_in_log(
-			$node_standby,
+	if ($node_standby->log_contains(
 			"requested WAL segment [0-9A-F]+ has already been removed",
 			$logstart))
 	{
@@ -381,8 +379,7 @@ my $msg_logged = 0;
 my $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
 while ($max_attempts-- >= 0)
 {
-	if (find_in_log(
-			$node_primary3,
+	if ($node_primary3->log_contains(
 			"terminating process $senderpid to release replication slot \"rep3\"",
 			$logstart))
 	{
@@ -406,8 +403,8 @@ $msg_logged = 0;
 $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
 while ($max_attempts-- >= 0)
 {
-	if (find_in_log(
-			$node_primary3, 'invalidating obsolete replication slot "rep3"',
+	if ($node_primary3->log_contains(
+			'invalidating obsolete replication slot "rep3"',
 			$logstart))
 	{
 		$msg_logged = 1;
@@ -446,18 +443,4 @@ sub get_log_size
 	return (stat $node->logfile)[7];
 }
 
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
-	my ($node, $pat, $off) = @_;
-
-	$off = 0 unless defined $off;
-	my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
-	return 0 if (length($log) <= $off);
-
-	$log = substr($log, $off);
-
-	return $log =~ m/$pat/;
-}
-
 done_testing();
diff --git a/src/test/recovery/t/033_replay_tsp_drops.pl b/src/test/recovery/t/033_replay_tsp_drops.pl
index 0a35a7bda6..307c30bc6b 100644
--- a/src/test/recovery/t/033_replay_tsp_drops.pl
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -135,22 +135,11 @@ while ($max_attempts-- >= 0)
 {
 	last
 	  if (
-		find_in_log(
-			$node_standby,
+		$node_standby->log_contains(
 			qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
 			$logstart));
 	usleep(100_000);
 }
 ok($max_attempts > 0, "invalid directory creation is detected");
 
-done_testing();
-
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
-	my ($node, $pat, $off) = @_;
-
-	my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $off);
-
-	return $log =~ m/$pat/;
-}
+done_testing();
\ No newline at end of file
diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl b/src/test/recovery/t/035_standby_logical_decoding.pl
index 64beec4bd3..480e6d6caa 100644
--- a/src/test/recovery/t/035_standby_logical_decoding.pl
+++ b/src/test/recovery/t/035_standby_logical_decoding.pl
@@ -28,20 +28,6 @@ my $res;
 my $primary_slotname = 'primary_physical';
 my $standby_physical_slotname = 'standby_physical';
 
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
-	my ($node, $pat, $off) = @_;
-
-	$off = 0 unless defined $off;
-	my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
-	return 0 if (length($log) <= $off);
-
-	$log = substr($log, $off);
-
-	return $log =~ m/$pat/;
-}
-
 # Fetch xmin columns from slot's pg_replication_slots row, after waiting for
 # given boolean condition to be true to ensure we've reached a quiescent state.
 sub wait_for_xmins
@@ -235,14 +221,12 @@ sub check_for_invalidation
 	my $inactive_slot = $slot_prefix . 'inactiveslot';
 
 	# message should be issued
-	ok( find_in_log(
-			$node_standby,
+	ok( $node_standby->log_contains(
 			"invalidating obsolete replication slot \"$inactive_slot\"",
 			$log_start),
 		"inactiveslot slot invalidation is logged $test_name");
 
-	ok( find_in_log(
-			$node_standby,
+	ok( $node_standby->log_contains(
 			"invalidating obsolete replication slot \"$active_slot\"",
 			$log_start),
 		"activeslot slot invalidation is logged $test_name");
@@ -657,14 +641,12 @@ $node_primary->safe_psql(
 $node_primary->wait_for_replay_catchup($node_standby);
 
 # message should not be issued
-ok( !find_in_log(
-		$node_standby,
+ok( !$node_standby->log_contains(
 		"invalidating obsolete slot \"no_conflict_inactiveslot\"", $logstart),
 	'inactiveslot slot invalidation is not logged with vacuum on conflict_test'
 );
 
-ok( !find_in_log(
-		$node_standby,
+ok( !$node_standby->log_contains(
 		"invalidating obsolete slot \"no_conflict_activeslot\"", $logstart),
 	'activeslot slot invalidation is not logged with vacuum on conflict_test'
 );
-- 
2.34.1

---

From: Michael Paquier <[email-masked]>
Date: Sat, 3 Jun 2023 18:21:27 -0400

-if (find_in_log(
-		$node, $log_offset,

This looks like a typo to me, the log offset is eaten.

Except of that, I am on board with log_contains().

There are two things that bugged me with the refactoring related to
connect_ok and connect_fails:
- check_connect_log_contents() is a name too complicated.  While
looking at that I have settled to a simpler log_check(), as we could
perfectly use that for something else than connections as long as we
want to check multiple patterns at once.
- The refactoring of the documentation for the routines of Cluster.pm
became incorrect.  For example, the patch does not list anymore
log_like and log_unlike for connect_ok.

With all that in mind I have hacked a few adjustments in a 0003,
though I agree with the separation between 0001 and 0002.

033_replay_tsp_drops and 019_replslot_limit are not new to v16, but
003_peer.pl and 035_standby_logical_decoding.pl, making the number of
places where find_in_log() exists twice as much.  So I would be
tempted to refactor these tests in v16.  Perhaps anybody from the RMT
could comment?  We've usually been quite flexible with the tests even
in beta.

Thoughts?
--
Michael
From 49473ab523adf43e82fe71d7e33be478287ad2c8 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email-masked]>
Date: Fri, 26 May 2023 20:07:30 +0530
Subject: [PATCH v4 1/3] Remove duplicate find_in_log sub routines from tap
 tests.

 

+=pod
+

 $node_standby->stop;
 
-ok( !find_in_log(
-		$node_standby,

 
-done_testing();
-

 
-	ok( find_in_log(
-			$node_standby,

-ok( !find_in_log(
-		$node_standby,
2.40.1

From 966ea0c337a187522bcabc877214761d9818b58a Mon Sep 17 00:00:00 2001
From: Vignesh C <[email-masked]>
Date: Sat, 27 May 2023 05:36:34 +0530
Subject: [PATCH v4 2/3] Move common connection log content verification code
 to a common function.

Move common connection log content verification code to a common
function.
---
 src/test/perl/PostgreSQL/Test/Cluster.pm | 130 +++++++++++------------
 1 file changed, 64 insertions(+), 66 deletions(-)

@@ -2168,6 +2168,68 @@ sub pgbench
 
 =pod
 
+=item $node->check_connect_log_contents($offset, $test_name, %parameters)
+
+Check connection log contents.
+
+=over
+
+=item $test_name
+
+Name of test for error messages.
+
+=item $offset
+
+Offset of the log file.
+
+=item log_like => [ qr/required message/ ]
+
+If given, it must be an array reference containing a list of regular
+expressions that must match against the server log, using
+C<Test::More::like()>.
+
+=item log_unlike => [ qr/prohibited message/ ]
+
+If given, it must be an array reference containing a list of regular
+expressions that must NOT match against the server log.  They will be
+passed to C<Test::More::unlike()>.
+
+=back
+
+=cut
+
+sub check_connect_log_contents
+{
+	my ($self, $test_name, $offset, %params) = @_;
+
+	my (@log_like, @log_unlike);
+	if (defined($params{log_like}))
+	{
+		@log_like = @{ $params{log_like} };
+	}
+	if (defined($params{log_unlike}))
+	{
+		@log_unlike = @{ $params{log_unlike} };
+	}
+
+	if (@log_like or @log_unlike)
+	{
+		my $log_contents =
+		  PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset);
+
+		while (my $regex = shift @log_like)
+		{
+			like($log_contents, $regex, "$test_name: log matches");
+		}
+		while (my $regex = shift @log_unlike)
+		{
+			unlike($log_contents, $regex, "$test_name: log does not match");
+		}
+	}
+}
+
+=pod
+
 =item $node->connect_ok($connstr, $test_name, %params)
 
 Attempt a connection with a custom connection string.  This is expected
@@ -2184,18 +2246,6 @@ instead of the default.

-=item log_like => [ qr/required message/ ]
-
-If given, it must be an array reference containing a list of regular
-expressions that must match against the server log, using
-C<Test::More::like()>.
-
-=item log_unlike => [ qr/prohibited message/ ]
-
-If given, it must be an array reference containing a list of regular
-expressions that must NOT match against the server log.  They will be
-passed to C<Test::More::unlike()>.
-
 =back
 
 =cut

 

 =pod

 
 =cut

 
 
 =pod
-- 
2.40.1

From fce041c5223d14b8bc654461e344f098df95c52a Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Sat, 3 Jun 2023 18:15:42 -0400
Subject: [PATCH v4 3/3] Adjust a bit previous patches ;)

---
 src/test/authentication/t/003_peer.pl       |   4 +-
 src/test/perl/PostgreSQL/Test/Cluster.pm    | 139 +++++++++++---------
 src/test/recovery/t/033_replay_tsp_drops.pl |   2 +-
 3 files changed, 78 insertions(+), 67 deletions(-)

diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index 2a035c2d0d..d8e4976072 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -81,8 +81,8 @@ reset_pg_hba($node, 'peer');
 my $log_offset = -s $node->logfile;
 $node->psql('postgres');
 if ($node->log_contains(
-		qr/peer authentication is not supported on this platform/),
-		$log_offset,)
+		qr/peer authentication is not supported on this platform/,
+		$log_offset))
 {
 	plan skip_all => 'peer authentication is not supported on this platform';
 }
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 912892e28b..19cbc46390 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2168,68 +2168,6 @@ sub pgbench
 
 =pod
 
-=item $node->check_connect_log_contents($offset, $test_name, %parameters)
-
-Check connection log contents.
-
-=over
-
-=item $test_name
-
-Name of test for error messages.
-
-=item $offset
-
-Offset of the log file.
-
-=item log_like => [ qr/required message/ ]
-
-If given, it must be an array reference containing a list of regular
-expressions that must match against the server log, using
-C<Test::More::like()>.
-
-=item log_unlike => [ qr/prohibited message/ ]
-
-If given, it must be an array reference containing a list of regular
-expressions that must NOT match against the server log.  They will be
-passed to C<Test::More::unlike()>.
-
-=back
-
-=cut
-
-sub check_connect_log_contents
-{
-	my ($self, $test_name, $offset, %params) = @_;
-
-	my (@log_like, @log_unlike);
-	if (defined($params{log_like}))
-	{
-		@log_like = @{ $params{log_like} };
-	}
-	if (defined($params{log_unlike}))
-	{
-		@log_unlike = @{ $params{log_unlike} };
-	}
-
-	if (@log_like or @log_unlike)
-	{
-		my $log_contents =
-		  PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset);
-
-		while (my $regex = shift @log_like)
-		{
-			like($log_contents, $regex, "$test_name: log matches");
-		}
-		while (my $regex = shift @log_unlike)
-		{
-			unlike($log_contents, $regex, "$test_name: log does not match");
-		}
-	}
-}
-
-=pod
-
 =item $node->connect_ok($connstr, $test_name, %params)
 
 Attempt a connection with a custom connection string.  This is expected
@@ -2246,6 +2184,12 @@ instead of the default.

+=item log_like => [ qr/required message/ ]
+
+=item log_unlike => [ qr/prohibited message/ ]
+
+See C<log_check(...)>.
+
 =back
 
 =cut
@@ -2285,7 +2229,7 @@ sub connect_ok

-	$self->check_connect_log_contents($test_name, $log_location, %params);
+	$self->log_check($test_name, $log_location, %params);
 }
 
 =pod
@@ -2301,6 +2245,12 @@ to fail.

+=item log_like => [ qr/required message/ ]
+
+=item log_unlike => [ qr/prohibited message/ ]
+
+See C<log_check(...)>.
+
 =back
 
 =cut
@@ -2327,7 +2277,7 @@ sub connect_fails
 		like($stderr, $params{expected_stderr}, "$test_name: matches");
 	}
 
-	$self->check_connect_log_contents($test_name, $log_location, %params);
+	$self->log_check($test_name, $log_location, %params);
 }
 
 =pod
@@ -2533,6 +2483,67 @@ sub log_content
 	return PostgreSQL::Test::Utils::slurp_file($self->logfile);
 }
 
+=pod
+
+=item $node->log_check($offset, $test_name, %parameters)
+
+Check contents of server logs.
+
+=over
+
+=item $test_name
+
+Name of test for error messages.
+
+=item $offset
+
+Offset of the log file.
+
+=item log_like => [ qr/required message/ ]
+
+If given, it must be an array reference containing a list of regular
+expressions that must match against the server log, using
+C<Test::More::like()>.
+
+=item log_unlike => [ qr/prohibited message/ ]
+
+If given, it must be an array reference containing a list of regular
+expressions that must NOT match against the server log.  They will be
+passed to C<Test::More::unlike()>.
+
+=back
+
+=cut
+
+sub log_check
+{
+	my ($self, $test_name, $offset, %params) = @_;
+
+	my (@log_like, @log_unlike);
+	if (defined($params{log_like}))
+	{
+		@log_like = @{ $params{log_like} };
+	}
+	if (defined($params{log_unlike}))
+	{
+		@log_unlike = @{ $params{log_unlike} };
+	}
+
+	if (@log_like or @log_unlike)
+	{
+		my $log_contents =
+		  PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset);
+
+		while (my $regex = shift @log_like)
+		{
+			like($log_contents, $regex, "$test_name: log matches");
+		}
+		while (my $regex = shift @log_unlike)
+		{
+			unlike($log_contents, $regex, "$test_name: log does not match");
+		}
+	}
+}
 
 =pod
 
diff --git a/src/test/recovery/t/033_replay_tsp_drops.pl b/src/test/recovery/t/033_replay_tsp_drops.pl
index 307c30bc6b..af97ed9f70 100644
--- a/src/test/recovery/t/033_replay_tsp_drops.pl
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -142,4 +142,4 @@ while ($max_attempts-- >= 0)
 }
 ok($max_attempts > 0, "invalid directory creation is detected");
 
-done_testing();
2.40.1

---

From: vignesh C <[email-masked]>
Date: Mon, 5 Jun 2023 21:39:22 +0530

> Except of that, I am on board with log_contains().

Thanks for fixing this.

> log_like and log_unlike for connect_ok.

This new name suggested by you looks simpler, your documentation of
having it in connect_ok and connect_fails and referring it to
log_check makes it more clearer.

Regards,
Vignesh

---

From: Amit Kapila <[email-masked]>
Date: Tue, 6 Jun 2023 08:05:49 +0530

> could comment?  We've usually been quite flexible with the tests even

Personally, I don't see any problem to do this refactoring for v16.
However, sometimes, we do decide to backpatch refactoring in tests to
avoid backpatch effort. I am not completely sure if that is the case
here.

-- 
With Regards,
Amit Kapila.

---

From: Michael Paquier <[email-masked]>
Date: Tue, 6 Jun 2023 13:06:33 +0900

> avoid backpatch effort. I am not completely sure if that is the case

033_replay_tsp_drops.pl has one find_in_log() down to 11, and
019_replslot_limit.pl has four calls down to 14.  Making things
consistent everywhere is a rather appealing argument to ease future
backpatching.  So I am OK to spend a few extra cycles in adjusting
these routines all the way down where needed.  I'll do that tomorrow
once I get back in front of my laptop.

Note that connect_ok() and connect_fails() are new to 14, so this
part has no need to go further down than that.
--
Michael

---

From: Bharath Rupireddy <[email-masked]>
Date: Tue, 6 Jun 2023 10:00:00 +0530

> > Except of that, I am on board with log_contains().

+1 for deduplicating find_in_log. How about deduplicating advance_wal
too so that 019_replslot_limit.pl, 033_replay_tsp_drops.pl,
035_standby_logical_decoding.pl and 001_stream_rep.pl can benefit
immediately?

FWIW, a previous discussion related to this is here
https://www.postgresql.org/message-id/flat/CALj2ACVUcXtLgHRPbx28ZQQyRM6j%2BeSH3jNUALr2pJ4%2Bf%3DHRGA%40mail.gmail.com.

-- 
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

---

From: Michael Paquier <[email-masked]>
Date: Tue, 6 Jun 2023 19:41:18 +0900

> 035_standby_logical_decoding.pl and 001_stream_rep.pl can benefit

As in a small wrapper for pg_switch_wal() that generates N segments at
will?  I don't see why we could not do that if it proves useful in the
long run.
--
Michael

---

From: Bharath Rupireddy <[email-masked]>
Date: Tue, 6 Jun 2023 17:53:40 +0530

On Tue, Jun 6, 2023 at 4:11 PM Michael Paquier <[email-masked]> wrote:
> will?

Yes. A simpler way of doing it would be to move advance_wal() in
019_replslot_limit.pl to Cluster.pm, something like the attached. CI
members don't complain with it
https://github.com/BRupireddy/postgres/tree/add_a_function_in_Cluster.pm_to_generate_WAL.
Perhaps, we can name it better instead of advance_wal, say
generate_wal or some other?

> I don't see why we could not do that if it proves useful in the
> long run.

Besides the beneficiaries listed above, the test case added by
https://commitfest.postgresql.org/43/3663/ can use it. And, the
test_table bits in 020_pg_receivewal.pl can use it (?).

---

From: vignesh C <[email-masked]>
Date: Tue, 6 Jun 2023 18:43:44 +0530

> part has no need to go further down than that.

Please find the attached patches that can be applied on back branches
too. v5*master.patch can be applied on master, v5*PG15.patch can be
applied on PG15, v5*PG14.patch can be applied on PG14, v5*PG13.patch
can be applied on PG13, v5*PG12.patch can be applied on PG12, PG11 and
PG10.

Regards,
Vignesh
From d582676214f3fd5068c343f42116fa256be18958 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email-masked]>
Date: Tue, 6 Jun 2023 17:07:37 +0530
Subject: [PATCH v5] Remove duplicate find_in_log sub routines from tap tests.

Remove duplicate find_in_log sub routines from tap tests.
---
 src/test/perl/PostgresNode.pm               | 15 +++++++++++++
 src/test/recovery/t/019_replslot_limit.pl   | 25 ++++-----------------
 src/test/recovery/t/033_replay_tsp_drops.pl | 14 ++----------
 3 files changed, 21 insertions(+), 33 deletions(-)

diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index b7e4dbd0e8..ce924de0b2 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -2081,6 +2081,21 @@ sub issues_sql_like
 
 =pod
 
+=item $node->log_contains(pattern, offset)
+
+Find pattern in logfile of node after offset byte.
+
+=cut
+
+sub log_contains
+{
+	my ($self, $pattern, $offset) = @_;
+
+	return TestLib::slurp_file($self->logfile, $offset) =~ m/$pattern/;
+}
+
+=pod
+
 =item $node->run_log(...)
 
 Runs a shell command like TestLib::run_log, but with connection parameters set
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index 7a8954dad2..d36c954588 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -165,8 +165,7 @@ $node_master->wait_for_catchup($node_standby, 'replay', $start_lsn);
 
 $node_standby->stop;
 
-ok( !find_in_log(
-		$node_standby,
 
@@ -188,8 +187,7 @@ $node_master->safe_psql('postgres', "CHECKPOINT;");
 my $invalidated = 0;
 for (my $i = 0; $i < 10000; $i++)
 {
-	if (find_in_log(
-			$node_master,
+	if ($node_master->log_contains(
 			"invalidating slot \"rep1\" because its restart_lsn [0-9A-F/]+ exceeds max_slot_wal_keep_size",
 			$logstart))
 	{
@@ -212,7 +210,7 @@ is($result, "rep1|f|t|lost|",
 my $checkpoint_ended = 0;
 for (my $i = 0; $i < 10000; $i++)
 {
-	if (find_in_log($node_master, "checkpoint complete: ", $logstart))
+	if ($node_master->log_contains("checkpoint complete: ", $logstart))
 	{
 		$checkpoint_ended = 1;
 		last;
@@ -242,8 +240,7 @@ $node_standby->start;
 my $failed = 0;
 for (my $i = 0; $i < 10000; $i++)
 {
-	if (find_in_log(
-			$node_standby,
+	if ($node_standby->log_contains(
 			"requested WAL segment [0-9A-F]+ has already been removed",
 			$logstart))
 	{
@@ -318,17 +315,3 @@ sub get_log_size
 
 	return (stat $node->logfile)[7];
 }
-
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
-	my ($node, $pat, $off) = @_;
-
-	$off = 0 unless defined $off;
-	my $log = TestLib::slurp_file($node->logfile);
-	return 0 if (length($log) <= $off);
-
-	$log = substr($log, $off);
-
-	return $log =~ m/$pat/;
-}
diff --git a/src/test/recovery/t/033_replay_tsp_drops.pl b/src/test/recovery/t/033_replay_tsp_drops.pl
index 140d94db31..4c8f4e7970 100644
--- a/src/test/recovery/t/033_replay_tsp_drops.pl
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -132,21 +132,11 @@ while ($max_attempts-- >= 0)
 {
 	last
 	  if (
-		find_in_log(
-			$node_standby, qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
+		$node_standby->log_contains(
+			qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
 			$logstart));
 	usleep(100_000);
 }
 ok($max_attempts > 0, "invalid directory creation is detected");
 
 done_testing();
-

From 7570e55a9f0121a9e2e0a12f4e8718cc531811c9 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email-masked]>
Date: Tue, 6 Jun 2023 17:20:34 +0530
Subject: [PATCH v5] Have find_in_log sub routine as a common routine available
 for all tap tests.

Have find_in_log sub routine as a common routine available for all tap
tests.
---
 src/test/perl/PostgresNode.pm               | 15 +++++++++++++++
 src/test/recovery/t/033_replay_tsp_drops.pl | 14 ++------------
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 6506308ff1..8fae4eb1bb 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -1972,6 +1972,21 @@ sub issues_sql_like
 
 =pod
 
+=item $node->log_contains(pattern, offset)
+
+Find pattern in logfile of node after offset byte.
+
+=cut
+
+sub log_contains
+{
+	my ($self, $pattern, $offset) = @_;
+
+	return TestLib::slurp_file($self->logfile, $offset) =~ m/$pattern/;
+}
+
+=pod
+
 =item $node->run_log(...)
 
 Runs a shell command like TestLib::run_log, but with connection parameters set
diff --git a/src/test/recovery/t/033_replay_tsp_drops.pl b/src/test/recovery/t/033_replay_tsp_drops.pl
index 140d94db31..4c8f4e7970 100644
--- a/src/test/recovery/t/033_replay_tsp_drops.pl
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -132,21 +132,11 @@ while ($max_attempts-- >= 0)
 {
 	last
 	  if (
-		find_in_log(
-			$node_standby, qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
+		$node_standby->log_contains(
+			qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
 			$logstart));
 	usleep(100_000);
 }
 ok($max_attempts > 0, "invalid directory creation is detected");
 
 done_testing();
-

From 94a9c3e62df5c4044271bada78af8fa5869f0a26 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email-masked]>
Date: Tue, 6 Jun 2023 16:40:17 +0530
Subject: [PATCH v5 1/2] Remove duplicate find_in_log sub routines from tap
 tests.

Remove duplicate find_in_log sub routines from tap tests.
---
 src/test/perl/PostgresNode.pm               | 15 ++++++++++
 src/test/recovery/t/019_replslot_limit.pl   | 31 ++++-----------------
 src/test/recovery/t/033_replay_tsp_drops.pl | 14 ++--------
 3 files changed, 23 insertions(+), 37 deletions(-)

diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index cce3f621e7..b48bf17a40 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -2459,6 +2459,21 @@ sub issues_sql_like
 
 =pod
 
+=item $node->log_contains(pattern, offset)
+
+Find pattern in logfile of node after offset byte.
+
+=cut
+
+sub log_contains
+{
+	my ($self, $pattern, $offset) = @_;
+
+	return TestLib::slurp_file($self->logfile, $offset) =~ m/$pattern/;
+}
+
+=pod
+
 =item $node->run_log(...)
 
 Runs a shell command like TestLib::run_log, but with connection parameters set
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index f6dcf0af1d..1eec98f2c9 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -168,8 +168,7 @@ $node_primary->wait_for_catchup($node_standby, 'replay', $start_lsn);
 
 $node_standby->stop;
 
-ok( !find_in_log(
-		$node_standby,
 
@@ -191,8 +190,7 @@ $node_primary->safe_psql('postgres', "CHECKPOINT;");
 my $invalidated = 0;
 for (my $i = 0; $i < 10000; $i++)
 {
-	if (find_in_log(
-			$node_primary,
+	if ($node_primary->log_contains(
 			"invalidating slot \"rep1\" because its restart_lsn [0-9A-F/]+ exceeds max_slot_wal_keep_size",
 			$logstart))
 	{
@@ -215,7 +213,7 @@ is($result, "rep1|f|t|lost|",
 my $checkpoint_ended = 0;
 for (my $i = 0; $i < 10000; $i++)
 {
-	if (find_in_log($node_primary, "checkpoint complete: ", $logstart))
+	if ($node_primary->log_contains("checkpoint complete: ", $logstart))
 	{
 		$checkpoint_ended = 1;
 		last;
@@ -245,8 +243,7 @@ $node_standby->start;
 my $failed = 0;
 for (my $i = 0; $i < 10000; $i++)
 {
-	if (find_in_log(
-			$node_standby,
+	if ($node_standby->log_contains(
 			"requested WAL segment [0-9A-F]+ has already been removed",
 			$logstart))
 	{
@@ -351,8 +348,7 @@ advance_wal($node_primary3, 2);
 my $max_attempts = $TestLib::timeout_default;
 while ($max_attempts-- >= 0)
 {
-	if (find_in_log(
-			$node_primary3,
+	if ($node_primary3->log_contains(
 			"terminating process $senderpid to release replication slot \"rep3\"",
 			$logstart))
 	{
@@ -374,8 +370,7 @@ $node_primary3->poll_query_until('postgres',
 $max_attempts = $TestLib::timeout_default;
 while ($max_attempts-- >= 0)
 {
-	if (find_in_log(
-			$node_primary3,
+	if ($node_primary3->log_contains(
 			'invalidating slot "rep3" because its restart_lsn', $logstart))
 	{
 		ok(1, "slot invalidation logged");
@@ -412,17 +407,3 @@ sub get_log_size
 
 	return (stat $node->logfile)[7];
 }
-
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
-	my ($node, $pat, $off) = @_;
-
-	$off = 0 unless defined $off;
-	my $log = TestLib::slurp_file($node->logfile);
-	return 0 if (length($log) <= $off);
-
-	$log = substr($log, $off);
-
-	return $log =~ m/$pat/;
-}
diff --git a/src/test/recovery/t/033_replay_tsp_drops.pl b/src/test/recovery/t/033_replay_tsp_drops.pl
index 140d94db31..4c8f4e7970 100644
--- a/src/test/recovery/t/033_replay_tsp_drops.pl
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -132,21 +132,11 @@ while ($max_attempts-- >= 0)
 {
 	last
 	  if (
-		find_in_log(
-			$node_standby, qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
+		$node_standby->log_contains(
+			qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
 			$logstart));
 	usleep(100_000);
 }
 ok($max_attempts > 0, "invalid directory creation is detected");
 
 done_testing();
-

From e831e26c06994c656b61a99dee86c4710d5e37a1 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email-masked]>
Date: Fri, 26 May 2023 20:07:30 +0530
Subject: [PATCH v5 1/2] Remove duplicate find_in_log sub routines from tap
 tests.

Remove duplicate find_in_log sub routines from tap tests.
---
 src/test/authentication/t/003_peer.pl         | 17 ++--------
 src/test/perl/PostgreSQL/Test/Cluster.pm      | 15 +++++++++
 src/test/recovery/t/019_replslot_limit.pl     | 33 +++++--------------
 src/test/recovery/t/033_replay_tsp_drops.pl   | 13 +-------
 .../t/035_standby_logical_decoding.pl         | 26 +++------------
 5 files changed, 31 insertions(+), 73 deletions(-)

index 3272e52cae..d8e4976072 100644

+=pod
+

 $node_standby->stop;
 
-ok( !find_in_log(
-		$node_standby,

index 0a35a7bda6..af97ed9f70 100644
@@ -135,8 +135,7 @@ while ($max_attempts-- >= 0)
@@ -144,13 +143,3 @@ while ($max_attempts-- >= 0)
 
 done_testing();
-

 
-	ok( find_in_log(
-			$node_standby,

-ok( !find_in_log(
-		$node_standby,

From a91e8b12840dfe9aaea425f1cf0255d5849cbcb0 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email-masked]>
Date: Tue, 6 Jun 2023 15:48:50 +0530
Subject: [PATCH v5 1/2] Remove duplicate find_in_log sub routines from tap
 tests.

Remove duplicate find_in_log sub routines from tap tests.
---
 src/test/perl/PostgreSQL/Test/Cluster.pm    | 15 ++++++++++
 src/test/recovery/t/019_replslot_limit.pl   | 31 ++++-----------------
 src/test/recovery/t/033_replay_tsp_drops.pl | 14 ++--------
 3 files changed, 23 insertions(+), 37 deletions(-)

diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 69a8ff3d74..5063e77368 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2559,6 +2559,21 @@ sub issues_sql_like
 
 =pod

 Runs a shell command like PostgreSQL::Test::Utils::run_log, but with connection parameters set
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index 84c9c4b0b3..4ec1a9ab33 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -163,8 +163,7 @@ $node_primary->wait_for_catchup($node_standby);
 
 $node_standby->stop;
 
-ok( !find_in_log(
-		$node_standby,
 
@@ -186,8 +185,7 @@ $node_primary->safe_psql('postgres', "CHECKPOINT;");
 my $invalidated = 0;
 for (my $i = 0; $i < 10000; $i++)
 {
-	if (find_in_log(
-			$node_primary,
+	if ($node_primary->log_contains(
 			"invalidating slot \"rep1\" because its restart_lsn [0-9A-F/]+ exceeds max_slot_wal_keep_size",
 			$logstart))
 	{
@@ -210,7 +208,7 @@ is($result, "rep1|f|t|lost|",
 my $checkpoint_ended = 0;
 for (my $i = 0; $i < 10000; $i++)
 {
-	if (find_in_log($node_primary, "checkpoint complete: ", $logstart))
+	if ($node_primary->log_contains("checkpoint complete: ", $logstart))
 	{
 		$checkpoint_ended = 1;
 		last;
@@ -240,8 +238,7 @@ $node_standby->start;
 my $failed = 0;
 for (my $i = 0; $i < 10000; $i++)
 {
-	if (find_in_log(
-			$node_standby,
+	if ($node_standby->log_contains(
 			"requested WAL segment [0-9A-F]+ has already been removed",
 			$logstart))
 	{
@@ -384,8 +381,7 @@ advance_wal($node_primary3, 2);
 my $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
 while ($max_attempts-- >= 0)
 {
-	if (find_in_log(
-			$node_primary3,
+	if ($node_primary3->log_contains(
 			"terminating process $senderpid to release replication slot \"rep3\"",
 			$logstart))
 	{
@@ -407,8 +403,7 @@ $node_primary3->poll_query_until('postgres',
 $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
 while ($max_attempts-- >= 0)
 {
-	if (find_in_log(
-			$node_primary3,
+	if ($node_primary3->log_contains(
 			'invalidating slot "rep3" because its restart_lsn', $logstart))
 	{
 		ok(1, "slot invalidation logged");
@@ -446,18 +441,4 @@ sub get_log_size
 	return (stat $node->logfile)[7];
 }
 
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
-	my ($node, $pat, $off) = @_;
-
-	$off = 0 unless defined $off;
-	my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
-	return 0 if (length($log) <= $off);
-
-	$log = substr($log, $off);
-
-	return $log =~ m/$pat/;
-}
-
 done_testing();
diff --git a/src/test/recovery/t/033_replay_tsp_drops.pl b/src/test/recovery/t/033_replay_tsp_drops.pl
index 57fee21276..64c3296a70 100644
--- a/src/test/recovery/t/033_replay_tsp_drops.pl
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -138,21 +138,11 @@ while ($max_attempts-- >= 0)
 {
 	last
 	  if (
-		find_in_log(
-			$node_standby, qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
+		$node_standby->log_contains(
+			qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
 			$logstart));
 	usleep(100_000);
 }
 ok($max_attempts > 0, "invalid directory creation is detected");
 
 done_testing();
-

From f96a537129a740fe8998e7201ac052d3e63d4129 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email-masked]>
Date: Sat, 27 May 2023 05:36:34 +0530
Subject: [PATCH v5 2/2] Move common connection log content verification code
 to a common function.

Move common connection log content verification code to a common
function.
---
 src/test/perl/PostgreSQL/Test/Cluster.pm | 121 ++++++++++++-----------
 1 file changed, 65 insertions(+), 56 deletions(-)

diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 4df7dd4dec..19cbc46390 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2186,15 +2186,9 @@ If this regular expression is set, matches it with the output generated.

-If given, it must be an array reference containing a list of regular
-expressions that must match against the server log, using
-C<Test::More::like()>.
-
 =item log_unlike => [ qr/prohibited message/ ]

 =back
 
@@ -2215,16 +2209,6 @@ sub connect_ok
 		$sql = "SELECT \$\$connected with $connstr\$\$";
 	}

 	# Never prompt for a password, any callers of this routine should
@@ -2245,20 +2229,7 @@ sub connect_ok

 
 =pod
@@ -2278,7 +2249,7 @@ If this regular expression is set, matches it with the output generated.
 
 =item log_unlike => [ qr/prohibited message/ ]

 =back
 
@@ -2289,16 +2260,6 @@ sub connect_fails
 	local $Test::Builder::Level = $Test::Builder::Level + 1;
 	my ($self, $connstr, $test_name, %params) = @_;

 	# Never prompt for a password, any callers of this routine should
@@ -2316,20 +2277,7 @@ sub connect_fails
 		like($stderr, $params{expected_stderr}, "$test_name: matches");
 	}

 =pod
@@ -2535,6 +2483,67 @@ sub log_content
 	return PostgreSQL::Test::Utils::slurp_file($self->logfile);
 }
 
+=pod
+
 
 =pod
 
-- 
2.34.1

From 16cabb908af170b90cf971f7e47a69712aa2720e Mon Sep 17 00:00:00 2001
From: Vignesh C <[email-masked]>
Date: Tue, 6 Jun 2023 16:51:56 +0530
Subject: [PATCH v5 2/2] Move common connection log content verification code
 to a common function.

Move common connection log content verification code to a common
function.
---
 src/test/perl/PostgresNode.pm | 117 +++++++++++++++++++---------------
 1 file changed, 65 insertions(+), 52 deletions(-)

diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index b48bf17a40..22baa4cf80 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -2148,15 +2148,9 @@ If this regular expression is set, matches it with the output generated.

-If given, it must be an array reference containing a list of regular
-expressions that must match against the server log, using
-C<Test::More::like()>.
-
 =item log_unlike => [ qr/prohibited message/ ]

 =back
 
@@ -2177,16 +2171,6 @@ sub connect_ok
 		$sql = "SELECT \$\$connected with $connstr\$\$";
 	}

 	# Never prompt for a password, any callers of this routine should
@@ -2204,19 +2188,7 @@ sub connect_ok
 	{
 		like($stdout, $params{expected_stdout}, "$test_name: matches");
 	}
-	if (@log_like or @log_unlike)
-	{
-		my $log_contents = TestLib::slurp_file($self->logfile, $log_location);
-
-		while (my $regex = shift @log_like)
-		{
-			like($log_contents, $regex, "$test_name: log matches");
-		}
-		while (my $regex = shift @log_unlike)
-		{
-			unlike($log_contents, $regex, "$test_name: log does not match");
-		}
-	}
+	$self->log_check($test_name, $log_location, %params);
 }
 
 =pod
@@ -2236,7 +2208,7 @@ If this regular expression is set, matches it with the output generated.
 
 =item log_unlike => [ qr/prohibited message/ ]

 =back
 
@@ -2248,14 +2220,6 @@ sub connect_fails
 	my ($self, $connstr, $test_name, %params) = @_;
 
 	my (@log_like, @log_unlike);

@@ -2274,19 +2238,7 @@ sub connect_fails
 		like($stderr, $params{expected_stderr}, "$test_name: matches");
 	}
 
-		my $log_contents = TestLib::slurp_file($self->logfile, $log_location);
 
 =pod
@@ -2459,6 +2411,67 @@ sub issues_sql_like
 
 =pod
 
+		my $log_contents = TestLib::slurp_file($self->logfile, $offset);
 =item $node->log_contains(pattern, offset)
 
 Find pattern in logfile of node after offset byte.
-- 
2.34.1

From 27d621e922717b01171d12a28d96d699596b992a Mon Sep 17 00:00:00 2001
From: Vignesh C <[email-masked]>
Date: Tue, 6 Jun 2023 15:55:56 +0530
Subject: [PATCH v5 2/2] Move common connection log content verification code
 to a common function.

Move common connection log content verification code to a common
function.
---
 src/test/perl/PostgreSQL/Test/Cluster.pm | 122 ++++++++++++-----------
 1 file changed, 66 insertions(+), 56 deletions(-)

diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 5063e77368..0d7a1bcfbc 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2222,15 +2222,9 @@ If this regular expression is set, matches it with the output generated.

-If given, it must be an array reference containing a list of regular
-expressions that must match against the server log, using
-C<Test::More::like()>.
-
 =item log_unlike => [ qr/prohibited message/ ]

 =back
 
@@ -2251,16 +2245,6 @@ sub connect_ok
 		$sql = "SELECT \$\$connected with $connstr\$\$";
 	}

 	# Never prompt for a password, any callers of this routine should
@@ -2281,20 +2265,7 @@ sub connect_ok

 
 =pod
@@ -2314,7 +2285,7 @@ If this regular expression is set, matches it with the output generated.
 
 =item log_unlike => [ qr/prohibited message/ ]

 =back
 
@@ -2325,16 +2296,6 @@ sub connect_fails
 	local $Test::Builder::Level = $Test::Builder::Level + 1;
 	my ($self, $connstr, $test_name, %params) = @_;

 	# Never prompt for a password, any callers of this routine should
@@ -2352,20 +2313,7 @@ sub connect_fails
 		like($stderr, $params{expected_stderr}, "$test_name: matches");
 	}

 =pod
@@ -2559,6 +2507,68 @@ sub issues_sql_like
 
 =pod
 
 =item log_contains(pattern, offset)
 
 Find pattern in logfile of node after offset byte.
-- 
2.34.1

---

From: Michael Paquier <[email-masked]>
Date: Fri, 9 Jun 2023 11:59:03 +0900

> generate_wal or some other?

Why not discussing that on a separate thread?  What you are proposing
is independent of what Vignesh has proposed.  Note that the patch
format is octet-stream, causing extra CRs to exist in the patch.
Something happened on your side when you sent your patch, I guess?
--
Michael

---

From: Michael Paquier <[email-masked]>
Date: Fri, 9 Jun 2023 12:01:24 +0900

> can be applied on PG13, v5*PG12.patch can be applied on PG12, PG11 and

Thanks.  The amount of minimal conflicts across all these branches is
always fun to play with.  I have finally got around and applied all
that, after doing a proper split, applying one part down to 14 and the
second back to 11.
--
Michael

---

From: Bharath Rupireddy <[email-masked]>
Date: Sun, 11 Jun 2023 07:17:55 +0530

On Fri, Jun 9, 2023 at 8:29 AM Michael Paquier <[email-masked]> wrote:
> is independent of what Vignesh has proposed.

Sure. Here it is  -
https://www.postgresql.org/message-id/CALj2ACU3R8QFCvDewHCMKjgb2w_-CMCyd6DAK%3DJb-af14da5eg%40mail.gmail.com.

> Note that the patch

Had to attach the patch in .txt format to not block Vignesh's patch
from testing by CF Bot (if at all this thread was added there).

---

From: vignesh C <[email-masked]>
Date: Sun, 11 Jun 2023 08:42:04 +0530

> that, after doing a proper split, applying one part down to 14 and the

Thanks for pushing this.

Regards,
Vignesh