is_method_ok

[xenoterracide]

would you accept the addition of a test like

is_method_ok $obj, 'method', 'val' # ok 1 My::Class->method 

that also does a preliminary can check, so that it doesn't throw exceptions if not available.

I get really tired of writing

can_ok $obj, 'method'; is $obj->method, 'foo', 'My::Class->method';

and then to have the latter to cause the test to die because I didn't check the can on the method. (this is only rarely a problem, but still I think it should cause that ok to fail, not the test to die )

also if I provided a patch what branch and how long would it be before CPAN saw it (relatively speaking)

[schwern]

While I generally believe that a test should continue past failure, I'm dubious about adding a special function to allow a test to continue on after its found a method is missing. It's a pretty major problem, and one likely to cause further problems down the line.

In addition, this is a pretty simple method to code up yourself. There's not a compelling technical need for it to be in Test::More.

Another criterion is whether it provides more information than what it replaces? I don't think can_ok() says more than the normal exception does, but you might show otherwise.

Is it flexible enough to cover most of what it's supposed to replace? Or will we wind up with a spread of similarly named combined functions? In this case is_method_ok fails. Once you have something that combines can_ok + is, what about can_ok + like for when you can't say exactly what's going to be returned? can_ok + is_deeply, for methods which return complex data? can_ok

• cmp_ok for mathematical methods? This could be mitigated somewhat by making it use is_deeply, but that doesn't capture the full breadth of what one does with a method's return value.

In addition, the proposed interface has no provision to pass arguments into the method. Without that it's very special cased indeed. It would have to be like new_ok...

is_method_ok $obj, $method, \@args, $want, $name;

is equivalent to...

can_ok $obj, $method;
is $obj->method(@args), $want, $name;

Without needing to resolve if it's a good idea, I'd have to say no based on its lack of flexibility to cover the types of tests you want to do on a return value. If that were resolved then discussion could move forward.

Fortunately, it's very easy to implement what you want for your own use.

sub is_method_ok {
    my($obj, $method, $args, $want, $name);

    local $Test::Builder::Level = $Test::Builder::Level + 1;

    return unless can_ok $obj, $method, $name;
return is $obj->method(@$args), $want, $name;
}

also if I provided a patch what branch and how long would it be before CPAN saw it (relatively speaking)

I wouldn't put out a stable release just for this, though it's long overdue for one so you might win out. The devel version is still months out.

emacs -- THAT'S NO EDITOR... IT'S AN OPERATING SYSTEM!

[pdl]

Are you calling it in scalar, void, or list context? What you get out depends on this. You're calling it in scalar, but what if you wanted to get a list out?

[xenoterracide]

I agree with your proposed changes.

To be fair my main case is to simplify code like this, for which I have a few hundred tests (in one dist, and that many in many dists, ~300 simple tests in cybersource alone), none of which check that the object can first, meaning if I should have an error of missing method the exception would cause the test to bail, and I can't tell if further tests would succeed. (note: obviously I have some nested objects in this example, I would just do my $auth = $ret->auth first). If any one of these tests fails it does not prevent the next one from passing (unless it's $ret is undef).

is( $ret->is_accept,          1,                        'success'      );
is( $ret->decision,           'ACCEPT',                 'decision'     );
is( $ret->reason_code,         100,                     'reason_code'  );
is( $ret->currency,           'USD',                    'currency'     );
is( $ret->auth->amount,       '3000.00',                'amount'       );
is( $ret->auth->avs_code,     'Y',                      'avs_code'     );
is( $ret->auth->avs_code_raw, 'Y',                      'avs_code_raw' );
is( $ret->reason_text,        'Successful transaction', 'reason_text'  );
is( $ret->auth->auth_code,    '831000',                 'auth_code'    );

and how it should be written now ( does this even work? not sure what can_ok returns)

isa_ok my $auth = $ret->auth, 'Business::CyberSource::ResponsePart::AuthReply';

is( $ret->is_accept,          1,                        'success'      ) if can_ok( $ret, 'is_accept');
is( $ret->decision,           'ACCEPT',                 'decision'     )  if can_ok( $ret, 'decision');
is( $ret->reason_code,         100,                     'reason_code'  ) if can_ok( $ret, 'reason_code');
is( $ret->currency,           'USD',                    'currency'     )  if can_ok($ret, 'currency');
is( $auth->amount,       '3000.00',                'amount'       ) if can_ok( $auth, 'amount');
is( $auth->avs_code,     'Y',                      'avs_code'     ) if can_ok( $auth, 'avs_code';
is( $auth->avs_code_raw, 'Y',                      'avs_code_raw' ) if can_ok( $auth, 'avs_code_raw');
is( $ret->reason_text,        'Successful transaction', 'reason_text'  ) if can_ok( $ret, 'reason_text');
is( $ret->auth->auth_code,    '831000',                 'auth_code'    ) if can_ok  $auth, 'auth_code';

as you state it's entirely sugar, but it would seem to me that all can_ok is is sugar around ok $obj->can(...). I think that Test::More should make the most common uses cases incredibly easy. I personally would use this on my "accessors" and not on methods with side effects. What I mean to say is that the 'entire' breadth might not need to be covered.

However, the various combinations could be good, or perhaps there are even easier/more powerful designs that would solve all..

my %method_args = (
    foo          => [ 'bar', ],
    japanese => ['sashimi', 'hibachi'],
 );

my %want = (
     foo          => ['baz'],
     japanese => ['Japanese Express'],
 );

are_methods_deeply $obj, \%method_args, \%want, $name;
# ok 1 Class->foo('bar')
# ok 2 Class->japanese(...)

I'd be ok with having things like like_method( ... ), though in all fairness the more complex the need the less I'm concerned because i'm trying to remove repetitiveness in the simple cases. If that resulted in these functions method_ok, is_method, like_method, cmp_method, method_deeply or similar that'd be great, imo.

my $auth = method_isa_ok $ret, 'auth', undef, ['Business::CyberSource::RequestPart::AuthReply'];

method_ok  $ret,  'is_accept',    undef, [ 1 ];
method_is  $ret,  'decision' ,    undef, ['ACCEPT'];
method_is  $ret,  'reason_code',  undef, ['100'];
method_is  $ret,  'currency',     undef, ['USD' ];
method_is  $auth, 'amount',       undef, ['3000.00'],

here's could be a concept for list returns

# return list put in
method_is, $obj, 'sort', [qw( foo bar )], [qw( bar foo )];
# same as
# my $obj = Class->new;
if ( can_ok ($obj, 'sort' ) ) {
     my @sorted = $obj->sort( 'foo', 'bar' );
     my @expected = ( 'bar', 'foo' );
     is_deeply \@sorted, \@expected, ' Class->sort(...)'
}

my main objective is to stop writing repetitive, error prone, incomplete, and difficult to refactor ( due to laziness) code. keep in mind that in addition to the can check that the automatic name providing also helps. I'll reiterate that I don't think every possible method call/return must be supported.

[pdl]

method_is, $obj, 'sort', [qw( foo bar )], [qw( bar foo )];

What about a method which returns one or more arrayrefs? What about a method which returns a list in list context but something else in scalar context? And something else in void? The context must be either assumed by the method or passed as a parameter.

By the way, in the examples you give, I'd strongly recommend writing the execution code once and define a test script in an array of data, e.g.

my $script = [
  ['is_accept', 1, 'success', 'Documentation on why we run this test']
  [ 'decision', undef, 'ACCEPT', 'Documentation']
];
foreach @$script{
  can_ok($ret, $_->[0]);
  is($ret->$_->[0]($_[1]), $_[2], $_->[0].': '.$_[3]); # ok, I should have used names, but you see what I mean
}

[xenoterracide]

I can do the script later, as for void context I'd say the whole purpose of these is really to check a methods returns, if the method returns nothing then using something else is probably a better idea. (maybe using Test::Fatal to see if the exception returned is false).

As far as one or more arrayrefs, I'm not sure, though since you can tell an array ref in list it seems like the same behavior for is_deeply, which I ended up fleshing out (by updating the comment) more after I sent the post realizing I probably hadn't done that enough.

[schwern]

While I agree the most common things should be easy (or at least convenient) I've never seen anyone write code like that or wanted tests to continue in general after a method has gone missing. Again, it's a pretty major problem to have a method go missing. I'd need to see some solid evidence that there's a general need for this pattern.

In general, these sorts of problems usually boil down to packing too many tests into one file or having a costly setup/teardown process. This leads to individual test file runs taking too long, especially in tight debugging/development loops. You have to repeat unrelated stuff over and over again to get to what you really want which is slow and boring.

The simple solution in test suite well factored into short files, and one that will give a reliable test result, is to just fix the missing method and re-run the test. You're going to have to fix it and re-run the test no matter what. A single test file dying doesn't stop the test suite, so you get all the other results. This is similar to how there's a whole class of programming problems which are solved by having short, well factored functions and variable scopes.

can_ok() is a bit of an historical oddity and I don't know that it would pass muster today. It was added because I used to address your issue in a different way. I had tests which said can_ok "Some::Class", qw(big list of methods). I've since dropped that as busy work, it's never maintained properly, and it gave a false sense that the API was better tested than it really was. Now I just go ahead and test the method directly. If it blows up because it's missing, that's good.

As for new_ok(), Adam Kennedy bribed me by buying me a video game... so don't use that as an interface rationale. Though I am wanting a copy of Borderlands 2... j/k.

However, can_ok passed a crucial test in that it gives more information than a normal can call.

ok( $class->can($method) );
    not ok 1
    #   Failed test at -e line 1.

What class? What method?

can_ok $class, $method;
    not ok 1 - Foo->can('bar')
    #   Failed test 'Foo->can('bar')'
    #   at -e line 1.
    #     Foo->can('bar') failed

That said, method_blah() could fill that role. Like can_ok and cmp_ok it knows all the pieces of the method call.

is( $obj->method(@args), $want );
    not ok 1
    #   Failed test at -e line 1.
    #          got: '23'
    #     expected: '42'

method_is( $obj, $method, \@args, $want );
    not ok 1
    #   Failed test 'Some::Class=HASH(0x100806ce8)->foo('this', 'that')'
    #    at -e line 1.
    #          got: '23'
    #     expected: '42'

That's interesting. But what about functions? Now we need func_is/like/is_deeply...

When you start having to define O(N*M) functions, you start looking for better patterns. Like passing in the comparison function.

method_ok { like $_[0], $_[1], $_[2] } $obj, $method, \@args, $want;

But that's still a lot of typing. Since most test functions are of the form foo_ok( $have, $want, $name ) you can get away with this:

method_ok \&like, $obj, $method, \@args, $want;
func_ok \&like, $function, \@args, $want;

Now we're getting to something that might be a Test::More candidate. It adds extra diagnostics. It's general purpose. It doesn't need O(N*M) methods. But I'd still prefer it was knocked around in a CPAN module before being enshrined in Test::More.

Even then I'd still say a missing method is an exception. There's a qualitative difference in severity and expectation between "I got the wrong result" and "the method doesn't exist". Similarly, passing in a non-object would be an exception as would the method executing but throwing an exception. Either it catches every exception or it catches none. I also wouldn't want to tie together the improved diagnostics with a presumption that certain exceptions get caught.

PS Everything passed to most Test::More functions are in scalar context, so this isn't going to be any worse in that respect.

PPS new_ok could also output these extra diagnostics.

[xenoterracide]

yeah, I'm ok with most of that. I don't know that the can check is necessary, it just seems like a nice convenience so that other tests can continue, which could end up telling you to fix multiple at once. You're right that this is largely because I have expensive to setup tests. I have to test various SOAP request that return different results, and make sure I get the results from my accessors that I expect. I don't actually see a way around this. I've already significantly reduced the footprint of the test setup, but i still need to know the response has each value returned as specified by the SOAP API or how I translated the SOAP API to something perlish, (I've had to fix 2 coercions twice before I learned the lesson of not checking (BigInt -> string ) and ( NetAddr::IP -> string ))

[xenoterracide]

I started an implementation here: https://github.com/xenoterracide/Test-Subroutine

one problem/concern I'm having with using comparators is it appears they will have to provide the "ok" themselves? otherwise we end up with multiple oks? or is the latter preferred.

I'm also allowing for a default comparator of &is, to allow for shorter calls if you're looking for what I feel is the most common and most simple case. But not sure how you'd feel about that.

feedback welcome

[xenoterracide]

not sure how one could pass the function name with a string... we could test the function by passing it as a code ref too, maybe

 func_is \&like, \&function, $func_name, \@args, $want, [$name];

but this is not as useful diagnostics wise, maybe

func_is [ &like ], $package, $func, \@args, $want, [$name];

[schwern]

I'm going to close this up and point future discussion at https://github.com/xenoterracide/Test-Method