TAP subplan integrated
Michael G Schwern
branch. I stuck it in a branch on the general principle of keeping new
features separate to allow pushing a stable release out faster. My recent
experience having to tear INSTALLHTML out of MakeMaker is to blame.
The code looks excellent. Thank you Ovid and it looks like Adrian as well.
Now I will tear it apart.
I have two internal issues with the subplan branch.
Having to call finalize() on the child... it seems like it should just do that
when it gets destroyed. Fortunately this is shielded from the user by the
subtest() method. This bit of the docs is the red flag...
If the child falls out of scope before C<finalize> is called, a failure
diagnostic will be issued and the child is considered to have failed.
No attempt to call methods on a child after C<finalize> is called is
guaranteed to succeed.
[Special nit, I hate the throwback &foo syntax, modern Perl programmers
probably won't even know what it means, and much prefer foo() to indicate a
function call in the docs]
Which leads into issue #2: The child functionality feels like it should go
into a subclass (or role or whatever). Though the subplan code is decidedly
non-invasive so its quite open to argument.
Externally, I have only one nit. suite_passed() is a good idea, except...
$ perl -Ilib -wle 'use Test::More; pass(); print "Yes" if
Test::More->builder->suite_passed'
ok 1
Yes
# Tests were run but no plan was declared and done_testing() was not seen.
For anything but a finalized child object it is a lie. The fact that the
method name is past tense is the problem. You can only know if the suite has
passed after all the testing is done. Before that it can not be known and the
output is gibberish. What you can know is if the suite is *currently
passing*. That is, has anything happened up to this point to state that the
suite has failed? This is useful information at any time in the life of a
builder object and its an oft requested feature.
The work done for suite_passed() can easily be subverted for is_passing()
which I will do now.
--
170. Not allowed to "defect" to OPFOR during training missions.
-- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army
http://skippyslist.com/list/
Ovid
>
> Now that a stable release is out I've pulled in Ovid's fork as a subplan
> branch. I stuck it in a branch on the general principle of keeping new
> features separate to allow pushing a stable release out faster. My recent
> experience having to tear INSTALLHTML out of MakeMaker is to blame.
>
> The code looks excellent. Thank you Ovid and it looks like Adrian as well.
>
> Now I will tear it apart.
>
> I have two internal issues with the subplan branch.
>
> Having to call finalize() on the child... it seems like it should just do that
> when it gets destroyed. Fortunately this is shielded from the user by the
> subtest() method. This bit of the docs is the red flag...
I'm struggling to recall exactly why I used a finalize() method
instead of DESTROY. I believe it's Piers Cawley who argued for that,
but I can't be certain. That being said, I'm quite comfortable with
this and the other things you've said. Thanks for pushing forward on
this!
Cheers,
Ovid