New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

t/options.t fails with non-English locale and older perls (0.9999) #427

Open
eserte opened this Issue Feb 1, 2019 · 5 comments

Comments

Projects
None yet
2 participants
@eserte
Copy link

eserte commented Feb 1, 2019

On some of my systems:

#   Failed test 'Error message should be correct'
#   at t/options.t line 202.
#          got: 'Cannot change to directory nonesuch: Ungültiger Dateideskriptor'
#     expected: 'Cannot change to directory nonesuch: Bad file descriptor'
# Looks like you failed 1 test of 95.
t/options.t ......... 
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/95 subtests 

This happens with a German locale (LC_ALL=de_DE.ISO8859-1 or similar) and older perls (< 5.22).

@theory

This comment has been minimized.

Copy link
Collaborator

theory commented Feb 2, 2019

Ugh, thanks. Does this fix it?

--- a/t/options.t
+++ b/t/options.t
@@ -196,13 +196,14 @@ is_deeply $opts, {}, 'Should have preserved no opts';
 CHDIE: {
     local $! = 9;
     $chdir_fail = 1;
+    my $exp_err = do { chdir 'nonesuch'; $! };
     throws_ok { $CLASS->_parse_core_opts(['-C', 'nonesuch']) }
         'App::Sqitch::X', 'Should get error when chdir fails';
     is $@->ident, 'fs', 'Error ident should be "fs"';
     is $@->message, __x(
         'Cannot change to directory {directory}: {error}',
         directory => 'nonesuch',
-        error     => 'Bad file descriptor',
+        error     => $exp_err,
     ), 'Error message should be correct';
 }
 
@eserte

This comment has been minimized.

Copy link
Author

eserte commented Feb 2, 2019

Yes.

theory added a commit that referenced this issue Feb 2, 2019

@theory

This comment has been minimized.

Copy link
Collaborator

theory commented Feb 2, 2019

Thanks! I'd also like to take a stab at silencing these warnings:

Prototype mismatch: sub IO::Pager::new (*;$@) vs none at /usr/perl5.18.4p/lib/site_perl/5.18.4/Test/MockModule.pm line 163.
Prototype mismatch: sub IO::Pager::new: none vs (*;$@) at /usr/perl5.18.4p/lib/site_perl/5.18.4/Test/MockModule.pm line 163.

So weird that new() has a prototype. Oddly, I've not been able to reproduce the warning, though. Can you tell me if this patch removes it?

diff --git a/t/base.t b/t/base.t
index b97c0f6c..5779ecaf 100644
--- a/t/base.t
+++ b/t/base.t
@@ -213,7 +213,11 @@ PAGER_PROGRAM: {
 
     # Mock the IO::Pager constructor.
     my $mock_pager = Test::MockModule->new('IO::Pager');
-    $mock_pager->mock(new => sub { return bless => {} => 'IO::Pager' });
+    do {
+        # Suppress prototype mismatch warning; IO::Pager::new oddly has a prototype.
+        no warnings 'prototype';
+        $mock_pager->mock(new => sub { return bless => {} => 'IO::Pager' });
+    };
 
     # No pager if no TTY.
     my $pager_class = -t *STDOUT ? 'IO::Pager' : 'IO::Handle';

And if not, I suspect this one will:

diff --git a/t/base.t b/t/base.t
index b97c0f6c..847d63ce 100644
--- a/t/base.t
+++ b/t/base.t
@@ -213,7 +213,11 @@ PAGER_PROGRAM: {
 
     # Mock the IO::Pager constructor.
     my $mock_pager = Test::MockModule->new('IO::Pager');
-    $mock_pager->mock(new => sub { return bless => {} => 'IO::Pager' });
+    do {
+        # Suppress prototype mismatch warning; IO::Pager::new oddly has a prototype.
+        local $SIG{__WARN__} = sub {};
+        $mock_pager->mock(new => sub { return bless => {} => 'IO::Pager' });
+    };
 
     # No pager if no TTY.
     my $pager_class = -t *STDOUT ? 'IO::Pager' : 'IO::Handle';

Thank you for your help @eserte, Sqitch is better thanks to all of your smoke testing.

@eserte

This comment has been minimized.

Copy link
Author

eserte commented Feb 2, 2019

I think it's better to require a higher Test::MockModule version: https://metacpan.org/source/GFRANKS/Test-MockModule-v0.170.0/Changes#L10
I can confirm that there's no warning with 0.170.0, and prototype warnings up to 0.15 (no reports generated on my system with Test::MockModule versions in between)

@theory

This comment has been minimized.

Copy link
Collaborator

theory commented Feb 4, 2019

Ah, right, thanks. done in 3c0d22b.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment