Replace (stat(<file>))[7] in TAP tests with -s
Hi hackers,
Please find attached a tiny patch to $SUBJECT.
It:
- provides more consistency to the way we get files size in TAP tests
- seems more elegant that relying on a hardcoded result position
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
v1-0001-Be-consistent-in-TAP-tests-for-the-way-we-get-fil.patchtext/plain; charset=UTF-8; name=v1-0001-Be-consistent-in-TAP-tests-for-the-way-we-get-fil.patchDownload
From a227eb957ca32dcb1b53a7786992732b1d02c130 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Mon, 2 Oct 2023 09:01:20 +0000
Subject: [PATCH v1] Be consistent in TAP tests for the way we get files size
Using -s is mainly used to get files size but there is some places making use
of "stat" and its 7th result position.
Replacing those by -s instead as it's more elegant than relying on this hardcoded
7th position.
---
src/bin/pg_controldata/t/001_pg_controldata.pl | 2 +-
src/bin/pg_resetwal/t/002_corrupted.pl | 2 +-
src/test/recovery/t/019_replslot_limit.pl | 14 +++-----------
3 files changed, 5 insertions(+), 13 deletions(-)
12.5% src/bin/pg_controldata/t/
12.5% src/bin/pg_resetwal/t/
74.8% src/test/recovery/t/
diff --git a/src/bin/pg_controldata/t/001_pg_controldata.pl b/src/bin/pg_controldata/t/001_pg_controldata.pl
index 4918ea8944..9db8015d0b 100644
--- a/src/bin/pg_controldata/t/001_pg_controldata.pl
+++ b/src/bin/pg_controldata/t/001_pg_controldata.pl
@@ -24,7 +24,7 @@ command_like([ 'pg_controldata', $node->data_dir ],
# check with a corrupted pg_control
my $pg_control = $node->data_dir . '/global/pg_control';
-my $size = (stat($pg_control))[7];
+my $size = -s $pg_control;
open my $fh, '>', $pg_control or BAIL_OUT($!);
binmode $fh;
diff --git a/src/bin/pg_resetwal/t/002_corrupted.pl b/src/bin/pg_resetwal/t/002_corrupted.pl
index 6d19a1efd5..b3a37728a4 100644
--- a/src/bin/pg_resetwal/t/002_corrupted.pl
+++ b/src/bin/pg_resetwal/t/002_corrupted.pl
@@ -14,7 +14,7 @@ my $node = PostgreSQL::Test::Cluster->new('main');
$node->init;
my $pg_control = $node->data_dir . '/global/pg_control';
-my $size = (stat($pg_control))[7];
+my $size = -s $pg_control;
# Read out the head of the file to get PG_CONTROL_VERSION in
# particular.
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index 33e50ad933..7d94f15778 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -173,7 +173,7 @@ $node_primary->safe_psql('postgres',
"ALTER SYSTEM SET max_wal_size='40MB'; SELECT pg_reload_conf()");
# Advance WAL again. The slot loses the oldest segment by the next checkpoint
-my $logstart = get_log_size($node_primary);
+my $logstart = -s $node_primary->logfile;
advance_wal($node_primary, 7);
# Now create another checkpoint and wait until the WARNING is issued
@@ -229,7 +229,7 @@ $node_primary->safe_psql('postgres',
is($oldestseg, $redoseg, "check that segments have been removed");
# The standby no longer can connect to the primary
-$logstart = get_log_size($node_standby);
+$logstart = -s $node_standby->logfile;
$node_standby->start;
my $failed = 0;
@@ -368,7 +368,7 @@ my $receiverpid = $node_standby3->safe_psql('postgres',
"SELECT pid FROM pg_stat_activity WHERE backend_type = 'walreceiver'");
like($receiverpid, qr/^[0-9]+$/, "have walreceiver pid $receiverpid");
-$logstart = get_log_size($node_primary3);
+$logstart = -s $node_primary3->logfile;
# freeze walsender and walreceiver. Slot will still be active, but walreceiver
# won't get anything anymore.
kill 'STOP', $senderpid, $receiverpid;
@@ -433,12 +433,4 @@ sub advance_wal
return;
}
-# return the size of logfile of $node in bytes
-sub get_log_size
-{
- my ($node) = @_;
-
- return (stat $node->logfile)[7];
-}
-
done_testing();
--
2.34.1
"Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com> writes:
Hi hackers,
Please find attached a tiny patch to $SUBJECT.
It:
- provides more consistency to the way we get files size in TAP tests
- seems more elegant that relying on a hardcoded result position
I approve of removing use of the list form of stat, it's a horrible API.
If we weren't already using -s everywhere else, I would prefer
File::stat, which makes stat (in scalar context) return an object with
methods for the fields, so you'd do stat($file)->size. It's included in
Perl core since 5.4, and we're already using it in several places for
other fields (mode and ino at least).
I see another use of stat array positions (for mtime) in
src/tools/msvc/Solution.pm, but that's on the chopping block, so not
much point in fixing.
- ilmari
On Mon, Oct 02, 2023 at 12:44:59PM +0100, Dagfinn Ilmari Mannsåker wrote:
I approve of removing use of the list form of stat, it's a horrible API.
Agreed, I've appied the suggestion to use -s, like we do anywhere
else.
If we weren't already using -s everywhere else, I would prefer
File::stat, which makes stat (in scalar context) return an object with
methods for the fields, so you'd do stat($file)->size. It's included in
Perl core since 5.4, and we're already using it in several places for
other fields (mode and ino at least).
Right, like in 017_shm.pl. I didn't notice that.
I see another use of stat array positions (for mtime) in
src/tools/msvc/Solution.pm, but that's on the chopping block, so not
much point in fixing.
The removal of this code depends on a few more things, hopefully it
will be able to get rid of it during this release cycle.
--
Michael