cpan-authors / IPC-Run

https://metacpan.org/pod/IPC::Run
Other
21 stars 38 forks source link

allow win32_newlines.t to actually run #146

Closed wchristian closed 2 years ago

wchristian commented 3 years ago

While working on another test i found that this one had a bungled OS check and thus hadn't been running for a while. This PR simply reactivates it. Looks like it fails some tests.

@nmisch and @haarg might wanna have a look at this.

d:\cpan\IPC-Run>perl t\win32_newlines.t
1..10
ok 1 - "ab" - child got clean input
ok 2 - "ab" - parent received clean child output
not ok 3 - "\n" - child got clean input
#   Failed test '"\n" - child got clean input'
#   at t\win32_newlines.t line 42.
#          got: '"\r\n"'
#     expected: '"\n"'
ok 4 - "\n" - parent received clean child output
ok 5 - "\r" - child got clean input
ok 6 - "\r" - parent received clean child output
ok 7 - "\r\n" - child got clean input
not ok 8 - "\r\n" - parent received clean child output
#   Failed test '"\r\n" - parent received clean child output'
#   at t\win32_newlines.t line 42.
#          got: '"\n"'
#     expected: '"\r\n"'
not ok 9 - "\n\r" - child got clean input
#   Failed test '"\n\r" - child got clean input'
#   at t\win32_newlines.t line 42.
#          got: '"\r\n\r"'
#     expected: '"\n\r"'
ok 10 - "\n\r" - parent received clean child output
# Looks like you failed 3 tests of 10.
nmisch commented 3 years ago

This change is correct.

The "$^O = 'Win32';" in windows_search_path.t should change, too.

wchristian commented 3 years ago

Thanks. :)

And, on a cursory check that windows_search_path.t line appears to be a NOP. Can you point out where that leads to mistaken results on checks?

nmisch commented 3 years ago

No, I did not not look for mistaken results. The person who fixes it might choose to remove the $^O = 'Win32'; or might choose to change it to $^O = 'MSWin32';. In its current form, that line, at best, invites future accidents.

wchristian commented 3 years ago

Fair. Also important to mention that the code does /Win32/ in a bunch of places.