is( $foo, $bar )->todo($reason);

[Michael G Schwern]

I just once again wrote something like this:

TODO: {
local $TODO = $reason;

is( $foo, $bar );
}

Which is an awful lot of scaffolding to declare just one test TODO.

With Test::Builder2 functions returning result objects rather than just
true/false, and the ability to continue to build up the result object after
the test has already been run, the above can reduce to this:

is( $foo, $bar )->todo($reason);

--
Defender of Lexical Encapsulation

[benh]

I can see that being useful for one test but it seems heavy as a
replacement for entire blocks. Are you thinking that test would just
DWIM when ever it shows up in a stream, thus emulating a series of
tests as a block?

is(1,2)->todo($reason)->is(4,5)->ok($obj->method);

Or are you thinking of keeping both syntaxes for either case?

is(1,2)->todo($reason);
TODO: {
...
}

--
benh~

http://three.sentenc.es/

[Gaurav Vaidya]

2009/3/30 benh <ben.h...@gmail.com>:

>
> I can see that being useful for one test but it seems heavy as a
> replacement for entire blocks. Are you thinking that test would just
> DWIM when ever it shows up in a stream, thus emulating a series of
> tests as a block?
>
> is(1,2)->todo($reason)->is(4,5)->ok($obj->method);

It's not as blatant as local $TODO = "...", but it should be able to
handle blocks quite neatly. But we'll probably need some syntax to
indicate where todo's influence ends:
plan(3)
->is(12, 3)
->todo($reason)
->is(15, 2)
->clear() # Not neat
->isnt(2, 2);

Alternatively, todo could mark result objects from another stream
entirely as TODO before returning it into the plan stream:
plan(4)
->is(12, 3)
->todo($reason, is(15, 2)->is(15, 6))
->isnt(2, 2);

This allows us to do crazy things like:
plan(2)
->is(12, 3)
->todo($reason, subplan(2)
->is(12, 2)
->isnt(13, 4)
)->is(14, 3);

which mirrors the final TAP output quite nicely.

cheers,
Gaurav

[Michael G Schwern]

benh wrote:
> I can see that being useful for one test but it seems heavy as a
> replacement for entire blocks. Are you thinking that test would just
> DWIM when ever it shows up in a stream, thus emulating a series of
> tests as a block?
>
> is(1,2)->todo($reason)->is(4,5)->ok($obj->method);
>
> Or are you thinking of keeping both syntaxes for either case?
>
> is(1,2)->todo($reason);
> TODO: {
> ...
> }

Test::More keeps the TODO block thing.

Everything gets the ok()->todo() syntax for free by virtue of that test
functions simply return a Result object (actually a ResultWrapper but that's
another detail).

--
Robrt: People can't win
Schwern: No, but they can riot after the game.

[Michael G Schwern]

Gaurav Vaidya wrote:
> It's not as blatant as local $TODO = "...", but it should be able to
> handle blocks quite neatly. But we'll probably need some syntax to
> indicate where todo's influence ends:
> plan(3)
> ->is(12, 3)
> ->todo($reason)
> ->is(15, 2)
> ->clear() # Not neat
> ->isnt(2, 2);

And where exactly in all that iron-bound object chain does the real code
you're testing go? (Eagerly awaiting further nested chained method call
contortions or something really brilliant).

A bit of history. Part of the reason for the TODO block was that it avoids
the problem of having to attach a special "todo" parameter onto every
function, many of which have variable numbers of arguments. Or the
alternative, which Test.pm does, is to have a single todo() function that
tries to do everything. [1]

Since we can just tack method calls onto Result objects to modify them there's
no longer that need. If you want to coordinate the reason, just stick it in a
variable (which is what you're doing right now anyway).

So here's a TODO test from DBIx::Class.

TODO: {
local $TODO = 'bind args order needs fixing (semifor)';

# First, the simple cases...
$rs = $schema->resultset('Artist')->search(
{ artistid => 1 },
$where_bind,
);

is ( $rs->count, 1, 'where/bind combined' );

$rs= $schema->resultset('Artist')->search({}, $where_bind)
->search({ artistid => 1});

is ( $rs->count, 1, 'where/bind first' );

$rs = $schema->resultset('Artist')->search({ artistid => 1})
->search({}, $where_bind);

is ( $rs->count, 1, 'where/bind last' );
}

And here's what it would look like using todo().

TODO: {
my $reason = 'bind args order needs fixing (semifor)';

# First, the simple cases...
$rs = $schema->resultset('Artist')->search(
{ artistid => 1 },
$where_bind,
);

is ( $rs->count, 1, 'where/bind combined' )->todo($reason);

$rs= $schema->resultset('Artist')->search({}, $where_bind)
->search({ artistid => 1});

is ( $rs->count, 1, 'where/bind first' )->todo($reason);

$rs = $schema->resultset('Artist')->search({ artistid => 1})
->search({}, $where_bind);

is ( $rs->count, 1, 'where/bind last' )->todo($reason);
}

Its basically exactly the same but allows the wonky $TODO logic to go away.

Which is to say, I don't think there's any pressing reason to have any special
TODO block syntax any more.

But if you really want some sort of chained OO thing you can do it. I'm
really striving to keep Test::More-isms out of the base Test::Builder2 object.

[1] The other is so that you can add/remove the TODO status without altering
the wrapped test code, but I'm willing to give on that one.

--
185. My name is not a killing word.
-- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army
http://skippyslist.com/list/

[Gaurav Vaidya]

On Mar 31, 2009, at 12:16 AM, Michael G Schwern wrote:

> Gaurav Vaidya wrote:
>>
>> plan(3)
>> ->is(12, 3)
>> ->todo($reason)
>> ->is(15, 2)
>> ->clear() # Not neat
>> ->isnt(2, 2);
>
> And where exactly in all that iron-bound object chain does the real
> code
> you're testing go? (Eagerly awaiting further nested chained method
> call
> contortions or something really brilliant).

Haha, sure, one nested chained method call contortion coming right up.

use ...;
my $dbh; # Not good: variables should be near where they are used.

subplan(3, "Checking db connectivity") #
A result object with a plan for two tests, $r, is created
->test( "Connecting to the database", sub { #
and inserted into this scope
$dbh = connect_db($username, $password);
is($dbh,
'Database::Handle'); # so main::is()
can call $r->is(@_), which appends a
}) # single
result onto $r.
->TODO("Getting a list of users", sub
{ # TODO() works exactly like test(), but $r's
TODO flag is set,

subplan
(3)
# main::subplan() calls $r->subplan(),
->test("Sending the DB a query", sub
{ # and future tests() will add '#TODO' to the TAP output
... # automatically
.
})
->test("Checking for a response", \&response_check)
->test("Response clean", \&response_test);
})
;

done_testing();

... but I imagine it would make for horribly complicated code to make
all of this work correctly. It's also way too high level to go into
Test::Builder 2. The benefits are nice TODOs without using local, and
the ability to have multiple TAP producers working together (since
producer-specific stuff can go into the $r object and the tree of
subplans and tests it carries around).

> TODO: {
> my $reason = 'bind args order needs fixing (semifor)';
>
> # First, the simple cases...
> $rs = $schema->resultset('Artist')->search(
> { artistid => 1 },
> $where_bind,
> );
>
> is ( $rs->count, 1, 'where/bind combined' )->todo($reason);
>
> $rs= $schema->resultset('Artist')->search({}, $where_bind)
> ->search({ artistid => 1});
>
> is ( $rs->count, 1, 'where/bind first' )->todo($reason);
>
> $rs = $schema->resultset('Artist')->search({ artistid => 1})
> ->search({}, $where_bind);
>
> is ( $rs->count, 1, 'where/bind last' )->todo($reason);
> }

My worry with this is that while the "TODO:" lets you know what's
going on, the ->todo()s which actually change things are buried away
at the end of the statements. So a developer writing tests could
remove some or all of the ->todo()s without changing the "TODO:/
$reason" upfront, confusing anybody who sees it. It's just like
comments and code going out of sync. Yes, it's a fairly silly thing to
protect users from - especially since, as this is going into T::B,
users of Test::More might never see it at all - but this is a
protection which isn't needed in Test::Builder 1, since removing the
"local $TODO = ..." line removes both the reason and the #TODOs in the
TAP output.

> But if you really want some sort of chained OO thing you can do it.
> I'm
> really striving to keep Test::More-isms out of the base
> Test::Builder2 object.

++, keeping T::B2 clean and simple is definitely important. Any major
issues can be abstracted away (to some extent) in T::M and others. I
hadn't thought about that.

> [1] The other is so that you can add/remove the TODO status without
> altering
> the wrapped test code, but I'm willing to give on that one.

Isn't that helpful (or atleast really handy)? The new ->todo() breaks
that, too, but once again I don't have a good alternative.

Besides, you can't make eggs without breaking omelettes, so while I'm
hoping as many of T::B1's best features stay around, many of them will
have to be axed to bring us all that T::B2 goodness. I'm just arguing
for my favourite eggs here, but I know most of them are toast anyway :).

cheers,
Gaurav

[Michael G Schwern]

I think the mangled mess above points out the major flaw: nesting drives
everything to the right and horizontal space is still at a premium in most
coding styles.

> ... but I imagine it would make for horribly complicated code to make
> all of this work correctly. It's also way too high level to go into
> Test::Builder 2.

Not necessarily. The implementation would probably be pretty straight
forward. As for it being too high level I have a way to address that,
essentially splitting TB2 into an absolutely minimum version and one that
contains all the convenience stuff. Something I observed while writing TB2 is
that most of the complexity of TB1 is in the convenience functions.

More on that later.

>> TODO: {
>> my $reason = 'bind args order needs fixing (semifor)';
>>
>> # First, the simple cases...
>> $rs = $schema->resultset('Artist')->search(
>> { artistid => 1 },
>> $where_bind,
>> );
>>
>> is ( $rs->count, 1, 'where/bind combined' )->todo($reason);
>>
>> $rs= $schema->resultset('Artist')->search({}, $where_bind)
>> ->search({ artistid => 1});
>>
>> is ( $rs->count, 1, 'where/bind first' )->todo($reason);
>>
>> $rs = $schema->resultset('Artist')->search({ artistid => 1})
>> ->search({}, $where_bind);
>>
>> is ( $rs->count, 1, 'where/bind last' )->todo($reason);
>> }
> My worry with this is that while the "TODO:" lets you know what's going
> on, the ->todo()s which actually change things are buried away at the
> end of the statements. So a developer writing tests could remove some or

> all of the ->todo()s without changing the "TODO:/$reason" upfront,

> confusing anybody who sees it. It's just like comments and code going
> out of sync. Yes, it's a fairly silly thing to protect users from -
> especially since, as this is going into T::B, users of Test::More might
> never see it at all - but this is a protection which isn't needed in
> Test::Builder 1, since removing the "local $TODO = ..." line removes
> both the reason and the #TODOs in the TAP output.

Yep, the concern is valid. I'm quite happy with giving users both ways and
let them figure it out.

>> [1] The other is so that you can add/remove the TODO status without
>> altering
>> the wrapped test code, but I'm willing to give on that one.
>
> Isn't that helpful (or atleast really handy)? The new ->todo() breaks
> that, too, but once again I don't have a good alternative.

Yes, it is helpful, but it comes at a cost of the ugly local $TODO stuff.

--
Hating the web since 1994.