When Good Ideas Go Bad: use_ok

Ovid on 2009-06-04T12:49:35

Sometimes reasonable ideas evolve into good ideas and then evolve into great ideas and paradoxically become a bad idea. Consider the following:

use Test::More tests => 1;
use_ok 'My::Module';

Most who are familiar with testing know that use_ok is a valuable test, but the above code is bad because some behaviors must happen at compile time (function exports and prototypes are the examples cited in the docs). So you really want this:

use Test::More tests => 1;
BEGIN { use_ok 'My::Module'; }

But as it turns out, that doesn't quite do it either. You see, if use_ok fails, My::Module may be partially compiled, causing your tests to run, but maybe throwing strange errors later. This can be extremely confusing. The strategy recommended by some is to fix the first test failures first, but a more reasonable strategy is to die immidiately:

use Test::More tests => 1;
BEGIN { use_ok 'My::Module' or die; }

Or if you prefer Test::Most to do that for you:

use Test::Most 'die', tests => 1;
BEGIN { use_ok 'My::Module'; }

It probably would have been better if use_ok simply died or skipped the rest of the tests for you, but backwards-compatability is being maintained.

This test is so useful, unfortunately, that we now have the following anti-pattern:

$ ack -l 'BEGIN.*use_ok' t |wc -l
23

Why is this bad? Consider the following:

is $foo, $bar, 'foo and bar are the same';
is $foo, $bar, 'foo and bar are the same';
is $foo, $bar, 'foo and bar are the same';
is $foo, $bar, 'foo and bar are the same';

You'd probably look at that a bit suspiciously. It might be valid (but the test names should make it clear why), but it's probably just useless, duplicated tests. When I started on this BBC team, I found the many use_ok statements scattered through the code and many were testing the same module over and over again. There's no need to do that. Just create one t/use.t file and put everything there.

Aside from the obvious duplication of behavior, why would I care about this? Well, a few days ago, Schwern announced he was pulling in my nested TAP fork (Adrian worked on that fork, too). Adrian Howard plans on updating Test::Class with this and he already has much of the code written for it. I plan to update Test::Aggregate with this and I already have much of the code written for this. Others have already discussed all sorts of interesting ideas with nested TAP. That's where the BEGIN idea breaks down.

Tests in BEGIN/CHECK/INIT/END won't run in the normal sequence of other tests. This means that subtests will likely break very badly if any subtests are run in those blocks. The latest dev release of Test::Aggregate already heavily deprecates tests run in those blocks (it deprecates them more strongly than I wish, but encouraging nested TAP is more important than Test::Aggregate. Nested TAP is very exciting (sad, ain't I?) and it's time we get this out there for people.


useless complexity

jdv on 2009-06-04T17:33:00

I never really understood the point of use_ok(). If a use fails it'll throw an exception. The use_ok() function doesn't seem to add any value to that. Someone please tell me if I'm missing something.

Write your own use_ok() but leave mine alone

brian_d_foy on 2009-06-04T18:09:54

I like to have use_ok() give me its answer then let me decide what to do with it. It always sucks to have side effects forced on you. You want to die (although you probably really want to bail_out), but maybe I want to do something else, like tell a pre-commit hook something, try to log an error message, or whatever. If you want to attach extra behaviour, make a use_ok_or_die().

I have use_ok() duplicated in several test files. That allows me to run just a single test file with those sanity checks. If you're talking about duplicating tests in the same file, that's probably a different story.

You don't want it to be use.t

Alias on 2009-06-05T03:09:00

Every module I own (and all my work projects) have this, but explicitly as the first test.

01_compile.t is responsible for compiling and loading absolutely everything in the distribution (including test support libraries), for running perl -c tests on all included scripts, and for things which loads module trees automatically, also for making sure that the $VERSION's of the modules all match.

And if any of this fails, it should do a BAIL_OUT.

All of this runs FIRST, so that all of the rest of the tests can trust implicitly that anything it loads will be ok. The other of the rest of the test scripts doesn't really matter THAT much.

But in my opinion it's critical that this 01_compile.t (often named as 01-load.t) script is the very first test that is run.