Accidentally duplicating tests with Test::Class

Imagine you open up a test file and you see the following:

is foobar(3), 17, 'foobar(3) should return 17';
is foobar(2), 15, 'foobar(2) should return 15';
is foobar(3), 17, 'foobar(3) should return 17';   # duplicate?
is foobar(4), 20, 'foobar(4) should return 20';
is foobar(3), 17, 'foobar(3) should return 17';   # duplicate?

Well, that looks strange and duplicated tests are a code smell. However, it could be a code smell in one of two ways. It's probably the case that some programmer got sloppy and duplicated the tests so do you delete the extra tests?

Of course not! You're an experienced programmer and you've learned to look both ways before crossing a one-way street. Maybe foobar() uses state variables to cache results and caching can sometimes go awry. Or maybe that function connects to an external service. Or maybe there's a rand() tossed in there just for fun. Who knows? If there is a reason those tests should be duplicated, we probably have bad test descriptions and we should rewrite the descriptions so that future programmers will understand why we have duplicated tests.

So you inspect the function and you determine that, yep, the tests are duplicated and should be deleted.

If you use Test::Class or related modules, appreciate the benefits of test inheritance, and are using separate *.t files for every test class, you're making the same mistake.

Consider the following three test classes:

MyParent (10 tests):

package MyParent;

use base 'Test::Class';
use Test::More;

sub test_this : Tests {
    ok 1, "test $_" for 1 .. 5;
}

sub test_that : Tests {
    ok 1, "test $_" for 1 .. 5;
}

1;

MyChild (3 tests):

package MyChild;

use base 'MyParent';
use Test::More;

sub test_this : Tests {
    ok 1, "test $_" for 1 .. 3;
}

1;

And MyGrandChild (no tests!):

package MyGrandChild;
use base 'MyChild';
1;

Those who like to run a separate driver *.t file will create simple driver scripts like this:

use lib 't';
use MyParent;    # or whatever test class we wish to run
Test::Class->runtests;

Running that produces output like this:

t/parent.t .. ok
All tests successful.
Files=1, Tests=10,  0 wallclock secs
Result: PASS

A quick glance at MyParent shows that yes, we're running 10 tests in two test methods.

What happens when we run that with MyChild?

t/child.t .. ok
All tests successful.
Files=1, Tests=18,  0 wallclock secs
Result: PASS

Whoa! What happened? 18 tests? MyChild only defines three tests?

That's because of test inheritance. Test::Class will see that MyChild inherits from another test class and any test methods in the base class that aren't overridden (test_that(), in this case) will be included in the subclass tests. Thus, MyChild runs 3 tests for test_this() (overriding the parent method) and 5 tests for test_that() (inheriting the parent method), for a total of eight tests.

Test::Class will also run the the tests for the parent class, MyParent. That's 5 tests for each test method, or 10 tests, thus leading to our total of 18 tests.

In the case of MyGrandChild, even though we defined no tests, due to test inheritance we get a total of 26 tests run (8 + 8 + 10)! So what happens when you have separate test drivers for each class?

t/child.t ....... ok
t/grandchild.t .. ok
t/parent.t ...... ok    
All tests successful.
Files=3, Tests=54,  0 wallclock secs
Result: PASS

The t/grandchild.t test ran all of the tests that are also in t/child.t and t/parent.t. In other words, you're running a bunch of tests multiple times. If you use inheritance (which you should in your tests if you use it in your classes), you're wasting CPU cycles with duplicated tests. If your test suite already takes too long to run, why would you want to do that?

There are a few ways of approaching this.

The obvious one is to not use test inheritance, but this is a bad idea if your classes inherit from one another. Test inheritance allows us to know that behavior that our child classes inherit hasn't changed. Test inheritance tests this for us free of charge.

Another strategy would be to delete the t/parent.t and t/child.t drivers. That has a couple of problems. First, if you use separate drivers per class and another developer notices that many classes don't have drivers, this could cause a lot of time and pain figuring out if this is an error or not. Worse, if you accidentally forget to write the driver for a class that should have one, you're not running the tests for that class! (I've seen this in large production code bases). Finally, this exposes an implementation detail (inheritance) that should be hidden. If you decide to switch from inheritance to delegation or roles for some behavior, have fun examining all of your driver classes to figure out which ones you really need.

The last strategy, and the one I recommend, is to simply run the tests in a single process:

use Test::Class::Load 't/lib'; # or wherever your test classes are
Test::Class->runtests;

And for the above example that generates:

t/test_classes.t .. ok    
All tests successful.
Files=1, Tests=26,  0 wallclock secs
Result: PASS

And that's what we wanted in the first place. You're not running tests twice and you're probably getting a performance boost to your test suite.

2 Comments

I don't think I get it. You want to preserve the test inheritance, but you don't want the tests to run for each class? 26 tests are run, but there's no message about which test classes were used to run which ones. So how do you know if there isn't broken functionality in the child or grandchild?

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.