Implement generalized sub routine find_in_log for tap test
Started by vignesh Cover 2 years ago19 messages
Hi,
The recovery tap test has 4 implementations of find_in_log sub routine
for various uses, I felt we can generalize these and have a single
function for the same. The attached patch is an attempt to have a
generalized sub routine find_in_log which can be used by all of them.
Thoughts?
Regards,
VIgnesh
Attachments:
0001-Remove-duplicate-find_in_log-sub-routines-from-tap-t.patchtext/x-patch; charset=US-ASCII; name=0001-Remove-duplicate-find_in_log-sub-routines-from-tap-t.patchDownload
From 7c4b6731a27c031ad27f6f88150c1dfaae1f46fe Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
Date: Thu, 25 May 2023 10:47:16 +0530
Subject: [PATCH] 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 | 15 ++----------
src/test/perl/PostgreSQL/Test/Utils.pm | 23 +++++++++++++++++++
src/test/recovery/t/019_replslot_limit.pl | 14 -----------
src/test/recovery/t/033_replay_tsp_drops.pl | 12 +---------
.../t/035_standby_logical_decoding.pl | 14 -----------
5 files changed, 26 insertions(+), 52 deletions(-)
diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index 3272e52cae..a8ff6e8642 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");
@@ -92,8 +81,8 @@ reset_pg_hba($node, 'peer');
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/))
+ $node, 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/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm
index a27fac83d2..5c9b2f6c03 100644
--- a/src/test/perl/PostgreSQL/Test/Utils.pm
+++ b/src/test/perl/PostgreSQL/Test/Utils.pm
@@ -67,6 +67,7 @@ our @EXPORT = qw(
slurp_file
append_to_file
string_replace_file
+ find_in_log
check_mode_recursive
chmod_recursive
check_pg_config
@@ -579,6 +580,28 @@ sub string_replace_file
=pod
+
+=item find_in_log(node, pattern, offset)
+
+Find pattern in logfile of node after offset byte.
+
+=cut
+
+sub find_in_log
+{
+ my ($node, $pattern, $offset) = @_;
+
+ $offset = 0 unless defined $offset;
+ my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
+
+ return 0 if (length($log) <= 0 || length($log) <= $offset);
+
+ $log = substr($log, $offset);
+ return $log =~ m/$pattern/;
+}
+
+=pod
+
=item check_mode_recursive(dir, expected_dir_mode, expected_file_mode, ignore_list)
Check that all file/dir modes in a directory match the expected values,
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index a1aba16e14..ebd15a0ec4 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -446,18 +446,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..5f1287cd90 100644
--- a/src/test/recovery/t/033_replay_tsp_drops.pl
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -143,14 +143,4 @@ while ($max_attempts-- >= 0)
}
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..31e347ce90 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
--
2.34.1
Re: Implement generalized sub routine find_in_log for tap test
vignesh C <vignesh21@gmail.com> writes:
Hi,
The recovery tap test has 4 implementations of find_in_log sub routine
for various uses, I felt we can generalize these and have a single
function for the same. The attached patch is an attempt to have a
generalized sub routine find_in_log which can be used by all of them.
Thoughts?
+1 on factoring out this common code. Just a few comments on the implementation.
diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm index a27fac83d2..5c9b2f6c03 100644 --- a/src/test/perl/PostgreSQL/Test/Utils.pm +++ b/src/test/perl/PostgreSQL/Test/Utils.pm @@ -67,6 +67,7 @@ our @EXPORT = qw( slurp_file append_to_file string_replace_file + find_in_log check_mode_recursive chmod_recursive check_pg_config @@ -579,6 +580,28 @@ sub string_replace_file=pod
+ +=item find_in_log(node, pattern, offset) + +Find pattern in logfile of node after offset byte. + +=cut + +sub find_in_log +{ + my ($node, $pattern, $offset) = @_; + + $offset = 0 unless defined $offset; + my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
Since this function is in the same package, there's no need to qualify
it with the full name. I know the callers you copied it from did, but
they wouldn't have had to either, since it's exported by default (in the
@EXPORT array above), unless the use statement has an explicit argument
list that excludes it.
+ return 0 if (length($log) <= 0 || length($log) <= $offset); + + $log = substr($log, $offset);
Also, the existing callers don't seem to have got the memo that
slurp_file() takes an optinal offset parameter, which will cause it to
seek to that postion before slurping the file, which is more efficient
than reading the whole file in and substr-ing it. There's not much
point in the length checks either, since regex-matching against an empty
string is very cheap (and if the provide pattern can match the empty
string the whole function call is rather pointless).
+ return $log =~ m/$pattern/;
+}
All in all, it could be simplified to:
sub find_in_log {
my ($node, $pattern, $offset) = @_;
return slurp_file($node->logfile, $offset) =~ $pattern;
}
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.
- ilmari
Re: Implement generalized sub routine find_in_log for tap test
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.
--
Michael
Re: Implement generalized sub routine find_in_log for tap test
On Thu, 25 May 2023 at 23:04, Dagfinn Ilmari Mannsåker
<ilmari@ilmari.org> wrote:
vignesh C <vignesh21@gmail.com> writes:
Hi,
The recovery tap test has 4 implementations of find_in_log sub routine
for various uses, I felt we can generalize these and have a single
function for the same. The attached patch is an attempt to have a
generalized sub routine find_in_log which can be used by all of them.
Thoughts?+1 on factoring out this common code. Just a few comments on the implementation.
diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm index a27fac83d2..5c9b2f6c03 100644 --- a/src/test/perl/PostgreSQL/Test/Utils.pm +++ b/src/test/perl/PostgreSQL/Test/Utils.pm @@ -67,6 +67,7 @@ our @EXPORT = qw( slurp_file append_to_file string_replace_file + find_in_log check_mode_recursive chmod_recursive check_pg_config @@ -579,6 +580,28 @@ sub string_replace_file=pod
+ +=item find_in_log(node, pattern, offset) + +Find pattern in logfile of node after offset byte. + +=cut + +sub find_in_log +{ + my ($node, $pattern, $offset) = @_; + + $offset = 0 unless defined $offset; + my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);Since this function is in the same package, there's no need to qualify
it with the full name. I know the callers you copied it from did, but
they wouldn't have had to either, since it's exported by default (in the
@EXPORT array above), unless the use statement has an explicit argument
list that excludes it.
I have moved this function to Cluster.pm file now, since it is moved,
I had to qualify the name with the full name.
+ return 0 if (length($log) <= 0 || length($log) <= $offset); + + $log = substr($log, $offset);Also, the existing callers don't seem to have got the memo that
slurp_file() takes an optinal offset parameter, which will cause it to
seek to that postion before slurping the file, which is more efficient
than reading the whole file in and substr-ing it. There's not much
point in the length checks either, since regex-matching against an empty
string is very cheap (and if the provide pattern can match the empty
string the whole function call is rather pointless).+ return $log =~ m/$pattern/;
+}All in all, it could be simplified to:
sub find_in_log {
my ($node, $pattern, $offset) = @_;return slurp_file($node->logfile, $offset) =~ $pattern;
}
Modified in similar lines
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.
Modified
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.
Modified.
Thanks for the comments, the attached v2 version patch has the changes
for the same.
Regards,
Vignesh
Attachments:
v2-0001-Remove-duplicate-find_in_log-sub-routines-from-ta.patchtext/x-patch; charset=US-ASCII; name=v2-0001-Remove-duplicate-find_in_log-sub-routines-from-ta.patchDownload
From 8df06d233de4e5b5abe7a4dd4bc314c215b2dfc2 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
Date: Fri, 26 May 2023 20:07:30 +0530
Subject: [PATCH v2] 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 | 16 +++++++++
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, 33 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..f10e44ba51 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2536,6 +2536,22 @@ 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) = @_;
+
+ $offset = 0 unless defined $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
Re: Implement generalized sub routine find_in_log for tap test
On Fri, 26 May 2023 at 04:09, Michael Paquier <michael@paquier.xyz> 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.
Regards,
Vignesh
Attachments:
v2-0001-Remove-duplicate-find_in_log-sub-routines-from-ta.patchtext/x-patch; charset=US-ASCII; name=v2-0001-Remove-duplicate-find_in_log-sub-routines-from-ta.patchDownload
From 1e835b6d032ea380bc213ff130e87fbb250be77f Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
Date: Fri, 26 May 2023 20:07:30 +0530
Subject: [PATCH v2 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 | 16 +++++++++
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, 33 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..f10e44ba51 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2536,6 +2536,22 @@ 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) = @_;
+
+ $offset = 0 unless defined $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
v2-0002-Move-common-connection-log-content-verification-c.patchtext/x-patch; charset=US-ASCII; name=v2-0002-Move-common-connection-log-content-verification-c.patchDownload
From aa587e7cedd92d60e93075d643cb8ad93ca07c5b Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
Date: Sat, 27 May 2023 05:36:34 +0530
Subject: [PATCH v2 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 f10e44ba51..7458406ee6 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
Re: Implement generalized sub routine find_in_log for tap test
On 2023-05-26 Fr 20:35, vignesh C wrote:
On Fri, 26 May 2023 at 04:09, Michael Paquier<michael@paquier.xyz> 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
Re: Implement generalized sub routine find_in_log for tap test
On Sat, 27 May 2023 at 17:32, Andrew Dunstan <andrew@dunslane.net> wrote:
On 2023-05-26 Fr 20:35, vignesh C wrote:
On Fri, 26 May 2023 at 04:09, Michael Paquier <michael@paquier.xyz> 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;
Thanks for the comment, the attached v3 version patch has the changes
for the same.
Regards,
Vignesh
Attachments:
v3-0002-Move-common-connection-log-content-verification-c.patchtext/x-patch; charset=US-ASCII; name=v3-0002-Move-common-connection-log-content-verification-c.patchDownload
From 02cda5bc291d2e951b971081981a336e22c74ec1 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
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
v3-0001-Remove-duplicate-find_in_log-sub-routines-from-ta.patchtext/x-patch; charset=US-ASCII; name=v3-0001-Remove-duplicate-find_in_log-sub-routines-from-ta.patchDownload
From 12dea35bbb20d2ee09a3dfea1f67895d08110096 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
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
Re: Implement generalized sub routine find_in_log for tap test
On Mon, May 29, 2023 at 07:49:52AM +0530, vignesh C wrote:
Thanks for the comment, the attached v3 version patch has the changes
for the same.
-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,)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
Attachments:
v4-0001-Remove-duplicate-find_in_log-sub-routines-from-ta.patchtext/plain; charset=us-asciiDownload
From 49473ab523adf43e82fe71d7e33be478287ad2c8 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
Date: Fri, 26 May 2023 20:07:30 +0530
Subject: [PATCH v4 1/3] 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.40.1
v4-0002-Move-common-connection-log-content-verification-c.patchtext/plain; charset=us-asciiDownload
From 966ea0c337a187522bcabc877214761d9818b58a Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
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(-)
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,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.
If this regular expression is set, matches it with the output generated.
-=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
@@ -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.40.1
v4-0003-Adjust-a-bit-previous-patches.patchtext/plain; charset=us-asciiDownload
From fce041c5223d14b8bc654461e344f098df95c52a Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
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.
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<log_check(...)>.
+
=back
=cut
@@ -2285,7 +2229,7 @@ sub connect_ok
is($stderr, "", "$test_name: no stderr");
- $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.
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<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();
\ No newline at end of file
+done_testing();
--
2.40.1
Re: Implement generalized sub routine find_in_log for tap test
On Sun, 4 Jun 2023 at 03:51, Michael Paquier <michael@paquier.xyz> wrote:
On Mon, May 29, 2023 at 07:49:52AM +0530, vignesh C wrote:
Thanks for the comment, the attached v3 version patch has the changes
for the same.-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,)This looks like a typo to me, the log offset is eaten.
Except of that, I am on board with log_contains().
Thanks for fixing this.
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.
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
Re: Implement generalized sub routine find_in_log for tap test
On Sun, Jun 4, 2023 at 3:51 AM Michael Paquier <michael@paquier.xyz> wrote:
On Mon, May 29, 2023 at 07:49:52AM +0530, vignesh C wrote:
Thanks for the comment, the attached v3 version patch has the changes
for the same.-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,)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.
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.
Re: Implement generalized sub routine find_in_log for tap test
On Tue, Jun 06, 2023 at 08:05:49AM +0530, Amit Kapila wrote:
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.
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
Re: Implement generalized sub routine find_in_log for tap test
On Mon, Jun 5, 2023 at 9:39 PM vignesh C <vignesh21@gmail.com> wrote:
On Sun, 4 Jun 2023 at 03:51, Michael Paquier <michael@paquier.xyz> wrote:
This looks like a typo to me, the log offset is eaten.
Except of that, I am on board with log_contains().
Thanks for fixing this.
+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
/messages/by-id/CALj2ACVUcXtLgHRPbx28ZQQyRM6j+eSH3jNUALr2pJ4+f=HRGA@mail.gmail.com.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Re: Implement generalized sub routine find_in_log for tap test
On Tue, Jun 06, 2023 at 10:00:00AM +0530, Bharath Rupireddy wrote:
+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?
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
Re: Implement generalized sub routine find_in_log for tap test
On Tue, Jun 6, 2023 at 4:11 PM Michael Paquier <michael@paquier.xyz> wrote:
On Tue, Jun 06, 2023 at 10:00:00AM +0530, Bharath Rupireddy wrote:
+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?As in a small wrapper for pg_switch_wal() that generates N segments at
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 (?).
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
v1-0001-Add-a-function-in-Cluster.pm-to-generate-WALapplication/octet-stream; name=v1-0001-Add-a-function-in-Cluster.pm-to-generate-WALDownload
From fdd45d870bff93e70aeadc23189567f168ebad9b Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Tue, 6 Jun 2023 11:34:23 +0000
Subject: [PATCH v1] Add a function in Cluster.pm to generate WAL
---
src/test/perl/PostgreSQL/Test/Cluster.pm | 20 ++++++++++++
src/test/recovery/t/001_stream_rep.pl | 6 +---
src/test/recovery/t/019_replslot_limit.pl | 31 +++++--------------
.../t/035_standby_logical_decoding.pl | 7 ++---
4 files changed, 31 insertions(+), 33 deletions(-)
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index baea0fcd1c..296db5e84e 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -3095,6 +3095,26 @@ sub create_logical_slot_on_standby
=pod
+=item $node->advance_wal($n)
+
+Advance WAL of $node by $n segments
+
+=cut
+
+sub advance_wal
+{
+ my ($self, $n) = @_;
+
+ # Advance by $n segments (= (wal_segment_size * $n) bytes).
+ for (my $i = 0; $i < $n; $i++)
+ {
+ $self->safe_psql('postgres',
+ "CREATE TABLE tt (); DROP TABLE tt; SELECT pg_switch_wal();");
+ }
+}
+
+=pod
+
=back
=cut
diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl
index 0c72ba0944..0e256dab8d 100644
--- a/src/test/recovery/t/001_stream_rep.pl
+++ b/src/test/recovery/t/001_stream_rep.pl
@@ -496,11 +496,7 @@ $node_primary->safe_psql('postgres',
my $segment_removed = $node_primary->safe_psql('postgres',
'SELECT pg_walfile_name(pg_current_wal_lsn())');
chomp($segment_removed);
-$node_primary->psql(
- 'postgres', "
- CREATE TABLE tab_phys_slot (a int);
- INSERT INTO tab_phys_slot VALUES (generate_series(1,10));
- SELECT pg_switch_wal();");
+$node_primary->advance_wal(1);
my $current_lsn =
$node_primary->safe_psql('postgres', "SELECT pg_current_wal_lsn();");
chomp($current_lsn);
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index a1aba16e14..d4cc438464 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -59,7 +59,7 @@ $result = $node_primary->safe_psql('postgres',
is($result, "reserved|t", 'check the catching-up state');
# Advance WAL by five segments (= 5MB) on primary
-advance_wal($node_primary, 1);
+$node_primary->advance_wal(1);
$node_primary->safe_psql('postgres', "CHECKPOINT;");
# The slot is always "safe" when fitting max_wal_size
@@ -69,7 +69,7 @@ $result = $node_primary->safe_psql('postgres',
is($result, "reserved|t",
'check that it is safe if WAL fits in max_wal_size');
-advance_wal($node_primary, 4);
+$node_primary->advance_wal(4);
$node_primary->safe_psql('postgres', "CHECKPOINT;");
# The slot is always "safe" when max_slot_wal_keep_size is not set
@@ -100,7 +100,7 @@ $result = $node_primary->safe_psql('postgres',
is($result, "reserved", 'check that max_slot_wal_keep_size is working');
# Advance WAL again then checkpoint, reducing remain by 2 MB.
-advance_wal($node_primary, 2);
+$node_primary->advance_wal(2);
$node_primary->safe_psql('postgres', "CHECKPOINT;");
# The slot is still working
@@ -118,7 +118,7 @@ $node_standby->stop;
$result = $node_primary->safe_psql('postgres',
"ALTER SYSTEM SET wal_keep_size to '8MB'; SELECT pg_reload_conf();");
# Advance WAL again then checkpoint, reducing remain by 6 MB.
-advance_wal($node_primary, 6);
+$node_primary->advance_wal(6);
$result = $node_primary->safe_psql('postgres',
"SELECT wal_status as remain FROM pg_replication_slots WHERE slot_name = 'rep1'"
);
@@ -134,7 +134,7 @@ $node_primary->wait_for_catchup($node_standby);
$node_standby->stop;
# Advance WAL again without checkpoint, reducing remain by 6 MB.
-advance_wal($node_primary, 6);
+$node_primary->advance_wal(6);
# Slot gets into 'reserved' state
$result = $node_primary->safe_psql('postgres',
@@ -145,7 +145,7 @@ is($result, "extended", 'check that the slot state changes to "extended"');
$node_primary->safe_psql('postgres', "CHECKPOINT;");
# Advance WAL again without checkpoint; remain goes to 0.
-advance_wal($node_primary, 1);
+$node_primary->advance_wal(1);
# Slot gets into 'unreserved' state and safe_wal_size is negative
$result = $node_primary->safe_psql('postgres',
@@ -175,7 +175,7 @@ $node_primary->safe_psql('postgres',
# Advance WAL again. The slot loses the oldest segment by the next checkpoint
my $logstart = get_log_size($node_primary);
-advance_wal($node_primary, 7);
+$node_primary->advance_wal(7);
# Now create another checkpoint and wait until the WARNING is issued
$node_primary->safe_psql('postgres',
@@ -375,7 +375,7 @@ $logstart = get_log_size($node_primary3);
# freeze walsender and walreceiver. Slot will still be active, but walreceiver
# won't get anything anymore.
kill 'STOP', $senderpid, $receiverpid;
-advance_wal($node_primary3, 2);
+$node_primary3->advance_wal(2);
my $msg_logged = 0;
my $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
@@ -423,21 +423,6 @@ kill 'CONT', $receiverpid;
$node_primary3->stop;
$node_standby3->stop;
-#####################################
-# Advance WAL of $node by $n segments
-sub advance_wal
-{
- my ($node, $n) = @_;
-
- # Advance by $n segments (= (wal_segment_size * $n) bytes) on primary.
- for (my $i = 0; $i < $n; $i++)
- {
- $node->safe_psql('postgres',
- "CREATE TABLE t (); DROP TABLE t; SELECT pg_switch_wal();");
- }
- return;
-}
-
# return the size of logfile of $node in bytes
sub get_log_size
{
diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl b/src/test/recovery/t/035_standby_logical_decoding.pl
index 64beec4bd3..1fcf3bc248 100644
--- a/src/test/recovery/t/035_standby_logical_decoding.pl
+++ b/src/test/recovery/t/035_standby_logical_decoding.pl
@@ -540,11 +540,8 @@ my $walfile_name = $node_primary->safe_psql('postgres',
chomp($walfile_name);
# Generate some activity and switch WAL file on the primary
-$node_primary->safe_psql(
- 'postgres', "create table retain_test(a int);
- select pg_switch_wal();
- insert into retain_test values(1);
- checkpoint;");
+$node_primary->advance_wal(1);
+$node_primary->safe_psql('postgres', "checkpoint;");
# Wait for the standby to catch up
$node_primary->wait_for_replay_catchup($node_standby);
--
2.34.1
Re: Implement generalized sub routine find_in_log for tap test
On Tue, 6 Jun 2023 at 09:36, Michael Paquier <michael@paquier.xyz> wrote:
On Tue, Jun 06, 2023 at 08:05:49AM +0530, Amit Kapila wrote:
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.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.
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
Attachments:
v5-0001-Remove-duplicate-find_in_log-sub-routines-from-ta_PG13.patchtext/x-patch; charset=US-ASCII; name=v5-0001-Remove-duplicate-find_in_log-sub-routines-from-ta_PG13.patchDownload
From d582676214f3fd5068c343f42116fa256be18958 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
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,
+ok( !$node_standby->log_contains(
"requested WAL segment [0-9A-F]+ has already been removed"),
'check that required WAL segments are still available');
@@ -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();
-
-# 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/;
-}
--
2.34.1
v5-0001-Have-find_in_log-sub-routine-as-a-common-routine_PG12.patchtext/x-patch; charset=US-ASCII; name=v5-0001-Have-find_in_log-sub-routine-as-a-common-routine_PG12.patchDownload
From 7570e55a9f0121a9e2e0a12f4e8718cc531811c9 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
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();
-
-# 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/;
-}
--
2.34.1
v5-0001-Remove-duplicate-find_in_log-sub-routines-from-ta_PG14.patchtext/x-patch; charset=US-ASCII; name=v5-0001-Remove-duplicate-find_in_log-sub-routines-from-ta_PG14.patchDownload
From 94a9c3e62df5c4044271bada78af8fa5869f0a26 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
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,
+ok( !$node_standby->log_contains(
"requested WAL segment [0-9A-F]+ has already been removed"),
'check that required WAL segments are still available');
@@ -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();
-
-# 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/;
-}
--
2.34.1
v5-0001-Remove-duplicate-find_in_log-sub-routines-from-ta_master.patchtext/x-patch; charset=US-ASCII; name=v5-0001-Remove-duplicate-find_in_log-sub-routines-from-ta_master.patchDownload
From e831e26c06994c656b61a99dee86c4710d5e37a1 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
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(-)
diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index 3272e52cae..d8e4976072 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..af97ed9f70 100644
--- a/src/test/recovery/t/033_replay_tsp_drops.pl
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -135,8 +135,7 @@ 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);
@@ -144,13 +143,3 @@ while ($max_attempts-- >= 0)
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/;
-}
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
v5-0001-Remove-duplicate-find_in_log-sub-routines-from-ta_PG15.patchtext/x-patch; charset=US-ASCII; name=v5-0001-Remove-duplicate-find_in_log-sub-routines-from-ta_PG15.patchDownload
From a91e8b12840dfe9aaea425f1cf0255d5849cbcb0 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
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
+=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(...)
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,
+ok( !$node_standby->log_contains(
"requested WAL segment [0-9A-F]+ has already been removed"),
'check that required WAL segments are still available');
@@ -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();
-
-# 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/;
-}
--
2.34.1
v5-0002-Move-common-connection-log-content-verification-c_master.patchtext/x-patch; charset=US-ASCII; name=v5-0002-Move-common-connection-log-content-verification-c_master.patchDownload
From f96a537129a740fe8998e7201ac052d3e63d4129 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
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.
=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()>.
+See C<log_check(...)>.
=back
@@ -2215,16 +2209,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 +2229,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->log_check($test_name, $log_location, %params);
}
=pod
@@ -2278,7 +2249,7 @@ If this regular expression is set, matches it with the output generated.
=item log_unlike => [ qr/prohibited message/ ]
-See C<connect_ok(...)>, above.
+See C<log_check(...)>.
=back
@@ -2289,16 +2260,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 +2277,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->log_check($test_name, $log_location, %params);
}
=pod
@@ -2535,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
--
2.34.1
v5-0002-Move-common-connection-log-content-verification-c_PG14.patchtext/x-patch; charset=US-ASCII; name=v5-0002-Move-common-connection-log-content-verification-c_PG14.patchDownload
From 16cabb908af170b90cf971f7e47a69712aa2720e Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
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.
=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()>.
+See C<log_check(...)>.
=back
@@ -2177,16 +2171,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
@@ -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/ ]
-See C<connect_ok(...)>, above.
+See C<log_check(...)>.
=back
@@ -2248,14 +2220,6 @@ sub connect_fails
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;
@@ -2274,19 +2238,7 @@ sub connect_fails
like($stderr, $params{expected_stderr}, "$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
@@ -2459,6 +2411,67 @@ sub issues_sql_like
=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 = TestLib::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->log_contains(pattern, offset)
Find pattern in logfile of node after offset byte.
--
2.34.1
v5-0002-Move-common-connection-log-content-verification-c_PG15.patchtext/x-patch; charset=US-ASCII; name=v5-0002-Move-common-connection-log-content-verification-c_PG15.patchDownload
From 27d621e922717b01171d12a28d96d699596b992a Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
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.
=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()>.
+See C<log_check(...)>.
=back
@@ -2251,16 +2245,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
@@ -2281,20 +2265,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->log_check($test_name, $log_location, %params);
}
=pod
@@ -2314,7 +2285,7 @@ If this regular expression is set, matches it with the output generated.
=item log_unlike => [ qr/prohibited message/ ]
-See C<connect_ok(...)>, above.
+See C<log_check(...)>.
=back
@@ -2325,16 +2296,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
@@ -2352,20 +2313,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->log_check($test_name, $log_location, %params);
}
=pod
@@ -2559,6 +2507,68 @@ sub issues_sql_like
=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
+
=item log_contains(pattern, offset)
Find pattern in logfile of node after offset byte.
--
2.34.1
Re: Implement generalized sub routine find_in_log for tap test
On Tue, Jun 06, 2023 at 05:53:40PM +0530, Bharath Rupireddy wrote:
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?
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
Re: Implement generalized sub routine find_in_log for tap test
On Tue, Jun 06, 2023 at 06:43:44PM +0530, vignesh C wrote:
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.
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
Re: Implement generalized sub routine find_in_log for tap test
On Fri, Jun 9, 2023 at 8:29 AM Michael Paquier <michael@paquier.xyz> wrote:
On Tue, Jun 06, 2023 at 05:53:40PM +0530, Bharath Rupireddy wrote:
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?Why not discussing that on a separate thread? What you are proposing
is independent of what Vignesh has proposed.
Sure. Here it is -
/messages/by-id/CALj2ACU3R8QFCvDewHCMKjgb2w_-CMCyd6DAK=Jb-af14da5eg@mail.gmail.com.
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?
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).
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Re: Implement generalized sub routine find_in_log for tap test
On Fri, 9 Jun 2023 at 08:31, Michael Paquier <michael@paquier.xyz> wrote:
On Tue, Jun 06, 2023 at 06:43:44PM +0530, vignesh C wrote:
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.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.
Thanks for pushing this.
Regards,
Vignesh