pgxn / pgxn-api

Maintain and serve a REST API to search PGXN mirrors
http://pgxn.org/
15 stars 2 forks source link

t/indexer.t may fail (non-English locale?) #25

Closed eserte closed 8 years ago

eserte commented 8 years ago

Some of my smokers report the following failure, probably only when a German locale is in effect, and probably also only for perl <= 5.22:

#   Failed test 'Should get exception with file names for bad copy'
#   at t/indexer.t line 166.
# expecting: Regexp ((?^:Cannot copy t\/test_doc_root\/mirror\/dist\/nonexistent\/0\.1\.0\/nonexistent\-0\.1\.0\.zip to t\/test_doc_root\/dist\/nonexistent\/0\.1\.0\/nonexistent\-0\.1\.0\.zip: No such file or directory))
# found: Cannot copy t/test_doc_root/mirror/dist/nonexistent/0.1.0/nonexistent-0.1.0.zip to t/test_doc_root/dist/nonexistent/0.1.0/nonexistent-0.1.0.zip: Datei oder Verzeichnis nicht gefunden
# Looks like you failed 1 test of 249.
t/indexer.t .. 
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/249 subtests 
theory commented 8 years ago

Hrm. Does this fix it?

diff --git a/t/indexer.t b/t/indexer.t
index 335b9db..550c2db 100644
--- a/t/indexer.t
+++ b/t/indexer.t
@@ -162,7 +162,7 @@ file_not_exists_ok(
 $meta->{name} = 'nonexistent';
 my $src = catfile $api->mirror_root, qw(dist nonexistent 0.1.0 nonexistent-0.1.0.zip);
 my $dst = catfile $api->doc_root,    qw(dist nonexistent 0.1.0 nonexistent-0.1.0.zip);
-throws_ok { $indexer->copy_files($params ) }
+throws_ok { local $ENV{LANG} = 'en_US'; $indexer->copy_files($params ) }
     qr{Cannot copy \Q$src\E to \Q$dst\E: No such file or directory},
     'Should get exception with file names for bad copy';
 $meta->{name} = 'pair';
theory commented 8 years ago

Or maybe set $LANG to "C"?

eserte commented 8 years ago

LC_ALL has higher precedence than LANG, so I propose to set that environment variable. Also, setting locale-specific environment variables within a perl script does not have an effect to the process itself, only to spawned children. But one can use POSIX::setlocale instead:

diff --git a/t/indexer.t b/t/indexer.t
index 335b9db..99bdf6d 100644
--- a/t/indexer.t
+++ b/t/indexer.t
@@ -16,6 +16,13 @@ use Test::MockModule;
 use Archive::Zip;
 use utf8;

+BEGIN {
+    if ($] < 5.022) {
+        require POSIX;
+        POSIX::setlocale(&POSIX::LC_ALL, 'C');
+    }
+}
+
 my $CLASS;
 BEGIN {
     $File::Copy::Recursive::KeepMode = 0;

I am quite positive that POSIX.pm is available in all perl installations nowadays, but maybe you want to be sure and don't fail here if POSIX.pm is missing, in this case you would need another eval block here.

theory commented 8 years ago

Sqitch already requires POSIX, so I think we're good there. Does your patch fix the issue?

eserte commented 8 years ago

Yes, I checked on a linux system with perl 5.20.3.

theory commented 8 years ago

Thank you, as always @eserte!