003_extrafiles.pl test fails on Windows with the newer Perl versions

Started by Nazir Bilal Yavuzalmost 2 years ago9 messages
#1Nazir Bilal Yavuz
byavuz81@gmail.com
1 attachment(s)

Hi,

I was trying to install newer Perl versions to Windows CI images and
found that 003_extrafiles.pl test fails on Windows with:

(0.183s) not ok 2 - file lists match
(0.000s) # Failed test 'file lists match'
# at C:/cirrus/src/bin/pg_rewind/t/003_extrafiles.pl line 81.
(0.000s) # Structures begin differing at:
# $got->[0] =
'C:/cirrus/build/testrun/pg_rewind/003_extrafiles/data/t_003_extrafiles_primary_local_data/pgdata/tst_both_dir'
# $expected->[0] =
'C:\cirrus\build/testrun/pg_rewind/003_extrafiles\data/t_003_extrafiles_primary_local_data/pgdata/tst_both_dir'

(0.263s) not ok 5 - file lists match
(0.000s) # Failed test 'file lists match'
# at C:/cirrus/src/bin/pg_rewind/t/003_extrafiles.pl line 81.
(0.000s) # Structures begin differing at:
# $got->[0] =
'C:/cirrus/build/testrun/pg_rewind/003_extrafiles/data/t_003_extrafiles_primary_remote_data/pgdata/tst_both_dir'
# $expected->[0] =
'C:\cirrus\build/testrun/pg_rewind/003_extrafiles\data/t_003_extrafiles_primary_remote_data/pgdata/tst_both_dir'

It looks like File::Find converts backslashes to slashes in the newer
Perl versions. I tried to find the related commit and found this:
https://github.com/Perl/perl5/commit/414f14df98cb1c9a20f92c5c54948b67c09f072d

To solve this, I did the same conversion for Windows before comparing
the paths. And to support all Perl versions, I decided to always
convert them on Windows regardless of the Perl's version. The fix is
attached.

I looked at other File::Find appearances in the code but they do not
compare the paths. So, I do not think there is any need to fix them.

Any kind of feedback would be appreciated.

--
Regards,
Nazir Bilal Yavuz
Microsoft

Attachments:

v1-0001-Fix-003_extrafiles.pl-test-for-the-Windows.patchapplication/x-patch; name=v1-0001-Fix-003_extrafiles.pl-test-for-the-Windows.patchDownload
From b28f48fe7d98d3dd7d2fcf652bfa5c8a4cd1c2d6 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: Tue, 30 Jan 2024 13:12:41 +0300
Subject: [PATCH v1] Fix 003_extrafiles.pl test for the Windows

File::Find converts backslashes to slashes in the newer Perl versions.
See: https://github.com/Perl/perl5/commit/414f14df98cb1c9a20f92c5c54948b67c09f072d

So, do the same conversion for Windows before comparing paths. To
support all Perl versions, always convert them on Windows regardless of
the Perl's version.
---
 src/bin/pg_rewind/t/003_extrafiles.pl | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/bin/pg_rewind/t/003_extrafiles.pl b/src/bin/pg_rewind/t/003_extrafiles.pl
index d54dcddcbb6..7e4c2771ce7 100644
--- a/src/bin/pg_rewind/t/003_extrafiles.pl
+++ b/src/bin/pg_rewind/t/003_extrafiles.pl
@@ -78,6 +78,19 @@ sub run_test
 		},
 		$test_primary_datadir);
 	@paths = sort @paths;
+
+	# File::Find converts backslashes to slashes in the newer Perl
+	# versions. To support all Perl versions, do the same conversion
+	# for Windows before comparing the paths.
+	if ($PostgreSQL::Test::Utils::windows_os)
+	{
+		for my $filename (@paths)
+		{
+			$filename =~ s{\\}{/}g;
+		}
+		$test_primary_datadir =~ s{\\}{/}g;
+	}
+
 	is_deeply(
 		\@paths,
 		[
-- 
2.43.0

#2Nazir Bilal Yavuz
byavuz81@gmail.com
In reply to: Nazir Bilal Yavuz (#1)
Re: 003_extrafiles.pl test fails on Windows with the newer Perl versions

Hi,

On Tue, 30 Jan 2024 at 14:21, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:

It looks like File::Find converts backslashes to slashes in the newer
Perl versions. I tried to find the related commit and found this:
https://github.com/Perl/perl5/commit/414f14df98cb1c9a20f92c5c54948b67c09f072d

I forgot to mention that I used Strawberry Perl and these errors
started with 'Perl v5.36.1.1'.

--
Regards,
Nazir Bilal Yavuz
Microsoft

#3Andrew Dunstan
andrew@dunslane.net
In reply to: Nazir Bilal Yavuz (#1)
Re: 003_extrafiles.pl test fails on Windows with the newer Perl versions

On 2024-01-30 Tu 06:21, Nazir Bilal Yavuz wrote:

Hi,

I was trying to install newer Perl versions to Windows CI images and
found that 003_extrafiles.pl test fails on Windows with:

(0.183s) not ok 2 - file lists match
(0.000s) # Failed test 'file lists match'
# at C:/cirrus/src/bin/pg_rewind/t/003_extrafiles.pl line 81.
(0.000s) # Structures begin differing at:
# $got->[0] =
'C:/cirrus/build/testrun/pg_rewind/003_extrafiles/data/t_003_extrafiles_primary_local_data/pgdata/tst_both_dir'
# $expected->[0] =
'C:\cirrus\build/testrun/pg_rewind/003_extrafiles\data/t_003_extrafiles_primary_local_data/pgdata/tst_both_dir'

(0.263s) not ok 5 - file lists match
(0.000s) # Failed test 'file lists match'
# at C:/cirrus/src/bin/pg_rewind/t/003_extrafiles.pl line 81.
(0.000s) # Structures begin differing at:
# $got->[0] =
'C:/cirrus/build/testrun/pg_rewind/003_extrafiles/data/t_003_extrafiles_primary_remote_data/pgdata/tst_both_dir'
# $expected->[0] =
'C:\cirrus\build/testrun/pg_rewind/003_extrafiles\data/t_003_extrafiles_primary_remote_data/pgdata/tst_both_dir'

It looks like File::Find converts backslashes to slashes in the newer
Perl versions. I tried to find the related commit and found this:
https://github.com/Perl/perl5/commit/414f14df98cb1c9a20f92c5c54948b67c09f072d

To solve this, I did the same conversion for Windows before comparing
the paths. And to support all Perl versions, I decided to always
convert them on Windows regardless of the Perl's version. The fix is
attached.

I looked at other File::Find appearances in the code but they do not
compare the paths. So, I do not think there is any need to fix them.

Any kind of feedback would be appreciated.

Looks reasonable on the face of it. I'll see about pushing this today.

cheers

andrew

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

#4Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#3)
Re: 003_extrafiles.pl test fails on Windows with the newer Perl versions

On 2024-01-30 Tu 06:49, Andrew Dunstan wrote:

On 2024-01-30 Tu 06:21, Nazir Bilal Yavuz wrote:

Hi,

I was trying to install newer Perl versions to Windows CI images and
found that 003_extrafiles.pl test fails on Windows with:

(0.183s) not ok 2 - file lists match
(0.000s) #   Failed test 'file lists match'
#   at C:/cirrus/src/bin/pg_rewind/t/003_extrafiles.pl line 81.
(0.000s) #     Structures begin differing at:
#          $got->[0] =
'C:/cirrus/build/testrun/pg_rewind/003_extrafiles/data/t_003_extrafiles_primary_local_data/pgdata/tst_both_dir'

#     $expected->[0] =
'C:\cirrus\build/testrun/pg_rewind/003_extrafiles\data/t_003_extrafiles_primary_local_data/pgdata/tst_both_dir'

(0.263s) not ok 5 - file lists match
(0.000s) #   Failed test 'file lists match'
#   at C:/cirrus/src/bin/pg_rewind/t/003_extrafiles.pl line 81.
(0.000s) #     Structures begin differing at:
#          $got->[0] =
'C:/cirrus/build/testrun/pg_rewind/003_extrafiles/data/t_003_extrafiles_primary_remote_data/pgdata/tst_both_dir'

#     $expected->[0] =
'C:\cirrus\build/testrun/pg_rewind/003_extrafiles\data/t_003_extrafiles_primary_remote_data/pgdata/tst_both_dir'

It looks like File::Find converts backslashes to slashes in the newer
Perl versions. I tried to find the related commit and found this:
https://github.com/Perl/perl5/commit/414f14df98cb1c9a20f92c5c54948b67c09f072d

To solve this, I did the same conversion for Windows before comparing
the paths. And to support all Perl versions, I decided to always
convert them on Windows regardless of the Perl's version. The fix is
attached.

I looked at other File::Find appearances in the code but they do not
compare the paths. So, I do not think there is any need to fix them.

Any kind of feedback would be appreciated.

Looks reasonable on the face of it. I'll see about pushing this today.

Pushed to all live branches. Thanks for the patch.

cheers

andrew

--

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

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#4)
Re: 003_extrafiles.pl test fails on Windows with the newer Perl versions

Andrew Dunstan <andrew@dunslane.net> writes:

Pushed to all live branches. Thanks for the patch.

v12 and v13 branches aren't looking good:

Global symbol "$test_primary_datadir" requires explicit package name (did you forget to declare "my $test_primary_datadir"?) at t/003_extrafiles.pl line 80.
Execution of t/003_extrafiles.pl aborted due to compilation errors.
# Looks like your test exited with 255 before it could output anything.
[17:19:57] t/003_extrafiles.pl ..........
Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 5/5 subtests

regards, tom lane

#6Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#5)
Re: 003_extrafiles.pl test fails on Windows with the newer Perl versions

On 2024-01-30 Tu 18:06, Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

Pushed to all live branches. Thanks for the patch.

v12 and v13 branches aren't looking good:

Global symbol "$test_primary_datadir" requires explicit package name (did you forget to declare "my $test_primary_datadir"?) at t/003_extrafiles.pl line 80.
Execution of t/003_extrafiles.pl aborted due to compilation errors.
# Looks like your test exited with 255 before it could output anything.
[17:19:57] t/003_extrafiles.pl ..........
Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 5/5 subtests

Will fix.

cheers

andrew

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

#7Nazir Bilal Yavuz
byavuz81@gmail.com
In reply to: Andrew Dunstan (#4)
Re: 003_extrafiles.pl test fails on Windows with the newer Perl versions

Hi,

On Wed, 31 Jan 2024 at 01:18, Andrew Dunstan <andrew@dunslane.net> wrote:

Pushed to all live branches. Thanks for the patch.

Thanks for the push!

--
Regards,
Nazir Bilal Yavuz
Microsoft

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nazir Bilal Yavuz (#7)
Re: 003_extrafiles.pl test fails on Windows with the newer Perl versions

Seems like the back branches are still not quite there: I'm seeing

Name "PostgreSQL::Test::Utils::windows_os" used only once: possible typo at t/003_extrafiles.pl line 74.

in v12 and v13, while it's

Name "PostgreSQL::Test::Utils::windows_os" used only once: possible typo at t/003_extrafiles.pl line 85.

in v14. v15 and up are OK.

regards, tom lane

#9Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#8)
Re: 003_extrafiles.pl test fails on Windows with the newer Perl versions

On 2024-02-01 Th 12:32, Tom Lane wrote:

Seems like the back branches are still not quite there: I'm seeing

Name "PostgreSQL::Test::Utils::windows_os" used only once: possible typo at t/003_extrafiles.pl line 74.

in v12 and v13, while it's

Name "PostgreSQL::Test::Utils::windows_os" used only once: possible typo at t/003_extrafiles.pl line 85.

in v14. v15 and up are OK.

*sigh*

Have pushed a fix. Thanks for noticing.

cheers

andrew

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