Repository navigation
Conversation
Three files fail on Windows and only on Windows in the cpantesters
results for 2.2.2 (matrix.perl-magpie.org/results/ac560063-6c5e-1014-9b5f-c5c807c19ce6).
Each is a platform assumption, in the code or in the test, about the
platform it is running on.
t/unit/logger/format.t
%t and %u are documented as dd/Mon/yyyy hh:mm:ss and were built by
handing "%d/%b/%Y %H:%M:%S" to strftime. The MinGW C runtime behind
Strawberry Perl renders %b as the month number, so a September log
line came out "29/9/2026 23:59:25" and failed the test's
[A-Z][a-z]{2}. The four date codes are now assembled field by field
with sprintf, and the month from a fixed Jan..Dec table, so that the
documented format is the format produced on every platform. strftime
is still used for %{...}t, where the format string is the caller's
own and whatever their C library makes of it is what they asked for.
t/e2e/cli/gen.t
The generated MANIFEST and MANIFEST.SKIP were written on handles
opened without an explicit layer. On Windows that is a text-mode
handle, so every \n became \r\n, and the pattern the generator
appends to MANIFEST.SKIP ended in a carriage return -
"^MyApp-App-\r" - which does not match the path it is there to
match. Both files are now written with binmode ':raw', as
_copy_templates already did for every other file it writes.
The subtest that checks an explicit -d is honoured verbatim used
"-d Other::App". A colon cannot appear in a Windows file name at
all - it introduces an alternate data stream - so no Windows file
system can hold that directory and a generator asked to make one
there can only fail. The check is now made with a name Windows can
hold, and the colon spelling is checked under a skip on Windows, so
the assertion still runs where such a directory can exist.
t/integration/handler/file.t
The surplus-../ case builds a path back down to the fixture from the
filesystem root, using $ROOT->relative('/'). On Windows that hands
back the whole path including the drive letter, and a drive letter
in the middle of a request path is not a name any file can have, so
the handler's existence check answered 404 before its containment
check was ever reached. That is still safe, but it is the wrong
thing for a subtest about the guard to be exercising. The path is now
built from the fixture's path with the volume and root stripped, so
the filesystem root is the only thing that surplus ../ can collapse
at on every platform, and the test is back to testing the guard.
Verified: the full suite passes (209 files, 1857 tests); the two
generated files come out with LF under PERLIO=crlf, which reproduces
Windows' text-mode newline translation elsewhere, before and after; and
the logger output is checked against a strftime patched to return the
month number, where the old code reproduces the reported failure and the
new code does not.
Review feedback, and a correction to my own earlier claim: the fixed Jan..Dec table I reached for first would have changed the output for everyone whose locale is not English, not just on the platform that broke. Perl calls setlocale(LC_ALL, "") at startup, so LANG does reach strftime - with LC_ALL=de_DE.UTF-8 the old code renders March as "Mär" and French as "mars", which is what people reading their own logs expect, and it is behaviour worth keeping. (I had assumed the environment locale would not reach strftime and would not have noticed. It does.) So ask strftime for %b as before, and fall back to the English abbreviation only when the answer is not a name at all - which is the MinGW case, where %b comes back as the month number. Locale-aware where the platform can be, deterministic where it cannot. Verified against de_DE and fr_FR built locally: the output is identical to the old code's under each, and under a strftime patched to return the month number the old code reproduces the reported 29/9/2026 while the new code gives 29/Sep/2026. Full suite passes.
|
Follow-up on the
Perl calls
Verified with
and with Full suite passes (209 files, 1857 tests). So the "behaviour change for non-English One correction to my own reasoningIn the original description I wrote that |
The test asserted the month is exactly `[A-Z][a-z]{2}`, which passes in
C/en_GB but fails in many locales: e.g. French `mars` or `sept.`, and
German `Mär` (non-ASCII). The old code would also have produced those
values and the test would have failed on such systems even though the
code behaved as documented.
Relax the check to require that the month field is not all digits and
contains at least one non-slash character — enough to catch the actual
MinGW regression where `%b` returned the month number (`29/9/2026`),
while not breaking on real locale month names. This matches the
locale-preserving fix to the logger itself.
All logger tests still pass in C, en_GB, de_DE, and fr_FR locales.
|
Second follow-up: the date-codes test itself was locale-sensitive, and is now loosened. Raised while fixing the month regression, so noting it here rather than opening a
This is a pre-existing failure in v2.2.2, not something the earlier commits The assertion now requires that the month field is not all digits (and is not my $MONTH = qr{[^\d/][^/]*};The digit check is what keeps the test meaningful. Every real month name contains
I also checked for the same assumption elsewhere — it appears only in this |
1 participant
Fixes the three test files that fail on Windows and only on Windows in the
cpantesters results for v2.2.2
(report).
Each was a platform assumption, in the code or in the test, about the platform
it is running on.
t/unit/logger/format.t
%tand%uare documented asdd/Mon/yyyy hh:mm:ss, and were built byhanding
"%d/%b/%Y %H:%M:%S"tostrftime. The MinGW C runtime behindStrawberry Perl renders
%bas the month number, so a September log line cameout
29/9/2026 23:59:25and failed the test's[A-Z][a-z]{2}.The four date codes are now assembled field by field with
sprintf, so theyzero-pad the same way everywhere, and the month falls back to the English
abbreviation only where
strftimehas no name to give back — so a localisedmonth name still works where the platform supports it.
strftimeis still usedfor
%{...}t, where the format string is the caller's own.See the follow-up comment: an earlier revision of this PR used a fixed month
table, which would have changed
%t/%ufor every non-English locale, not justfor the platform that broke.
t/e2e/cli/gen.t
The generated
MANIFESTandMANIFEST.SKIPwere written on handles openedwithout an explicit layer. On Windows that is a text-mode handle, so every
\nbecame\r\nand the pattern the generator appends toMANIFEST.SKIPended in a carriage return (
^MyApp-App-\r), which does not match the path itis there to match. Both files are now written with
binmode ':raw', as_copy_templatesalready did for every other file it writes.Separately, the subtest checking that an explicit
-dis honoured verbatimused
-d Other::App. A colon cannot appear in a Windows file name at all - itintroduces an alternate data stream - so no Windows file system can hold that
directory and a generator asked to make one there can only fail. That check now
uses a name Windows can hold, and the colon spelling is checked under a
SKIPon Windows, so the assertion still runs everywhere such a directory can exist.
The pre-existing assertions on the
-druns nowdiagSTDERR/STDOUT when thegenerator exits non-zero, since "died with status 5632" was not enough to
diagnose it from the report.
t/integration/handler/file.t
The surplus-
../case built a path back down to the fixture from thefilesystem root using
$ROOT->relative('/'). On Windows that hands back thewhole path including the drive letter, and a drive letter in the middle of a
request path is not a name any file can have, so the handler's existence check
answered 404 before its containment check was reached - still safe, but the
wrong thing for a subtest about the guard to be exercising. The path is now
built from the fixture's own path with the volume and root stripped, so the
filesystem root is the only thing surplus
../can collapse at on everyplatform.
Verification
PERLIO=crlf(which reproduces Windows' text-mode newline translation onother platforms) shows the generated
MANIFEST/MANIFEST.SKIPcoming outwith LF after the fix, and CRLF before it - and
gen.tfailing before andpassing after under the same setting.
strftimepatched to return the monthnumber: the old code reproduces the reported
29/9/2026 23:59:25and failsthe test's regex, the new code renders
29/Sep/2026 23:59:25and passes.Not fixed here
t/config_reader.talso failed in that report, with a syntax error at line 17(
__FILE__()is not parseable on perl 5.14.4). That is an old-perl issue ratherthan a Windows one, so it is left alone - worth its own fix, since it will fail
on any 5.14.