Small TAP tests cleanup for Windows and unused modules
Seeing msys in TAP tests mentioned in a thread [1]/messages/by-id/0ba775a2-8aa0-0d56-d780-69427cf6f33d@dunslane.net tonight reminded me about
two related (well, one of them) patches I had sitting around, so rather than
forgetting again here are some small cleanups.
0001 attempts to streamline how we detect Windows in the TAP tests (after that
there is a single msys check left that I'm not sure about, but [1]/messages/by-id/0ba775a2-8aa0-0d56-d780-69427cf6f33d@dunslane.net seems to
imply it could go); 0002 removes some unused module includes which either were
used at some point in the past or likely came from copy/paste.
--
Daniel Gustafsson https://vmware.com/
[1]: /messages/by-id/0ba775a2-8aa0-0d56-d780-69427cf6f33d@dunslane.net
Attachments:
0002-Remove-unused-modules-from-TAP-tests.patchapplication/octet-stream; name=0002-Remove-unused-modules-from-TAP-tests.patch; x-unix-mode=0644Download
From ea778e185c4fde46a2c2fd31a90f805dc2b8afd2 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Tue, 15 Feb 2022 10:41:22 +0100
Subject: [PATCH 2/2] Remove unused modules from TAP tests
This avoids pulling in Config and Cwd were they are no longer used.
---
contrib/amcheck/t/002_cic.pl | 1 -
contrib/amcheck/t/003_cic_2pc.pl | 1 -
src/bin/pg_basebackup/t/010_pg_basebackup.pl | 2 --
src/bin/pg_checksums/t/002_actions.pl | 1 -
src/bin/pg_dump/t/001_basic.pl | 1 -
src/bin/pg_dump/t/002_pg_dump.pl | 1 -
src/bin/pg_test_fsync/t/001_basic.pl | 1 -
src/bin/pg_test_timing/t/001_basic.pl | 1 -
src/bin/pg_verifybackup/t/002_algorithm.pl | 2 --
src/bin/pg_verifybackup/t/003_corruption.pl | 2 --
src/bin/pg_verifybackup/t/004_options.pl | 2 --
src/bin/pg_verifybackup/t/005_bad_manifest.pl | 2 --
src/bin/pg_verifybackup/t/006_encoding.pl | 2 --
src/bin/pg_verifybackup/t/007_wal.pl | 2 --
src/bin/pg_verifybackup/t/008_untar.pl | 1 -
src/bin/pg_verifybackup/t/009_extract.pl | 2 --
src/bin/pg_verifybackup/t/010_client_untar.pl | 1 -
src/bin/pgbench/t/001_pgbench_with_server.pl | 1 -
src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl | 1 -
src/test/modules/test_pg_dump/t/001_base.pl | 1 -
src/test/perl/PostgreSQL/Test/Cluster.pm | 1 -
src/test/recovery/t/006_logical_decoding.pl | 1 -
src/test/recovery/t/011_crash_recovery.pl | 1 -
src/test/recovery/t/013_crash_restart.pl | 1 -
src/test/recovery/t/017_shm.pl | 1 -
src/test/recovery/t/020_archive_status.pl | 1 -
src/test/recovery/t/021_row_visibility.pl | 1 -
src/test/recovery/t/022_crash_temp_files.pl | 1 -
28 files changed, 36 deletions(-)
diff --git a/contrib/amcheck/t/002_cic.pl b/contrib/amcheck/t/002_cic.pl
index d604def0d0..c84482b61e 100644
--- a/contrib/amcheck/t/002_cic.pl
+++ b/contrib/amcheck/t/002_cic.pl
@@ -5,7 +5,6 @@
use strict;
use warnings;
-use Config;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
diff --git a/contrib/amcheck/t/003_cic_2pc.pl b/contrib/amcheck/t/003_cic_2pc.pl
index f668ed3c40..33df7d1be7 100644
--- a/contrib/amcheck/t/003_cic_2pc.pl
+++ b/contrib/amcheck/t/003_cic_2pc.pl
@@ -5,7 +5,6 @@
use strict;
use warnings;
-use Config;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 8c70e5b32b..a2d08a777d 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -3,8 +3,6 @@
use strict;
use warnings;
-use Cwd;
-use Config;
use File::Basename qw(basename dirname);
use File::Path qw(rmtree);
use Fcntl qw(:seek);
diff --git a/src/bin/pg_checksums/t/002_actions.pl b/src/bin/pg_checksums/t/002_actions.pl
index 5563244f11..bba1f3494c 100644
--- a/src/bin/pg_checksums/t/002_actions.pl
+++ b/src/bin/pg_checksums/t/002_actions.pl
@@ -6,7 +6,6 @@
use strict;
use warnings;
-use Config;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index 48faeb4371..e0bf3eb032 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -4,7 +4,6 @@
use strict;
use warnings;
-use Config;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index dd065c758f..e23e8afa22 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -4,7 +4,6 @@
use strict;
use warnings;
-use Config;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
diff --git a/src/bin/pg_test_fsync/t/001_basic.pl b/src/bin/pg_test_fsync/t/001_basic.pl
index 0b579dde9d..8c5c0e7967 100644
--- a/src/bin/pg_test_fsync/t/001_basic.pl
+++ b/src/bin/pg_test_fsync/t/001_basic.pl
@@ -4,7 +4,6 @@
use strict;
use warnings;
-use Config;
use PostgreSQL::Test::Utils;
use Test::More;
diff --git a/src/bin/pg_test_timing/t/001_basic.pl b/src/bin/pg_test_timing/t/001_basic.pl
index ef9f6d6746..51bf68b0e1 100644
--- a/src/bin/pg_test_timing/t/001_basic.pl
+++ b/src/bin/pg_test_timing/t/001_basic.pl
@@ -4,7 +4,6 @@
use strict;
use warnings;
-use Config;
use PostgreSQL::Test::Utils;
use Test::More;
diff --git a/src/bin/pg_verifybackup/t/002_algorithm.pl b/src/bin/pg_verifybackup/t/002_algorithm.pl
index 20e3ca587a..ca2c0f0c7b 100644
--- a/src/bin/pg_verifybackup/t/002_algorithm.pl
+++ b/src/bin/pg_verifybackup/t/002_algorithm.pl
@@ -5,8 +5,6 @@
use strict;
use warnings;
-use Cwd;
-use Config;
use File::Path qw(rmtree);
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
diff --git a/src/bin/pg_verifybackup/t/003_corruption.pl b/src/bin/pg_verifybackup/t/003_corruption.pl
index f402d301ac..7778727eca 100644
--- a/src/bin/pg_verifybackup/t/003_corruption.pl
+++ b/src/bin/pg_verifybackup/t/003_corruption.pl
@@ -5,8 +5,6 @@
use strict;
use warnings;
-use Cwd;
-use Config;
use File::Path qw(rmtree);
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
diff --git a/src/bin/pg_verifybackup/t/004_options.pl b/src/bin/pg_verifybackup/t/004_options.pl
index 7461dee03f..6fdd74e5ee 100644
--- a/src/bin/pg_verifybackup/t/004_options.pl
+++ b/src/bin/pg_verifybackup/t/004_options.pl
@@ -5,8 +5,6 @@
use strict;
use warnings;
-use Cwd;
-use Config;
use File::Path qw(rmtree);
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
diff --git a/src/bin/pg_verifybackup/t/005_bad_manifest.pl b/src/bin/pg_verifybackup/t/005_bad_manifest.pl
index 118beb53d7..74d0a8d1b2 100644
--- a/src/bin/pg_verifybackup/t/005_bad_manifest.pl
+++ b/src/bin/pg_verifybackup/t/005_bad_manifest.pl
@@ -6,8 +6,6 @@
use strict;
use warnings;
-use Cwd;
-use Config;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
diff --git a/src/bin/pg_verifybackup/t/006_encoding.pl b/src/bin/pg_verifybackup/t/006_encoding.pl
index ce45c919a8..210f269d59 100644
--- a/src/bin/pg_verifybackup/t/006_encoding.pl
+++ b/src/bin/pg_verifybackup/t/006_encoding.pl
@@ -5,8 +5,6 @@
use strict;
use warnings;
-use Cwd;
-use Config;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
diff --git a/src/bin/pg_verifybackup/t/007_wal.pl b/src/bin/pg_verifybackup/t/007_wal.pl
index 56fcd84bec..bef2701ef7 100644
--- a/src/bin/pg_verifybackup/t/007_wal.pl
+++ b/src/bin/pg_verifybackup/t/007_wal.pl
@@ -5,8 +5,6 @@
use strict;
use warnings;
-use Cwd;
-use Config;
use File::Path qw(rmtree);
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
diff --git a/src/bin/pg_verifybackup/t/008_untar.pl b/src/bin/pg_verifybackup/t/008_untar.pl
index 6927ca4c74..3fd18143d0 100644
--- a/src/bin/pg_verifybackup/t/008_untar.pl
+++ b/src/bin/pg_verifybackup/t/008_untar.pl
@@ -7,7 +7,6 @@
use strict;
use warnings;
-use Config;
use File::Path qw(rmtree);
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
diff --git a/src/bin/pg_verifybackup/t/009_extract.pl b/src/bin/pg_verifybackup/t/009_extract.pl
index c51cdf79f8..f5a70ff8c9 100644
--- a/src/bin/pg_verifybackup/t/009_extract.pl
+++ b/src/bin/pg_verifybackup/t/009_extract.pl
@@ -6,8 +6,6 @@
use strict;
use warnings;
-use Cwd;
-use Config;
use File::Path qw(rmtree);
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
diff --git a/src/bin/pg_verifybackup/t/010_client_untar.pl b/src/bin/pg_verifybackup/t/010_client_untar.pl
index 3616529390..9fe5a43783 100644
--- a/src/bin/pg_verifybackup/t/010_client_untar.pl
+++ b/src/bin/pg_verifybackup/t/010_client_untar.pl
@@ -6,7 +6,6 @@
use strict;
use warnings;
-use Config;
use File::Path qw(rmtree);
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl
index 8b03900f32..8092d23736 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -7,7 +7,6 @@ use warnings;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
-use Config;
# start a pgbench specific server
my $node = PostgreSQL::Test::Cluster->new('main');
diff --git a/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl b/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl
index 0c164dcaba..cc79d96d47 100644
--- a/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl
+++ b/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl
@@ -4,7 +4,6 @@
use strict;
use warnings;
-use Config;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
diff --git a/src/test/modules/test_pg_dump/t/001_base.pl b/src/test/modules/test_pg_dump/t/001_base.pl
index c73bd37835..84a35590b7 100644
--- a/src/test/modules/test_pg_dump/t/001_base.pl
+++ b/src/test/modules/test_pg_dump/t/001_base.pl
@@ -4,7 +4,6 @@
use strict;
use warnings;
-use Config;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 41beadff9d..52c18f8c69 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -92,7 +92,6 @@ use warnings;
use Carp;
use Config;
-use Cwd;
use Fcntl qw(:mode);
use File::Basename;
use File::Path qw(rmtree);
diff --git a/src/test/recovery/t/006_logical_decoding.pl b/src/test/recovery/t/006_logical_decoding.pl
index a4d51eb995..77c85ca4b6 100644
--- a/src/test/recovery/t/006_logical_decoding.pl
+++ b/src/test/recovery/t/006_logical_decoding.pl
@@ -11,7 +11,6 @@ use warnings;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
-use Config;
# Initialize primary node
my $node_primary = PostgreSQL::Test::Cluster->new('primary');
diff --git a/src/test/recovery/t/011_crash_recovery.pl b/src/test/recovery/t/011_crash_recovery.pl
index 14154d1ce0..1b57d01046 100644
--- a/src/test/recovery/t/011_crash_recovery.pl
+++ b/src/test/recovery/t/011_crash_recovery.pl
@@ -9,7 +9,6 @@ use warnings;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
-use Config;
my $node = PostgreSQL::Test::Cluster->new('primary');
$node->init(allows_streaming => 1);
diff --git a/src/test/recovery/t/013_crash_restart.pl b/src/test/recovery/t/013_crash_restart.pl
index 3b740eb6f3..36e6230fac 100644
--- a/src/test/recovery/t/013_crash_restart.pl
+++ b/src/test/recovery/t/013_crash_restart.pl
@@ -16,7 +16,6 @@ use warnings;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
-use Config;
# To avoid hanging while expecting some specific input from a psql
# instance being driven by us, add a timeout high enough that it
diff --git a/src/test/recovery/t/017_shm.pl b/src/test/recovery/t/017_shm.pl
index 678a252165..9d26bd270e 100644
--- a/src/test/recovery/t/017_shm.pl
+++ b/src/test/recovery/t/017_shm.pl
@@ -6,7 +6,6 @@
#
use strict;
use warnings;
-use Config;
use File::stat qw(stat);
use IPC::Run 'run';
use PostgreSQL::Test::Cluster;
diff --git a/src/test/recovery/t/020_archive_status.pl b/src/test/recovery/t/020_archive_status.pl
index cbff26e122..e6e4eb56a9 100644
--- a/src/test/recovery/t/020_archive_status.pl
+++ b/src/test/recovery/t/020_archive_status.pl
@@ -9,7 +9,6 @@ use warnings;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
-use Config;
my $primary = PostgreSQL::Test::Cluster->new('primary');
$primary->init(
diff --git a/src/test/recovery/t/021_row_visibility.pl b/src/test/recovery/t/021_row_visibility.pl
index e01f3f57c7..b63e32d8cb 100644
--- a/src/test/recovery/t/021_row_visibility.pl
+++ b/src/test/recovery/t/021_row_visibility.pl
@@ -9,7 +9,6 @@ use warnings;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
-use Config;
# Initialize primary node
my $node_primary = PostgreSQL::Test::Cluster->new('primary');
diff --git a/src/test/recovery/t/022_crash_temp_files.pl b/src/test/recovery/t/022_crash_temp_files.pl
index c37723a144..c98475c4c7 100644
--- a/src/test/recovery/t/022_crash_temp_files.pl
+++ b/src/test/recovery/t/022_crash_temp_files.pl
@@ -7,7 +7,6 @@ use warnings;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
-use Config;
if ($windows_os)
{
--
2.24.3 (Apple Git-128)
0001-Make-Windows-detection-in-TAP-tests-consistent.patchapplication/octet-stream; name=0001-Make-Windows-detection-in-TAP-tests-consistent.patch; x-unix-mode=0644Download
From 3ccaafa454140436b50e2ab2f414bf5094f745db Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Mon, 14 Feb 2022 11:05:14 +0100
Subject: [PATCH 1/2] Make Windows detection in TAP tests consistent
Use the exported variable $windows_os from the test harness to ensure
that Windows detection is done consistently across the tests.
---
src/bin/pg_rewind/t/RewindTest.pm | 3 +--
src/test/recovery/t/006_logical_decoding.pl | 2 +-
src/test/recovery/t/021_row_visibility.pl | 2 +-
src/test/recovery/t/022_crash_temp_files.pl | 2 +-
4 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/bin/pg_rewind/t/RewindTest.pm b/src/bin/pg_rewind/t/RewindTest.pm
index 60e3234788..9c33d12654 100644
--- a/src/bin/pg_rewind/t/RewindTest.pm
+++ b/src/bin/pg_rewind/t/RewindTest.pm
@@ -35,7 +35,6 @@ use strict;
use warnings;
use Carp;
-use Config;
use Exporter 'import';
use File::Copy;
use File::Path qw(rmtree);
@@ -115,7 +114,7 @@ sub check_query
}
else
{
- $stdout =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
+ $stdout =~ s/\r\n/\n/g if $windows_os;
is($stdout, $expected_stdout, "$test_name: query result matches");
}
return;
diff --git a/src/test/recovery/t/006_logical_decoding.pl b/src/test/recovery/t/006_logical_decoding.pl
index fa6bd45332..a4d51eb995 100644
--- a/src/test/recovery/t/006_logical_decoding.pl
+++ b/src/test/recovery/t/006_logical_decoding.pl
@@ -143,7 +143,7 @@ SKIP:
{
# some Windows Perls at least don't like IPC::Run's start/kill_kill regime.
- skip "Test fails on Windows perl", 2 if $Config{osname} eq 'MSWin32';
+ skip "Test fails on Windows perl", 2 if $windows_os;
my $pg_recvlogical = IPC::Run::start(
[
diff --git a/src/test/recovery/t/021_row_visibility.pl b/src/test/recovery/t/021_row_visibility.pl
index e2743518de..e01f3f57c7 100644
--- a/src/test/recovery/t/021_row_visibility.pl
+++ b/src/test/recovery/t/021_row_visibility.pl
@@ -183,7 +183,7 @@ sub send_query_and_wait
while (1)
{
# See PostgreSQL::Test::Cluster.pm's psql()
- $$psql{stdout} =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
+ $$psql{stdout} =~ s/\r\n/\n/g if $windows_os;
last if $$psql{stdout} =~ /$untl/;
diff --git a/src/test/recovery/t/022_crash_temp_files.pl b/src/test/recovery/t/022_crash_temp_files.pl
index 6ab3092874..c37723a144 100644
--- a/src/test/recovery/t/022_crash_temp_files.pl
+++ b/src/test/recovery/t/022_crash_temp_files.pl
@@ -9,7 +9,7 @@ use PostgreSQL::Test::Utils;
use Test::More;
use Config;
-if ($Config{osname} eq 'MSWin32')
+if ($windows_os)
{
plan skip_all => 'tests hang on Windows';
exit;
--
2.24.3 (Apple Git-128)
On 2/16/22 16:36, Daniel Gustafsson wrote:
Seeing msys in TAP tests mentioned in a thread [1] tonight reminded me about
two related (well, one of them) patches I had sitting around, so rather than
forgetting again here are some small cleanups.0001 attempts to streamline how we detect Windows in the TAP tests (after that
there is a single msys check left that I'm not sure about, but [1] seems to
imply it could go); 0002 removes some unused module includes which either were
used at some point in the past or likely came from copy/paste.
0002 looks OK at first glance.
0001 is something we should investigate. It's really going in the wrong
direction I suspect. We should be looking to narrow the scope of these
platform-specific bits of processing, not expand them.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
On 16 Feb 2022, at 23:04, Andrew Dunstan <andrew@dunslane.net> wrote:
On 2/16/22 16:36, Daniel Gustafsson wrote:
Seeing msys in TAP tests mentioned in a thread [1] tonight reminded me about
two related (well, one of them) patches I had sitting around, so rather than
forgetting again here are some small cleanups.0001 attempts to streamline how we detect Windows in the TAP tests (after that
there is a single msys check left that I'm not sure about, but [1] seems to
imply it could go); 0002 removes some unused module includes which either were
used at some point in the past or likely came from copy/paste.0002 looks OK at first glance.
Thanks!
0001 is something we should investigate. It's really going in the wrong
direction I suspect. We should be looking to narrow the scope of these
platform-specific bits of processing, not expand them.
No arguments there, I was only looking at making sure that we detect the OS in
the same way everywhere, but if you're working towards eliminating this there
is little use in changing anything.
--
Daniel Gustafsson https://vmware.com/
On 2/16/22 17:04, Andrew Dunstan wrote:
On 2/16/22 16:36, Daniel Gustafsson wrote:
Seeing msys in TAP tests mentioned in a thread [1] tonight reminded me about
two related (well, one of them) patches I had sitting around, so rather than
forgetting again here are some small cleanups.0001 attempts to streamline how we detect Windows in the TAP tests (after that
there is a single msys check left that I'm not sure about, but [1] seems to
imply it could go); 0002 removes some unused module includes which either were
used at some point in the past or likely came from copy/paste.0002 looks OK at first glance.
0001 is something we should investigate. It's really going in the wrong
direction I suspect. We should be looking to narrow the scope of these
platform-specific bits of processing, not expand them.
More specifically, all the tests in question are now passing on jacana
and fairywren, and their $Config{osname} is NOT 'msys' (it's 'MSWin32').
So we should simply remove any line that ends "if $Config{osname} eq
'msys';" since we don't have any such animals any more.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Hi,
On February 16, 2022 2:04:01 PM PST, Andrew Dunstan <andrew@dunslane.net> wrote:
On 2/16/22 16:36, Daniel Gustafsson wrote:
Seeing msys in TAP tests mentioned in a thread [1] tonight reminded me about
two related (well, one of them) patches I had sitting around, so rather than
forgetting again here are some small cleanups.0001 attempts to streamline how we detect Windows in the TAP tests (after that
there is a single msys check left that I'm not sure about, but [1] seems to
imply it could go); 0002 removes some unused module includes which either were
used at some point in the past or likely came from copy/paste.0002 looks OK at first glance.
0001 is something we should investigate. It's really going in the wrong
direction I suspect. We should be looking to narrow the scope of these
platform-specific bits of processing, not expand them.
Yes. The vast majority of those skips looks frankly irresponsible, indicating bugs in the test or postgres. There's definitely valid cases (e.g. testing sysv shm), but most don't look valid to me.
Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
On Wed, Feb 16, 2022 at 05:36:14PM -0500, Andrew Dunstan wrote:
More specifically, all the tests in question are now passing on jacana
and fairywren, and their $Config{osname} is NOT 'msys' (it's 'MSWin32').
Indeed. 0001 is incorrect.
So we should simply remove any line that ends "if $Config{osname} eq
'msys';" since we don't have any such animals any more.
So, that's related to [1]/messages/by-id/0ba775a2-8aa0-0d56-d780-69427cf6f33d@dunslane.net -- Michael, meaning that we don't have any buildfarm
members that run the perl flavor from Msys. And, as a result, we
don't need to worry about the CRLF conversions in any of the perl
modules? That would be nice.
[1]: /messages/by-id/0ba775a2-8aa0-0d56-d780-69427cf6f33d@dunslane.net -- Michael
--
Michael
On 16 Feb 2022, at 23:36, Andrew Dunstan <andrew@dunslane.net> wrote:
On 2/16/22 17:04, Andrew Dunstan wrote:On 2/16/22 16:36, Daniel Gustafsson wrote:
Seeing msys in TAP tests mentioned in a thread [1] tonight reminded me about
two related (well, one of them) patches I had sitting around, so rather than
forgetting again here are some small cleanups.0001 attempts to streamline how we detect Windows in the TAP tests (after that
there is a single msys check left that I'm not sure about, but [1] seems to
imply it could go); 0002 removes some unused module includes which either were
used at some point in the past or likely came from copy/paste.0002 looks OK at first glance.
0001 is something we should investigate. It's really going in the wrong
direction I suspect. We should be looking to narrow the scope of these
platform-specific bits of processing, not expand them.More specifically, all the tests in question are now passing on jacana
and fairywren, and their $Config{osname} is NOT 'msys' (it's 'MSWin32').
+1
So we should simply remove any line that ends "if $Config{osname} eq
'msys';" since we don't have any such animals any more.
Since msys is discussed in the other thread let's drop the 0001 from this
thread and just go ahead with 0002.
--
Daniel Gustafsson https://vmware.com/
On 18 Feb 2022, at 22:02, Daniel Gustafsson <daniel@yesql.se> wrote:
.. let's drop the 0001 from this thread and just go ahead with 0002.
I applied the 0002 patch today, cleaning up the unused module imports.
--
Daniel Gustafsson https://vmware.com/
Daniel Gustafsson <daniel@yesql.se> writes:
On 18 Feb 2022, at 22:02, Daniel Gustafsson <daniel@yesql.se> wrote:
.. let's drop the 0001 from this thread and just go ahead with 0002.
I applied the 0002 patch today, cleaning up the unused module imports.
A quick git grep¹ revealed a few more extraneous `use Config;`
statements (and manual inspection a few more unused modules in one
file). Here's a patch that removes those. It passes tests using
./configure --enable-tap-tests --with-ldap
make check-world PG_TEST_EXTRA=ldap
- ilmari
[1]: git grep -L '[%@$]Config\b' $(git grep -l 'use Config')
Attachments:
0001-Remove-more-unused-module-imports-in-test.patchtext/x-diffDownload
From 681897c17c125a0ef380aa3ef0cba786dba24d0b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Fri, 25 Mar 2022 12:36:34 +0000
Subject: [PATCH] Remove more unused module imports in test
Commit 7dac61402e34c6d41d5d11cdc4c6a55f91e24026 removed a bunch of
superflous `use Config;` and `use Cwd;` statements, but some were left
behind.
---
src/bin/pg_ctl/t/001_start_stop.pl | 3 ---
src/bin/pg_rewind/t/RewindTest.pm | 1 -
src/test/ldap/t/001_auth.pl | 1 -
3 files changed, 5 deletions(-)
diff --git a/src/bin/pg_ctl/t/001_start_stop.pl b/src/bin/pg_ctl/t/001_start_stop.pl
index 3b45390ced..fdffd76d99 100644
--- a/src/bin/pg_ctl/t/001_start_stop.pl
+++ b/src/bin/pg_ctl/t/001_start_stop.pl
@@ -4,9 +4,6 @@
use strict;
use warnings;
-use Config;
-use Fcntl ':mode';
-use File::stat qw{lstat};
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
diff --git a/src/bin/pg_rewind/t/RewindTest.pm b/src/bin/pg_rewind/t/RewindTest.pm
index 5651602858..1e34768e27 100644
--- a/src/bin/pg_rewind/t/RewindTest.pm
+++ b/src/bin/pg_rewind/t/RewindTest.pm
@@ -35,7 +35,6 @@ package RewindTest;
use warnings;
use Carp;
-use Config;
use Exporter 'import';
use File::Copy;
use File::Path qw(rmtree);
diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl
index 9f15248935..b342146e55 100644
--- a/src/test/ldap/t/001_auth.pl
+++ b/src/test/ldap/t/001_auth.pl
@@ -6,7 +6,6 @@
use PostgreSQL::Test::Utils;
use PostgreSQL::Test::Cluster;
use Test::More;
-use Config;
my ($slapd, $ldap_bin_dir, $ldap_schema_dir);
--
2.30.2
On 25 Mar 2022, at 13:42, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:
A quick git grep¹ revealed a few more extraneous `use Config;`
statements (and manual inspection a few more unused modules in one
file).
Thanks for digging these up, they look correct to me.
src/bin/pg_ctl/t/001_start_stop.pl | 3 ---
src/bin/pg_rewind/t/RewindTest.pm | 1 -
These Config references were removed in 1c6d46293. Fcntl ':mode' and
File::stat were added in c37b3d08c which was probably a leftover from an
earlier version of the patch, as the function using these was added to
PostgresNode in that commit.
src/test/ldap/t/001_auth.pl | 1 -
The Config reference here was added in ee56c3b21 which in turn use $^O instead
of interrogating Config, so it can be removed as well.
I'll take it for a tour on the CI and will then apply.
--
Daniel Gustafsson https://vmware.com/
On 27 Mar 2022, at 00:20, Daniel Gustafsson <daniel@yesql.se> wrote:
I'll take it for a tour on the CI and will then apply.
Which is now done, thanks!
--
Daniel Gustafsson https://vmware.com/