use_ok isn't ok

Ovid on 2009-11-08T08:57:40

Update: I'm so annoyed by this anti-pattern that it appears I've griped about it before.

No matter how many times I write use_ok $some_module, I hate it. I hate it because this is wrong:

use_ok 'My::Module';

There are several issues with that. First, you really should wrap it in a BEGIN block. This can ensure, amongst other things, that prototypes are respected. So you need to do this:

BEGIN {
    use_ok 'My::Module';
}

Of course, that's wrong, too. I remember one very long, painful debugging day where I and another programmer paired to find out why tests were failing for our perfectly valid code. Every minimal test case showed the code worked, but we had multiple failures when integrated. What happened?

This was several years ago and if we had use Test::Most qw(no_plan die); available, the problem would have immediately shown itself. Otherwise, I needed to go back and do this:

BEGIN {
    use_ok 'My::Module' or die;
}

You see, internally, use_ok will eval something like "use My::Module" and if it fails, the test fails. What happens when it fails? Three guesses as to what's left in the symbol tables. That's right, you may very well have partial optrees sitting around in memory and when your code tries to run them, BOOM! I wonder what sort of interesting side-effects we could get there? Even the SYNOPSIS for Test::More gets this wrong:

BEGIN { use_ok( 'Some::Module' ); }

The programmer and I wasted hours trying to debug perfectly fine code because I didn't see the use_ok had failed at the top and the failures scrolled off the screen.

But even with the "or die", it's wrong, because when I look at test suites, I see the following:

# in t/00-load.t:
BEGIN {
    use_ok 'My::Module' or die;
}

# in t/basic.t:
BEGIN {
    use_ok 'My::Module' or die;
}

# in t/exceptions.t:
BEGIN {
    use_ok 'My::Module' or die;
}

# in t/regressions.t:
BEGIN {
    use_ok 'My::Module' or die;
}

# in t/advanced.t:
BEGIN {
    use_ok 'My::Module' or die;
}

That's a lot of boilerplate repetition. And that's assuming that it's done correctly with the BEGIN and or die. Why? Just for an extra test?

I see no advantage here, but I see plenty of disadvantages. Just put your use_ok tests (if you want them) in t/00-load.t and have that possibly bail out one doesn't load.

#!/usr/bin/env perl

use strict;
use warnings;

use Test::Most 'bail';

BEGIN {
    my @modules = get_modules();
    plan tests => scalar @modules;

    use_ok $_ foreach @modules;
}

And in your other tests:

use My::Module;

If that fails, it will fail correctly without you having to remember the boilerplate.


Wow awesome

Ether on 2009-11-08T17:07:40

I registered here just to say thank you. I've been long irritated by Test::More's default behaviour, and just last week was writing up a patch to add bail_on_failure() behaviour. Now I don't have to; I can just use Test::Most qw(die). Thank you so much! Tomorrow one company is going to see a revision in all its test suites... :D

PS. I recently learned about Devel::BeginLift (thanks jrockway@SO), which allows methods to be marked as running at compile-time.. It would be *really really* valuable to do this for Test::Most::use_ok.

Re:Wow awesome

Ovid on 2009-11-08T18:43:37

Glad to hear you like Test::Most. We use it extensively on our BBC project.

As for the Devel::BeginLift idea, that's a tempting thought. My only concern is that this is introducing a module which has a high failure install rate and changes the semantics of the module. We'd still have to have the or die behavior, but I'd rather use have use My::Module at the top of the code, I think.