A Test::Class Anti-Pattern

I've been having a great time at YAPC::NA (my first YAPC In the US) and so far everything's been going swimmingly ... well, except for when I was giving a lightning talk and my phone kept buzzing in my pocket because my wife was lost.

I gave a talk on Test::Class::Moose and there were plenty of interesting questions and many of them were about topics I would like to have included in the talk, but simply didn't have time. One person emailed me later asking me a detailed question about test control methods and while I've discussed this before, I thought it was important to re-explain a common anti-pattern I see in Test::Class test suites.

First, here are my slides:

In those slides, I explain that test control methods (methods that handle startup/setup/shutdown/teardown) in Test::Class::Moose are named test_startup, test_setup, test_teardown and test_shutdown. This surprises some people who are used to Test::Class because they're used to doing something like this:

sub connect_to_db : Test(startup) {
    ...
}

In Test::Class you can name a test control method anything you want and you annotate it with attributes. In this case, the Test(startup) attribute identifies this as a startup method (run before the test class executes).

That leads to seeing curious things like this in a subclass:

sub xxx_build_fixture : Test(startup) {
    ...
}

Why that "xxx_" prefix? Because that "build fixture" code relies on a database connection and the test control methods from your current class and your parent classes are run in alphabetical order.

And it gets worse. You find yet another subclass and this time you see a "zzz_" startup method and find out that xxx_build_fixuture() used to be named zzz_build_fixture() but was renamed to fit Yet Another Method into this inheritance chain but tweaked so it would run in the correct order.

This is OO code. These are classes. People somehow forget about this. What if you don't want the above behavior(s) to fire? Now you're hunting through parent methods to see which ones are implemented where and trying to figure out which method names to override and then some programmer adds a new startup method to a base class and BOOM.

I see the above anti-pattern constantly, but what you're really looking for is this:

sub startup : Test(startup) {
    my $test = shift;
    $test->next::method;
    # order is no longer dependent on method names
}

Name your test control method after the phase it represents. And that solves the problem. You know always know what the parent class method is. This is why these methods have explicit names in Test::Class::Moose:

sub test_setup { # no more wondering what this name is
    ...
}

Interestingly, I had someone request that this "multiple setup method" feature be added to Test::Class::Moose. They like the idea of breaking down their test control methods into different methods and running the sequentially, even within the same class! Ignoring the reasons why he wanted to do this, here's how I recommended this actually be written with Test::Class::Moose:

sub test_startup {
    my ( $test, $report ) = @_;

    $test->next::method($report); # always call your parent setup

    my @methods = $test->startup_methods; # Ah, much cleaner
    foreach my $method (@startup_methods) {
        $test->$method($report);
    }
}

He was quite happy with this cleaner solution. By using clean OO and taking advantage of it, you have tremendous flexibility.

Hopefully this explains why Test::Class::Moose chooses to be a bit rigid about some names. I see the above mistake so many times that I wanted to discourage it. You can still do that if you need to, as per my last example above, but now you have to consciously decide to do so rather than accidentally falling into this trap.

Leave a comment

About Ovid

user-pic Freelance Perl/Testing/Agile consultant and trainer. See http://www.allaroundtheworld.fr/ for our services. If you have a problem with Perl, we will solve it for you. And don't forget to buy my book! http://www.amazon.com/Beginning-Perl-Curtis-Poe/dp/1118013840/