Why no plan for nested TAP?

Ovid on 2009-11-17T11:24:13

At least one person has objected quite strongly to my patch for Test::Builder which adds an implicit done_testing() to subtests with no plan. I was kind of surprised with the vehemence of the discussion, so I want to clear this up.Consider the following test subtests:

subtest 'with plan' => sub {
    plan => 2;
    is $this, $that, 'this is that';
    is $foo,  $bar,  'foo is bar';
};

subtest 'without plan' => sub {
    is $this, $that, 'this is that';
    is $foo,  $bar,  'foo is bar';
};

Now imagine that this is a test suite you're currently working on and you keep adding tests to each. It will be annoying to keep updating the plan and sometimes get spurious test failures when your code is fine. The "without plan" subtest is much easier to use. If you want plans in your subtests, fine! Use them. There's nothing stopping you, but if you forget, I've added a safety net. Don't sweat it and have fun.

Unlike with the overall test program, you know exactly when that subtest ends. The plan buys you virtually nothing. In a subtest, instead of an annoyance which actually helps, it's an annoyance which hinders. Heck, for my top level tests, I'm thinking about writing a vim macro which automatically inserts this:

use Test::Most;  END { done_testing() }

Plans are almost not needed any more (this isn't entirely true, but for subtests, why bother?). I'm hard-pressed to believe that they now add enough value to overcome the annoyance of using them.


In agreement

mpeters on 2009-11-17T15:07:11

I hate "me too" style comments, but I in this case I have to say that I completely agree. Why force testers to count something that we can have the computer count. If you'd rather count, fine whip out your abacus and have fun. Otherwise, let us get on with useful stuff.

me too

markjugg on 2009-11-17T15:22:30

I also strongly support your proposed patch. For me, it could mean the difference between using and enjoying subtests, and not adopting them.

"Don't worry, be happy"

elliot on 2009-11-17T17:03:08

Regardless of what you think about counting tests, I think this is a significant change of the way that Test::More works. Up until now, everything has had to be explicit, whether through "tests", "no_plan", or done_testing().

Re:"Don't worry, be happy"

Ovid on 2009-11-17T17:23:54

Why is this a bad thing?

Re:"Don't worry, be happy"

Alias on 2009-11-18T01:43:18

Because being explicit allows you to be sure that the test hasn't stopped prematurely in ways less dramatic than crashing outright.

Re:"Don't worry, be happy"

Ovid on 2009-11-18T06:57:35

Sure, for the top level test. Any subtest without a plan or "no_plan" has done_testing() called for it. I did that to ensure that no subtest stopped prematurely in ways less dramatic than crashing outright. That being said, it won't stop someone from writing a subtest with a "return" statement prematurely terminating it, but then, "no_plan" won't stop that either. So if you want to be strict, add the plan. Otherwise, don't. Like every other form of testing in Perl, you'll probably get the results you want, but there are plenty of ways you can silently break your tests :)

Re:"Don't worry, be happy"

Aristotle on 2009-11-19T00:29:50

That being said, it won’t stop someone from writing a subtest with a return statement prematurely terminating it, but then, no_plan won’t stop that either.

That’s exactly the point. If done_testing isn’t safer than no_plan, then what’s the point of done_testing?

The idea behind done_testing is that because it is explicit, it makes a stronger assertion than any implicit assumption could be. To call done_testing implicitly is utterly redundant. It does not and cannot tell us anything we didn’t already know.

Re:"Don't worry, be happy"

Ovid on 2009-11-19T08:10:03

Two points: if we don't allow the implicit done_testing because we think it doesn't provide us much safety, then why are we allowing no_plan? It's hypocritical to allow one and not the other, no? (Oh, I know -- backwards compatability). Frankly, I find when I write subtests, I put many of them in a file and I don't want to keeps adding boilerplate over and over and I'd like to find a way to avoid that.

I have been, over the years, an explicit advocate of the plan and I think it's safer than no_plan or done_testing. That being said, I've realized that if I'm too strict (heh) in my sermonizing, the flock is going to get annoyed.

Re:"Don't worry, be happy"

Aristotle on 2009-11-19T09:40:38

Hypocrisy…? Nowhere did I say no_plan is bad nor did I talk about “allowing” implicit done_testing. I said the latter is pointless and redundant when we already have no_plan. If it’s bad, then that’s only because it might mislead people into thinking they’re safer than with no_plan.

Having a plan or not is a trade-off. It’s not about morals. If you don’t want to declare a plan, use no_plan. That’s what it’s for: to state your choice of trade-off clearly.

Re:"Don't worry, be happy"

Ovid on 2009-11-19T11:32:07

But if leaving off the no_plan gets you an implicit done_testing(), you don't need the boilerplate.

subtest 'one' => sub {
  plan 'no_plan';
  # bunch 'o tests
};

subtest 'two' => sub {
  plan 'no_plan';
  # bunch 'o tests
};

subtest 'three' => sub {
  plan 'no_plan';
  # bunch 'o tests
};

subtest 'four' => sub {
  plan 'no_plan';
  # bunch 'o tests
};

What's the point of that cut-n-paste code? I see no added value there. Most programmers that I know don't want to handle bookkeeping code, they want to program.

Re:"Don't worry, be happy"

Aristotle on 2009-11-19T11:48:25

Oh. For some reason I thought the 'with plan' and 'without plan' were syntax, not test names.

Hmm, that syntax seems to punish plans unnecessarily. How about requiring a plan to be declared up front something like this?

subtest { plan => 3 }, 'blah blah' => sub { ... };

Or maybe

subtest_noplan 'blah blah' => sub { ... };
subtest_plan 3, 'blah blah' => sub { ... };

Then it’s knowable whether there is a plan or not before invoking the sub, so the subtest can use no_plan instead of implicit done_testing. It would also make plan declaration less noisy, and thus not drive people toward omitting them out of sheer annoyance.

Re:"Don't worry, be happy"

Ovid on 2009-11-19T13:19:24

That sounds like a nice idea. The syntax is still a bit noisy, but still better than what's there. I'll give that a thought.

Don't cross the streams

Eric Wilhelm on 2009-11-18T02:16:10

The benefit of done_testing() is that you can be certain that "we got this far and ran this line of code."

But there's nothing like that in your subtest sub {}, so you can't be sure that there wasn't some a goto(), exit(), or return() which silently caused a bunch of tests to be skipped.

Similarly, END {done_testing()} does nothing that wasn't already accomplished by 'no_plan'. Calling done_testing() at the lexical end of your file is an assertion about control flow.

Thus, an implicit done_testing() on a 'without plan' subtest is redundant. But there are three modes here and your subtest() examples only show two: 1. plan, 2. no plan, 3. lexical plan (aka done_testing()). So:

    subtest sub {
        ok(1, 'whatever');
        done_testing()
    };

And why not 'with_plan'/'without_plan' so we can use the quoting of the fat arrow?

Re:Don't cross the streams

Ovid on 2009-11-18T07:10:32

Good points out the normal use of done_testing(). That was silly of me :)

As for the implicit version in subtests, you won't get that with "no_plan", either, something many people are going to use anyway, if they use tests at all (well, the "exit" isn't true, but your comments about "goto" and "return" are correct). Now I think you've made the clearest arguments about this, but your arguments apply to "no_plan", also. Would you argue we not have that? We do and it won't go away.

One argument in favor of having a number of tests asserted was that we it protects us from incorrect iteration counts. Many people who argued for disposing of plans altogether argued that you should just assert the number of iterations and not need a plan. This brings us full circle back to the whole "plan tests => $num" argument versus "no_plan". In this case, an implicit done_testing() is no worse than an explicit "no_plan" (and arguably better because it makes things easier to read), but we're going to see plenty of subtests with that because that's how people write tests. Is there a better way?

Note that you have similar issues to these in many xUnit frameworks but I don't hear people screaming about those :)