26 November 2017

Test All The Things

Is this thing on? Hello? Great, you can see me. This time is all about unit testing in Perl 6. Are you curious about what that t/ directory is for, and want to fill that empty space with some test files? You've come to the right post. If you don't know what I'm talking about, now might be a good idea to go look at a few Perl 6 modules and check out the testing directory.

Perl has a long tradition of extensive test suites for its modules, and Perl 6 continues that tradition. You can start by downloading a module from GitHub following the Perl 6 modules link and checking out its t/ directory.

Perl 6 comes with a built-in Test module, which looks a lot like Perl 5's Test::More module. I'm not going to go into great detail (in this post, at least) about what all the methods do, I'm going to focus on just two or three ideas that I came up with when writing my own test suites.

DRYing out your tests

Sometimes you get into the zone writing tests, and your tests start to bunch up.

is sprintf( "%s", "a" ), "a", "'a' roundtrips.";
is sprintf( "%s", "€" ), "€", "'€' roundtrips.";
is sprintf( "%s", "\x[263a]" ), "\x[263a]", "Smiley roundtrips.";

While something of a contrived example, it's pretty obvious that all of these tests should be bundled together. You certainly could put them in their own file, and in this case it might be a good idea because being able to roundtrip strings (make sure that the input is the same as the output) needs some pretty thorough testing.

Right now, though, as it stands, three lines hardly is worth the effort to think of a new name for the file, copy to the new location, delete the old content, add it to git and do a push. Let's come up with an easier way to group these.

Visually separating them with a block certainly works...

{
  is sprintf( "%s", "a" ), "a", "'a' roundtrips.";
  is sprintf( "%s", "€" ), "€", "'€' roundtrips.";
  is sprintf( "%s", "\x[263a]" ), "\x[263a]", "Smiley roundtrips.";
}

While it certainly looks better, it doesn't help the repetition any. "roundtrips" still repeats, and every time we come up with a new string that might break sprintf() we've got to add it in three places. Let's tackle that first, before going on to the final round.

It's certainly tempting to write a quick subroutine to do this, so let's dash off one.

sub roundtrip( $name ) {
  is sprintf( "%s", $name ), $name, "'$name' roundtrips."
}
{
  roundtrip( "a" );
  roundtrip( "€" );
  roundtrip( "\x[263a]" );
}

Yippee! We've eliminated almost all of the redundancy, and our test output still works!

ok 1 - 'a' roundtrips.
ok 2 - '€' roundtrips.
ok 3 - '☺' roundtrips.

There's a problem lurking here, though. A couple, actually. What happens when sprintf() breaks?

ok 1 - 'a' roundtrips.
not ok 2 - '€' roundtrips.
# Failed test ''€' roundtrips.'
# at t/10-sprintf.t line 4
ok 3 - '☺' roundtrips.

Pretending we're in a hurry and this is just one of a number of problems we have to debug this evening (just like in real life), open your editor and go to line 4 to quickly trace down the bug...

use Test;

sub roundtrip( $name ) {
  is sprintf( "%s", $name ), $name, "'$name' roundtrips." # line 4
}
{
  roundtrip( "a" );
  roundtrip( "€" );
  roundtrip( "\x[263a]" );
}

Hold the phone here, I expected to jump to the test that failed, and I'm on a test subroutine! This would get even more confusing if I had a test library, and roundtrip() wasn't even in my test file. It'd be even a bit confusing if I just saw the word 'roundtrip' repeated over and over, and just searched for that. Or even worse, imagine that this is a file with 200+ tests in it, and every tenth test is for a low Unicode character, so you've got to page through 20 different tests 'til you find the right one. There's got to be a better way.

Now you could certainly throw away your changes, roll back to the point where you refactored and call the time a waste. It's easy enough to salvage, though.

use Test;

sub roundtrip( $name ) {
  sprintf( "%s", $name )
}
{
  is roundtrip( "a" ), "a", "'a' roundtrips.";
  is roundtrip( "€" ), "€", "'€' roundtrips.";
  is roundtrip( "\x[263a]" ), "\x[263a]", "'\x[263a]' roundtrips.";
}

This solves the problem, so when an is() test fails, we'll get pointed directly at the line number, and can jump there in Atom, Emacs, Vim or whatever. But we've gotten our duplication back. Let's try to refactor our way out of this, while making sure that we don't put the is() back into the helper roundtrip() subroutine.

We'll start by noting that is( $string, $roundtripped ) (ignoring the $message parameter) is equivalent to ok( $string eq $roundtripped ). Change the test from is() to ok() first, then add eq $name inside roundtrip(), and get rid of the redundant argument.

use Test;

sub roundtrip( $name ) {
  sprintf( "%s", $name ) eq $name
}
{
  ok roundtrip( "a" ), "'a' roundtrips.";
  ok roundtrip( "€" ), "'€' roundtrips.";
  ok roundtrip( "\x[263a]" ), "'\x[263a]' roundtrips.";
}

That's pretty good, but there's a constant 'roundtrips.' string there. Also we've got this {..} block that's unused, so let's put that block to work by factoring out the 'roundtrips.' bit, using subtest {..}.

use Test;

sub roundtrip( $name ) {
  sprintf( "%s", $name ) eq $name
}
subtest 'roundtrips', {
  ok roundtrip( "a" ), "'a'";
  ok roundtrip( "€" ), "'€'";
  ok roundtrip( "\x[263a]" ), "'\x[263a]'";
};

Looking good, but we've lost a bit along the way. Earlier, when we ran our test suite, we'd get nicely labeled output. Now... not so much.

  ok 1 - 'a'
  ok 2 - '€'
  ok 3 - '☺'
ok 1 - roundtrips

In a simple, short file like this, scanning from the ok 1 - 'a' line, thinking "Okay, why are we testing 'a'?... Aha, roundtrip tests." is pretty quick, and the indentation tells us we're grouping a bunch of tests, but it would be really nice to be able to put that test message where it belongs, in the roundtrip message. So let's do just that.

use Test;

sub roundtrip( $name ) {
  sprintf( "%s", $name ) eq $name, "'$name' roundtrips."
}
subtest 'roundtrips', {
  ok roundtrip( "a" );
  ok roundtrip( "€" );
  ok roundtrip( "\x[263a]" );
};

Great, we've got just one test function that tests and gives us a message! Let's run it!

  ok 1 -
  ok 2 -
  ok 3 -
ok 1 - roundtrips

What's going on here? roundtrip() returns both the truthiness of our test and the message correctly, you can test that on the command line yourself. Yes, Virginia, Perl 6 subroutines can return more than one value - perl6 -e'sub test { "foo", "bar" }; say test();' will return [ foo bar ] as you'd expect.

So ok() is getting the list that roundtrip() returns, and should be unpacking that list and...hey, waitaminnit. roundtrip() returns a list, and ok() expects one argument and one optional argument... maybe that's what's going wrong here. So, how do we solve this?

Luckily for us, there's an easy way to unpack our list into two arguments. It's not quite destructuring (there's another way to do that) but it works for us. The | (pipe) symbol before a list "expands" that list inline into a bunch of individual arguments, so let's put that before the roundtrip() call and unpack the list.

use Test;

sub roundtrip( $name ) {
  sprintf( "%s", $name ) eq $name, "'$name' roundtrips."
}
subtest 'roundtrips', {
  ok |roundtrip( "a" );
  ok |roundtrip( "€" );
  ok |roundtrip( "\x[263a]" );
};

And rerunning our test suite now, our output is what we expect.

  ok 1 - 'a' roundtrips.
  ok 2 - '€' roundtrips.
  ok 3 - '☺' roundtrips.
ok 1 - roundtrips

This may be a bridge too far for some, and I respect your decision. Using the subtest() block may be all you need, because you can quickly search for a keyword in the string and bounce immediately to the start of the tests where one fails. You may even want to go to the lengths of using ok( roundtrip($_) ) for < a € ☺ > to get rid of the last duplicated call to roundtrip(), that's your choice. all I'm offering here is some ways to DRY out your test files.

Gentle Reader, if you've gotten this far, thank you. I do read all the comments that I get, at least eventually. I'm also @DrForr on Twitter, 'Jeff Goff' on Facebook and LinkedIn. Thank you for your time, and I hope it was worth your while.

19 April 2017

Wrangling Metadata the Right Way

If you're new to Regular Expressions or Grammars (at least as they're used in Perl 6), then I'd suggest starting with the 1st part of a full tutorial on Perl 6 grammars. Now to the heart of the matter!

I'm rewriting the ANTLR -> Perl 6 grammar translator, and the code was simply too old to really be refactored - it predated the Great List Refactor, ask any old Perl 6 hand about that. This does require some familiarity with how grammars and Abstract Syntax Trees are related, so please bear with me.
Grammar rules (rules that tell Perl 6 how you want to match text) look something like:
rule parserRuleSpec
        {
        <ID>
        ':'
        <parserAltList>
        ';'
        }
Here an <ID> is simply the name of a parser rule, and <parserAltList> is its body, consisting of a series of "alternations". Like YACC, Marpa or ANTLR, you can tell Perl 6 to run a block of code when it successfully parses a <parserRuleSpec> in the target string. Let's assume that our target looks like:
integer : \d+ ;
Even if you don't know the exact details, it shouldn't be hard to guess that <ID> matches 'integer', and <parserAltList> matches '\d+'. If you don't ask Perl 6 to do anything to this, the 'integer' and '\d+' bits tend to sort of get lost in what can look like a confusing blizzard of hashes and arrays, it certainly was confusing to me. But we can help make sense of all of this. When this particular rule succeeds, we can stop right there and do something useful with that data. It'd be nice to get back a neat little hash, with some consistently-named keys, so we can do something like this:
method parserRuleSpec( $/ )              # rule parserRuleSpec
        {                                #         {
        make {                           #
             name =>                     #
                 $/<ID>.Str,             #         <ID>
                 # ':'                   #         ':' 
             body =>                     #
                 $/<parserAltList>.Str   #         <parserAltList>
                 # ';'                   #         ';'
             }                           #
        }                                #         }
[If this doesn't render neatly, my apologies, but I wanted to make it clear what I've done.] I've added a parserRuleSpec() method that will be run whenever the parser completely and correctly matches a parserRuleSpec rule. Whenever I want to inspect a parserRuleSpec rule, I don't have to dig around inside Match objects and look for strings, all I need to do is something like 'say $/<parserRuleSpec>.ast.gist' and now I'll get a nice simple representation of a parserRuleSpec as a hash with 'name' and 'body' keys:
say $/<parserRuleSpec>.ast.gist;

# {name => 'integer', body => '\d+'}
So our potentially-gnarly parserRuleSpec is now tamed, and resides in a nice neat data structure. And if you note, it's a pretty mechanical process. Copy the original rule, change 'rule' to 'method', wrap with 'make { }', and give names to the <ID> etc. tags. In fact, you could probably automate it, but that's another article for another time :) My point here is that you don't always want this kind of routinely-generated data structure.

If you have a rule like 'integer : \d+ ;', then it's not unreasonable to assume that other rules will follow, like 'float : \d+ "." \d+ ;'. Eventually, you'll want to collect these into some other data structure, maybe like this:
my %rules;
for $/<parserRules> -> $rule
        {
        %rules{ $rule.<name> } = $rule.<body>;
        }
say %rules.gist;

# {integer => '\d+', float => '\d+ "." \d+'}
Or you could even write it more compactly:
my %rules;
%rules{ $_.<name> } = $_.<body> for $/<parserRules>;
say %rules.gist;

# {integer => '\d+', float => '\d+ "." \d+'}
There are of course other ways to write this, but there's another way altogether to avoid the complications. When I started out on this current project (early last year, as it happens) I was of the opinion that the AST return blocks should contain all of the information they needed, so that I never had to "go back" for more data. During the rewrite I realized that's not necessarily the case, and I can make things more flexible in this case by simply not returning the name as part of the AST, and letting the outer layer do what it wants with the actual name. So, this code now looks like: (skipping the original rule declaration)
method parserRuleSpec( $/ )
        {
        make $/<parserAltList>.Str
        }

my %rules;
for $/<parserRules> -> $rule
        {
        %rules{ $rule<ID>>.Str } = $rule.ast;
        }
say %rules.gist;

# {integer => '\d+', float => '\d+ "." \d+'}

A bit of philosophy, if I may

There are two subtle points to be made about this. The first is that I'm separating the metadata (just the name, but ANTLR decorates rules with other things) from the meat of the rule, so that the user doesn't have to wade through as much detail initially. When the user wants to look at the rule, a simple 'say $rule.ast.gist' will give them an idea of what the rule contains, without a blizzard of metadata like actions and exceptions.

Second, and more subtle, as the author of the code I don't have to care about what's in a rule, I just have to grab the .ast and return it. Say, for instance, I got halfway through coding these rules up, and realized that alongside the 'content' I needed to pass an action parameter as well, so
my %rules;
for $/<parserRules> -> $rule
        {
        %rules{ $rule.<name> } = $rule.<body>;
        }
say %rules.gist;

# {integer => '\d+', float => '\d+ "." \d+'}
now had to have an 'action' hash key added:
my %rules;
for $/<parserRules> -> $rule
        {
        %rules{ $rule.<name> } = # If only this were $rule.ast...
            {
            body => $rule.<body>,
            action => rule.<action>
            };
        }
say %rules.gist;

# {integer => {body => '\d+', action => '$int-count++'}, float => {body => '\d+ "." \d+', action => '$float-count++'}}
Now, every time I want to add a new argument alongside 'body', I have to add it in at least two places - the original method that generated the .ast data structure, and the place in the driver code where I capture the data and store it in a higher-level .ast data structure. Actually I've forgotten about the test code (sigh, yes, again) so make that three places. And the documentation makes at least four. Those last two don't count as much because no matter what you do you've got to update test data, and if your test suite didn't catch the change, it really shouldn't be called a test suite unless you're doing some very specific black-box testing.


In conclusion

After this long writeup I'm going to go back to the ANTLR4 test bed with an eye towards eliminating this redundancy. The idea of capturing everything I needed in a grammar layer in a single pass appealed to me when I was first starting to write this, and I imagine the idea of "one grammar rule returns one AST hash" has some appeal. It's of course not as simple, especially since grammar rules can match optional things, match X or Y or Z but not all, and so on, but knowing that everything you need is in a single hash is handy when you're starting out, but I'm now seeing this as a counterexample to the KISS (Keep It Simple, you Silly thing) principle, and am going to check in the current ANTLR layer and refactor with this in mind before doing more work.
Gentle Reader, if you've gotten this far, thank you. I do read all the comments that I get, at least eventually. I'm also @DrForr on Twitter, 'Jeff Goff' on Facebook and LinkedIn. Thank you for your time, and I hope it was worth your while.