An easy way to keep your number of tests up to date is using BEGIN blocks:

use Test::More;
use vars qw($n_tests);

use_ok 'Foo::bar';
BEGIN { $n_tests++ }

my $c = 10;
for (1..$c) {
   ok Foo::new->($_);
}
# BEGIN { $n_tests += $c } # oops, wrong. my $c not visible here
BEGIN { $n_tests += 10 }

# ... and so on
# at last:

BEGIN { plan tests => $n_tests }
[download]

Randomly moving logic into BEGIN blocks is problematic, and you accidentally demonstrated that there.

That code will give a plan of 1 test when you probably wanted 11.

Randomly moving logic into BEGIN blocks

Well, its not random: I just set a variable, and nothing wrong with that, except...

That code will give a plan of 1 test when you probably wanted 11.

...the my variable gotcha, where $c is not visible in the BEGIN block. Thanks, corrected.

Which may be useful, but isn't the same thing as what this module is trying to do: To check if you've actually done something like that or just chickened out.

Looks interesting; I'll try it out. I would like if some of the dependencies were optional... ('Test::PerlTidy' is really more of an author-only test, for instance.)

But it looks like something that might usefully be added to a Kwalitee score to me, eventually.

Edit:

One thought from looking at it: I'd want all_plans_ok() exported by default. It's kinda the point of useing the module...

Also, the message Failed test ''t/002_basic.t' does not have 'no_plan' set': You mean it has no_plan set. Not that it doesn't.

One thought from looking at it: I'd want all_plans_ok() exported by default. It's kinda the point of useing the module...

I had done that originally but Test::PerlCritic complained. Ill amend it and change the test.

Thanks for the comments.

Duncs

If it were my module, I would not export anything by default. Default exports are rather evil. When somebody comes along and reads the code:

    all_plans_okay();
[download]

how do they find where this subroutine is defined? Well, that becomes obvious when the code above it is:

    use Test::NoPlan qw( all_plans_okay );
[download]

I also find the module name implies nearly the opposite of what it does. I would rename it Test::AllPlans or similar.

I'd also never use the module since I have no problems just settings the 'plan' to some arbitrary number when I first start writing it and then fixing it as a final step. I never do a temporary "no plan". (It took me a while to realize what the motivation for the module was.)

But thanks for contributing this. I hope you find the feedback useful despite its shortcomings. :)

If it were my module, I would not export anything by default.

I have tried to write the module according to PBP (trying to improve my coding skills) and I just reread the section of it about exporting which suggests that since this one method will always be used by the programmer it should be exported by default. Also, since the test file it is in may only be a few lines long it seems reasonable for programmers to sumise where the method came from, so I think it may be better to export the one routine by default.

I also find the module name implies nearly the opposite of what it does. I would rename it Test::AllPlans or similar.

Until you pointed this out I didn't realise the name was a bit ambiguous - in my mind the name is 'Test-for-no-plan' which seemed to fit what it was about best - 'test-all-plans' implies to me that it does more than just checking to see if 'no_plan' has been used.

Its one of those that I can see both sides of the argument, so in the long run I am not sure which might be best.

I hope you find the feedback useful despite its shortcomings. :)

This is the first module I have written from scratch for use on CPAN so if this is the 'worst' of the feedback I am actually quite happy about the points raised so far - after all, it is about improving my own coding skills rather than just throwing something on CPAN :)

Duncs