On Sat, Sep 16, 2006 at 01:31:28AM -0700, Ovid wrote:
> The following line is giving me pause:
>
> ok 9 Elegy 9B # TOdO
>
> That's an 'unexpectedly succeeded' test ('bonus', in the Test::Harness
> world).
>
> Right now, if that read 'not ok # TODO', TAPx::Parser would have this:
>
> passed true
> actual_passed false
> todo_failed false
>
> But since it reads 'ok', I've reversed the sense:
>
> passed false
> actual_passed true
> todo_failed true
>
> In this case, Test::Harness and friends report that 'ok 9 # todo' is
> passing, not failing, but I'm reporting the opposite result. I think
> my behavior is more correct because I'm trying to write things so that
> someone who forgets writes a bad harness will still see what's going
> on. For example, let's say someone only wants to see failing tests:
>
> # the 'source' key is new. You no longer have to manually create a stream
> my $parser = TAPx::Parser->new( { source => $test_file } );
> while ( my $result = $parser->next ) {
> print $result->as_string if ! $parser->passed;
> }
>
> With that, they'll never notice that 'ok # TODO' has unexpectedly
> succeeded if I adopt the current behavior of Test::Harness unless they
> explicitly remember to check the $parser->todo_failed method.
I think this is partly a question of semantics. You have two bits of
information: whether the test printed "ok" or "not ok", and whether the
test is tagged "TODO".
The question then becomes should "passed" mean "printed 'ok'" or should
it mean "printed what the author was hoping for"? Presumably then,
"actual_passed" is the same as "passed" unless it is a TODO test, in
which case it is the opposite. Similarly, you could argue over whether
"todo_failed" means it's a TODO and the output was "ok", or it's a TODO
and the output was "not ok".
I have a similar problem with Devel::Cover where code is tagged as
"uncoverable", meaning that the developer asserts that this code should
never be executed.
> I propose that 'ok # TODO' tests be reported as failures. I think it
> would be good if Test::Harness also adopted this strategy because
> right now, if you see that tests unexpectedly succeeded, you don't
> know which tests they are and you have to try and grep through the TAP
> output manually.
I agree that passing TODO tests should be treated the same way as
failing "normal" tests.
> Thoughts? Is this going to break a lot of stuff (I suspect it might).
I have a suggestion based on what I did with Devel::Cover. For
something simple like statement coverage I have "covered" (the analogue
of "ok") and "uncoverable" (the analogue of "todo").
In the interface to the coverage results I make these two basic items
available, along with an "error" method, telling whether the whole
construct is something that needs to be looked at. So, for statement
coverage I have:
sub covered { $_[0][0] }
sub uncoverable { $_[0][1] }
sub error { $_[0][0] xor !$_[0][1] }
plus a couple of other methods. The whole thing is at
http://search.cpan.org/src/PJCJ/Devel-Cover-0.59/lib/Devel/Cover/Statement.pm
Things are a little more complicated for other criteria, but the idea is
the same.
So, I would suggest leaving things as they are if possible (or not, I'm
fairly ambivalent), but deprecating that API in favour of:
sub ok { } # did it print "ok" or "not ok"
sub todo { } # is it marked as TODO
sub error { ok xor !todo }
and gradually moving tools across to the new API. I think this has the
advantage that is is reasonably clear what each of the methods means.
Since these are all boolean methods, some people might prefer that they all be
prefixed with "is_".
Since there are only two bits of information, only two boolean methods are
strictly required, but I think providing (at least) three is helpful to save
people messing about with xors. (I think I have the logic correct above.)
I think we have to expect a certain level of competence from people who will
be writing code to use this interface, but obviously the easier it is to write
good code, and the harder it is to write bad code, the better.
--
Paul Johnson - [EMAIL PROTECTED]
http://www.pjcj.net
