Deception in tests considered harmful

I felt really good after finally ironing out all the kinks in PR #528 which added support to Perl 6 on MoarVM for handling Unicode script names, command line arguments, and environment strings on the Windows command line. (As a side note, I am indeed working on a similar fix for Perl, but that is going to take a bit more than 60 lines of changes).

A few days ago, after building MoarVM, NQP, and rakudo from a fresh git pull, I decided to check if any spec tests were failing. I reported my results. I was rather pleased that very few tests were failing and some of those failures were because I had forgotten to set my code page to UTF-8.

Zoffix Znet pointed me to nmake stresstest, so I ran that yesterday. That produced more test failures, but I thought the failures were probably due to a small number of actual problems so that fixing one small thing might fix a number of test failures. Then, I looked more closely at one of the test failures, and got a knot in my stomach:

$ 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"
...

I have been, shall we say, alarmed by the way Perl 6 handled differing EOL conventions, but I thought it had been fixed. Soon after I filed my bug report, Zoffix Znet pointed out:

Looks like several other tests in S17-procasync/basic.t would be failing as well if it weren’t for the explicit kludges added to replace "\r\n" to "\n". And grep -nFR '\r\n' | grep subs shows 32 potential places with a similar workaround.

My heart sank!

Handling different EOL conventions is straightforward: When reading text files on platforms where EOL is different than 0x0a, the platform specific EOL sequences in text streams are converted to a canonicalized form when reading. When writing out text streams, the canonical EOLs are converted to the platform specific byte sequence. I don’t know when people started doing this, but I have been aware of it for more than a few decades. But, alas:

$ perl -wE "$x = `perl -E ""say 'hello'""`; say 'ok' if $x eq qq{hello\n}"
ok

Good.

$ perl6 -e "my $x = qx<perl -E ""say 'hello'"">; say $x eq qq<hello\n>"
True

$ perl6 -e "my $x = qx<perl6 -e ""say 'hello'"">; say $x eq qq<hello\n>"
True

Still good. But, then, we have this:

$ more t.pl6
use v6.c;

my $proc = Proc::Async.new('perl', <-E "say 'hello'">);
 
$proc.stdout.tap(-> $v { say $v eq "hello\n"; say $v.perl });
my $promise = $proc.start;
 
await $promise;

$ perl6 t.pl6
False
"hello\r\n"

So, the problem is when Proc::Async is used with text streams, EOL conversion goes out the window. To mask this programming error, test files contain things like:

$so.act: { $stdout ~= $_.subst("\r\n", "\n", :g) };
$se.act: { $stderr ~= $_.subst("\r\n", "\n", :g) };

These kludges create false negatives: Tests that should have failed pass.

Lying in tests like this serve no purpose other than to deceive not just the author of the tests, but also anyone else who relies on these tests to alert them to problems in their code. Remember, this code is testing Proc::Async, and Proc::Async should be handling EOL conversion on text streams.

Simply put, this is disappointing.

While looking at this, I also discovered something else. I copied and pasted the first code example in the docs for Proc::Async and ran it without modification:

$ more asyncex.pl6
# command with arguments
my $proc = Proc::Async.new('echo', 'foo', 'bar');

# subscribe to new output from out and err handles:
$proc.stdout.tap(-> $v { print "Output: $v" }, quit => { say 'caught exception ' ~ .^name });
$proc.stderr.tap(-> $v { print "Error:  $v" });

say "Starting...";
my $promise = $proc.start;

# wait for the external program to terminate
await $promise;
say "Done.";

Note that echo is a cmd.exe builtin.

First, a couple of examples not using Perl 6’s Proc::Async:

$ perl -e "print `echo`"
ECHO is on.

$ perl6 -e "print qx<echo>"
ECHO is on.

So, both perl and perl6 are able to handle a qx using a cmd.exe builtin by spawning a shell and running the command there. That is nice, but not required.

Let’s see what happens when the Proc::Async example is run:

$ perl6 asyncex.pl6
Starting...
MoarVM panic: use of invalid eventloop work item index -1

The fix is simple: Change the constructor invocation to:

my $proc = Proc::Async.new('cmd', </c echo foo bar>);

but there should be no panic from MoarVM just because Proc::Async cannot spawn something.

Finally, another test failure was a false positive: A test that should not have been expected to pass was being included in the run. It was failing not due to a problem with any part of the Perl 6 machinery but due to the way symlinks are restricted on Windows.

When I write about flaws and problems I observe in Perl 6, people call me names and claim I am mocking Perl 6, lying about its stability etc. In truth, I want to be enthusiastic about Perl 6, but I am disappointed more often than not.

At this point, correct handling of differing EOL conventions should not be an issue.

You can comment on this post on r/perl.