Skip Menu |
Report information
Id: 130788
Status: resolved
Priority: 0/
Queue: perl6

Owner: Nobody
Requestors: sinan [at] unur.com
Cc:
AdminCc:

Severity: (no value)
Tag: (no value)
Platform: (no value)
Patch Status: (no value)
VM: (no value)



To: rakudobug [...] perl.org
From: "A. Sinan Unur" <sinan [...] unur.com>
Subject: Incorrect treatment of EOL causes test failures on Windows
Date: Wed, 15 Feb 2017 13:10:42 -0500
Download (untitled) / with headers
text/plain 945b
$ perl t\harness5 t\spec\S17-procasync\basic.rakudo.moar --verbosity=5 ... not ok 33 - Tapping stdout supply after start of process does not lose data # Failed test 'Tapping stdout supply after start of process does not lose data' # at t\spec\S17-procasync\basic.rakudo.moar line 109 # expected: "Hello World\n" # got: "Hello World\r\n" ok 34 - Process that doesn't output anything will not emit # FUDGED! # Looks like you failed 1 test of 34 Dubious, test returned 1 (wstat 256, 0x100) For text streams, the standard way of dealing with differing EOL conversions is to convert "\n" to the platform specific EOL sequence on output and convert the platform specific EOL sequence to "\n" on input. It looks like that is not being done in Proc::Async or whatever plumbing it depends on. See also https://www.nu42.com/2015/12/perl6-newline-behavior-fixed.html and https://github.com/perl6/roast/issues/232#issuecomment-280061047 -- Sinan
RT-Send-CC: perl6-compiler [...] perl.org
Download (untitled) / with headers
text/plain 611b
Looks like several other tests in S17-procasync/basic.t would be failing as well if it weren't for the explicit kludges[^1][^2] added to replace "\r\n" to "\n". And `grep -nFR '\r\n' | grep subs` shows[^3] 32 potential places with a similar workaround. Pretty LTA for portable Perl 6 code to require any such thing. [1] https://github.com/perl6/roast/blob/045e538f0f28dedb7261b7c013eec16a5cf2ee7e/S17-procasync/basic.t#L19-L20 [2] https://github.com/perl6/roast/blob/045e538f0f28dedb7261b7c013eec16a5cf2ee7e/S17-procasync/basic.t#L62-L63 [3] https://gist.github.com/zoffixznet/c343ed73b830d57e022eb99478764c78
Subject: Re: [perl #130788] Incorrect treatment of EOL causes test failures on Windows
To: perl6-bugs-followup [...] perl.org
Date: Wed, 15 Feb 2017 16:49:13 -0500
From: "A. Sinan Unur" <sinan [...] unur.com>
Download (untitled) / with headers
text/plain 337b
On Wed, Feb 15, 2017 at 3:40 PM, Zoffix Znet via RT <perl6-bugs-followup@perl.org> wrote: Show quoted text
> Looks like several other tests in S17-procasync/basic.t would be failing as well if it weren't for the explicit kludges[^1][^2] added to replace "\r\n" to "\n".
*Sigh* ... I was in a hurry, so I did not look. This is disappointing. -- Sinan
From: "A. Sinan Unur" <sinan [...] unur.com>
Subject: Re: [perl #130788] Incorrect treatment of EOL causes test failures on Windows
To: perl6-bugs-followup [...] perl.org
Date: Wed, 15 Feb 2017 17:08:06 -0500
Download (untitled) / with headers
text/plain 187b
RT-Send-CC: perl6-compiler [...] perl.org
Download (untitled) / with headers
text/plain 1.6k
On Wed, 15 Feb 2017 10:11:01 -0800, sinan@unur.com wrote: Show quoted text
> $ perl t\harness5 t\spec\S17-procasync\basic.rakudo.moar > --verbosity=5 > ... > not ok 33 - Tapping stdout supply after start of process does not lose > data > > # Failed test 'Tapping stdout supply after start of process does not > lose data' > # at t\spec\S17-procasync\basic.rakudo.moar line 109 > # expected: "Hello World\n" > # got: "Hello World\r\n" > ok 34 - Process that doesn't output anything will not emit > # FUDGED! > # Looks like you failed 1 test of 34 > Dubious, test returned 1 (wstat 256, 0x100) > > For text streams, the standard way of dealing with differing EOL > conversions is to convert "\n" to the platform specific EOL sequence > on output and convert the platform specific EOL sequence to "\n" on > input. > > It looks like that is not being done in Proc::Async or whatever > plumbing it depends on. >
Looks like it may have worked originally (back when the \r\n translation fixes were done late 2015), but then regressed more recently when a large set of changes were made to make decoding error handling more robust. I've fixed it now to do the translation again, and also added a :translate-nl option that can be used to control it (the default is to do the translation, but this will allow it to be disabled for anybody who wishes to do so). I've also added a test that runs a program with Proc::Async that spits a \r\n out whatever platform we're running on, so if we break this again we'll end up with a test failure even on Linux/OSX. Finally, the tests that did an explicit .subst have now been changed not to do so; I suspect they date back a few years, before we did any of the work on newline translation.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org