Re: "plan skip_all => $reason" is no plan?

[Adriano Ferreira]

On 4/24/07, Ovid <[EMAIL PROTECTED]> wrote:

--- Adriano Ferreira <[EMAIL PROTECTED]> wrote:

> While fussing around with some testing code, I've found that this
> script:
>
>     #!/usr/bin/perl
>
>     use Test::More;
>
>     my $Test = Test::Builder->new;
>
>     plan skip_all => "I'm leaving NOW!";
>
>     END { print "Bye" unless $Test->has_plan }
>
> produces the ouput
>
>     $ perl t-builder.t
>     1..0 # Skip I'm leaving NOW!
>     Bye

That looks like a bug.  We clearly have a plan.

When I say

    plan skip_all => "I'm leaving NOW!";

with the testing API, it really suggests/states that 'skip_all' is a
plan. But is kind of special as Ovid and Adrian said.

> I would really expect that "plan skip_all => $reason" was meant to do
> "Test::Builder->new->has_plan" to return a true value. According to
> the documentation, it is not the case:
>
>     has_plan
>
>     Find out whether a plan has been defined. $plan is either undef
>     (no plan has been set), no_plan (indeterminate # of tests)
>     or an integer (the number of expected tests).

My reading of that with your code suggests that '0' should be returned.
 However, that means that if $plan is 0, then has_plan will be false,
which clearly isn't the case.  ->has_plan should return a boolean value
and ->plan should take over the ->has_plan behavior.  However, we still
have a bug:

Would it be acceptable an extension to ->has_plan which maintained the
current behavior for $builder->has_plan() invocations and added a
behavior if an explicit argument was given? Something that would
require a documentation change like this:

    $plan = $builder->has_plan;
    $plan = $builder->has_plan('any');

    Find out whether a plan has been defined. $plan is either undef
    (no plan has been set), no_plan (indeterminate # of tests)
    or an integer (the number of expected tests). Skip_all plans
    are handled as if no plan was set (unless an explicit argument
    'any' was given, in which case 'skip_all' is returned).

Argh, that's not well phrased. But I think I made my point.

  #!/usr/bin/perl

  use Data::Dumper;
  use Test::More;

  my $Test = Test::Builder->new;
  plan skip_all => "I'm leaving NOW!";

  END {
      print Dumper( [ $Test->has_plan ] );
  }

That prints:

  1..0 # Skip I'm leaving NOW!
  $VAR1 = [
            undef
          ];

Cheers,
Ovid

--

Buy the book -- http://www.oreilly.com/catalog/perlhks/
Perl and CGI -- http://users.easystreet.com/ovid/cgi_course/

A tentative patch follows. How do I write a test for this? After
calling "plan skip_all => $reason", I cannot make tests anymore on the
Test::Builder instance. Should I use two Test::Builder instances to
make a test for such thing?

diff -u -r -N Test-Simple-0.70/lib/Test/Builder.pm
Test-Simple/lib/Test/Builder.pm
--- Test-Simple-0.70/lib/Test/Builder.pm        2007-03-15 19:57:10.000000000 
-0300
+++ Test-Simple/lib/Test/Builder.pm     2007-04-24 10:18:46.000000000 -0300
@@ -323,17 +323,26 @@

=item B<has_plan>

-  $plan = $Test->has_plan
+  $plan = $Test->has_plan;
+  $plan = $Test->has_plan('any');

-Find out whether a plan has been defined. $plan is either C<undef>
(no plan has been set), C<no_plan> (indeterminate # of tests) or an
integer (the number of expected tests).
+
+Find out whether a plan has been defined. $plan is either C<undef>
+(no plan has been set), C<no_plan> (indeterminate # of tests),
+or an integer (the number of expected tests).
+Skip_all plans are handled as if no plan was set (unless
+an explicit argument C<any> was given, in which case
+C<skip_all> is returned).

=cut

sub has_plan {
    my $self = shift;
+    my $any_plan = $_[0] ? $_[0] eq 'any' : '';

    return($self->{Expected_Tests}) if $self->{Expected_Tests};
    return('no_plan') if $self->{No_Plan};
+    return('skip_all') if $self->{Skip_All} && $any_plan;
    return(undef);
};

test-builder.diff
Description: Binary data