All the Perl that's Practical to Extract and Report

Guess Who Loses: Test::More::subtest versus Test::XML

Ovid on 2009-06-29T14:00:00

I've found that one of the best ways to try new testing ideas is to run them against our test suite at work. We've over 30,000 tests at this point, with several test harnesses integrating together, along with two fundamentally different test systems. This means that when I throw something at this test suite, I often shake out bugs. My new Test::Aggregate::Nested combined with our test suite managed to find a rather serious issue with the new subtest function in Test::More. The following, for example, fails:

    use Test::More tests => 2;
    use Test::XML;

    ok 1;
    subtest 'FAIL!' => sub {
        plan tests => 1;
        is_xml '', '', 'Singleton fail';
    };
    __END__
    xml.t ..
    1..2
    ok 1
        1..1
    Cannot run test (Singleton fail) with active children at /home/ovid/pips_dev/work/Pips3/branches/rights_modeling/deps/lib/perl5/Test/XML.pm line 57.
        # Child (FAIL!) exited without calling finalize()

There's nothing wrong with the code as it's written, but Test::XML knows that the Test::Builder object is a singleton, so there's a false optimization in the code. Near the top of the package, you see this line, defined outside of all functions:

    my $Test = Test::Builder->new;

If every Test::XML function simply had that line in the function rather than trying to share this across all test functions, the subtest code would work fine. Instead, the author knew he had a singleton and there's no point in re-instantiating, is there?

To be fair, I've done the same thing before (see Test::JSON), even though I knew it might be a bad idea. Heck, lots and lots of testing libraries have this issue. Now we need to figure out how to deal with this problem or else subtests aren't going to play well with a lot of code. Damn.

Ooops

Dom2 on 2009-06-30T06:33:25

I plead guilty. It was a severe case of cargo-cult coding at the time. I'll try and roll a new release for you, as soon as I can.

Re:Ooops

Ovid on 2009-06-30T07:44:19

That would be lovely, but don't feel too bad. I just released a new version of Test::JSON last night to fix the same issue :) There are lots and lots of test modules which have this issue and it's such a common anti-pattern that it's tough to blame anyone. Plus, at the time you wrote Test::XML, Test::Builder was still a singleton. How were you to know that the interface would have to change? (Er, or a subtle change in behavior, actually).

When I get some free time, I'm going to patch up Test::Builder to get around this issue. Meanwhile, "best practices" for test module authors should probably be published somewhere.

Re:Ooops

Dom2 on 2009-07-02T20:54:02

Sorry for the delay, but Test::XML 0.08 is now up, which fixes this issue. If you have any further issues, please let me know!

Re:Ooops

Ovid on 2009-07-03T08:28:58

Given that we use your module quite heavily hear at work, I appreciate this! Thanks :)