Merge pull request #210 from gempesaw/fix-209

Fixes #209

lib/Test/Selenium/Remote/Driver.pm

@@ -33,7 +33,8 @@ has func_list => (
             'send_modifier_ok', 'accept_alert_ok', 'dismiss_alert_ok',
             'get_ok', 'go_back_ok', 'go_forward_ok', 'add_cookie_ok',
             'get_page_source_ok', 'find_element_ok', 'find_elements_ok',
-            'find_child_element_ok', 'find_child_elements_ok', 'find_no_element_ok',
+            'find_child_element_ok', 'find_child_elements_ok',
+            'find_no_element_ok',
             'compare_elements_ok', 'click_ok', 'double_click_ok',
             'body_like',
         ];
@@ -44,14 +45,14 @@ sub has_args {
     my $self          = shift;
     my $fun_name      = shift;
     my $hash_fun_args = {
-        'find_element'     => 2,
-        'find_no_element' => 2,
-        'find_child_element'     => 3,
-        'find_child_elements'     => 3,
-        'find_element'     => 2,
-        'find_elements'     => 2,
-        'compare_elements' => 2,
-        'get' => 1,
+        'find_element'        => 2,
+        'find_no_element'     => 2,
+        'find_child_element'  => 3,
+        'find_child_elements' => 3,
+        'find_element'        => 2,
+        'find_elements'       => 2,
+        'compare_elements'    => 2,
+        'get'                 => 1,
     };
     return ( $hash_fun_args->{$fun_name} // 0 );
 }
@@ -62,13 +63,6 @@ has verbose => (
     is => 'rw',
 );
 
-has error_callback => (
-    is      => 'rw',
-    default => sub {
-        sub { }
-    },
-);
-
 
 sub BUILD {
     my $self = shift;
@@ -158,6 +152,56 @@ sub server_is_running {
 
 }
 
+=head2 error_handler
+
+As for L<Selenium::Remote::Driver>, this class also supports adding an
+optional C<error_handler> attribute during instantiation :
+
+    my $test_driver = Test::Selenium::Remote::Driver->new(
+        error_handler => sub { print $_[1]; croak 'goodbye'; }
+    );
+
+Additionally, you can set and/or clear it at any time on an
+already-instantiated driver:
+
+    # later, change the error handler to something else
+    $driver->error_handler( sub { print $_[1]; croak 'hello'; } );
+
+    # stop handling errors manually and use the default S:R:D behavior
+    # (we will croak about the exception)
+    $driver->clear_error_handler;
+
+Your error handler will receive two arguments,
+The first argument is the C<$driver> object itself.
+Due to some specificities of this class, the second argument passed to the
+handler can be: 
+
+=over
+
+=item the error message from the Webdriver 
+
+This is the case when the error message is raised by a WebDriver failure
+
+=item "Failed to find ..."
+
+This message is raised when the Webdriver call is successful but the failure
+occurs on the test performed aftwerwards. This is the case for functions like
+C<body_text_like>, C<body_text_unlike>, C<body_text_contains>, C<body_text_lacks>,
+C<content_like>, C<content_unlike>, C<content_contains>, C<content_lacks>.
+
+=back
+
+If you set your own handler, you should not rely that much on the message returned. 
+You should also remember that you are entirely responsible for handling exceptions, 
+which means that should the error handler be called, it means that the test you are
+doing has failed, so you should croak. 
+
+You should also call fail() in your handler, in case the function called raised a 
+webdriver error, because, as exceptions are not caught anymore when you specify a 
+handler, the function will not fail anymore, which translates to a 'ok' in your TAP
+output if you do not handle it properly.
+
+
 =head1 Testing Methods
 
 The following testing methods are available. For
@@ -239,46 +283,53 @@ more documentation, see the related test methods in L<Selenium::Remote::Driver>
 
     click_ok
     double_click_ok
+
 =cut
 
 
 # function composing a find_element with locator with a webelement test
 
-sub _find_element_with_action { 
-    my $self = shift; 
+sub _find_element_with_action {
+    my $self   = shift;
     my $method = shift;
-    my ($locator,$locator_strategy,$params,$desc) = @_;
-    # case 4 args 
-    if ($desc) { 
-        $self->croak('Invalid locator strategy') unless ($self->FINDERS->{$locator_strategy});
+    my ( $locator, $locator_strategy, $params, $desc ) = @_;
+
+    # case 4 args
+    if ($desc) {
+        $self->croak('Invalid locator strategy')
+          unless ( $self->FINDERS->{$locator_strategy} );
     }
-    else { 
-        if ($params) { 
+    else {
+        if ($params) {
+
             # means that we called it the 'old way' (no locator strategy)
-            if (!defined($self->FINDERS->{$locator_strategy})) { 
-                $desc = $params; 
-                $params = $locator_strategy; 
-                $locator_strategy = $self->_get_finder_key($self->default_finder);
+            if ( !defined( $self->FINDERS->{$locator_strategy} ) ) {
+                $desc   = $params;
+                $params = $locator_strategy;
+                $locator_strategy =
+                  $self->_get_finder_key( $self->default_finder );
             }
         }
-        else { 
-            # means it was called with no locator strategy and no desc 
-            if ($locator_strategy) { 
-                if (!defined($self->FINDERS->{$locator_strategy})) { 
-                    $params = $locator_strategy; 
-                    $locator_strategy = $self->_get_finder_key($self->default_finder);
+        else {
+            # means it was called with no locator strategy and no desc
+            if ($locator_strategy) {
+                if ( !defined( $self->FINDERS->{$locator_strategy} ) ) {
+                    $params = $locator_strategy;
+                    $locator_strategy =
+                      $self->_get_finder_key( $self->default_finder );
                 }
             }
-            else { 
+            else {
                 $self->croak('Not enough arguments');
             }
         }
     }
     unless ($desc) {
         $desc = $method;
-        $desc .= "'" . join( " ", ($params // '') ) . "'";
+        $desc .= "'" . join( " ", ( $params // '' ) ) . "'";
     }
-    return $self->find_element($locator,$locator_strategy)->$method( $params, $desc );
+    return $self->find_element( $locator, $locator_strategy )
+      ->$method( $params, $desc );
 }
 
 
@@ -294,9 +345,9 @@ label.
 =cut
 
 sub type_element_ok {
-    my $self    = shift;
-    my $method = 'send_keys_ok'; 
-    return $self->_find_element_with_action($method,@_);
+    my $self   = shift;
+    my $method = 'send_keys_ok';
+    return $self->_find_element_with_action( $method, @_ );
 }
 
 
@@ -307,9 +358,9 @@ sub type_element_ok {
 =cut
 
 sub element_text_is {
-    my $self = shift; 
+    my $self   = shift;
     my $method = 'text_is';
-    return $self->_find_element_with_action($method,@_);
+    return $self->_find_element_with_action( $method, @_ );
 }
 
 =head2 $twd->element_value_is($search_target[,$finder],$expected_value [,$desc]);
@@ -319,9 +370,9 @@ sub element_text_is {
 =cut
 
 sub element_value_is {
-    my $self = shift; 
+    my $self   = shift;
     my $method = 'value_is';
-    return $self->_find_element_with_action($method,@_);
+    return $self->_find_element_with_action( $method, @_ );
 }
 
 =head2 $twd->click_element_ok($search_target [,$desc]);
@@ -333,9 +384,9 @@ Find an element and then click on it.
 =cut
 
 sub click_element_ok {
-    my $self = shift; 
+    my $self   = shift;
     my $method = 'click_ok';
-    return $self->_find_element_with_action($method,@_);
+    return $self->_find_element_with_action( $method, @_ );
 }
 
 =head2 $twd->clear_element_ok($search_target [,$desc]);
@@ -347,9 +398,9 @@ Find an element and then clear on it.
 =cut
 
 sub clear_element_ok {
-    my $self = shift; 
+    my $self   = shift;
     my $method = 'clear_ok';
-    return $self->_find_element_with_action($method,@_);
+    return $self->_find_element_with_action( $method, @_ );
 }
 
 =head2 $twd->is_element_displayed_ok($search_target [,$desc]);
@@ -361,9 +412,9 @@ Find an element and check to confirm that it is displayed. (visible)
 =cut
 
 sub is_element_displayed_ok {
-    my $self = shift; 
+    my $self   = shift;
     my $method = 'is_displayed_ok';
-    return $self->_find_element_with_action($method,@_);
+    return $self->_find_element_with_action( $method, @_ );
 }
 
 =head2 $twd->is_element_enabled_ok($search_target [,$desc]);
@@ -375,9 +426,9 @@ Find an element and check to confirm that it is enabled.
 =cut
 
 sub is_element_enabled_ok {
-    my $self = shift; 
+    my $self   = shift;
     my $method = 'is_enabled_ok';
-    return $self->_find_element_with_action($method,@_);
+    return $self->_find_element_with_action( $method, @_ );
 }
 
 
@@ -385,21 +436,19 @@ sub is_element_enabled_ok {
 
    $twd->find_element_ok( $search_target [,$finder, $desc ] );
 
-Returns true if C<$search_target> is successfully found on the page. L<$search_target>
+Returns true if C<$search_target> is successfully found on the page. C<$search_target>
 is passed to L<Selenium::Remote::Driver/find_element> using a finder or the C<default_finder>
 if none passed.
 See there for more details on the format for C<find_element_ok()>.
 
 =cut
 
-# Eventually, it would be nice to support other finds like Test::WWW::Selenium does, like this:
-# 'xpath=//foo', or 'css=.foo', etc.
 
 =head2 $twd->find_no_element_ok($search_target [,$finder, $desc ]);
 
    $twd->find_no_element_ok( $search_target [,$finder, $desc ] );
 
-Returns true if C<$search_target> is I<not> found on the page. L<$search_target>
+Returns true if C<$search_target> is I<not> found on the page. C<$search_target>
 is passed to L<Selenium::Remote::Driver/find_element> using a finder or the
 C<default_finder> if none passed.See there for more details on the format. 
 for C<find_no_element_ok()>.
@@ -429,15 +478,23 @@ sub content_like {
     local $Test::Builder::Level = $Test::Builder::Level + 1;
 
     my $content = $self->get_page_source();
+    my $ret;
 
     if ( not ref $regex eq 'ARRAY' ) {
         $desc = qq{Content is like "$regex"} if ( not defined $desc );
-        return like_string( $content, $regex, $desc );
+        $ret = like_string( $content, $regex, $desc );
+        if ( !$ret ) {
+            $self->error_handler->($self,"Failed to find $regex");
+        }
+        return $ret;
     }
     elsif ( ref $regex eq 'ARRAY' ) {
         for my $re (@$regex) {
             $desc = qq{Content is like "$re"} if ( not defined $desc );
-            like_string( $content, $re, $desc );
+            $ret = like_string( $content, $re, $desc );
+            if ( !$ret ) {
+                $self->error_handler->($self,"Failed to find $re");
+            }
         }
     }
 }
@@ -463,15 +520,22 @@ sub content_unlike {
     local $Test::Builder::Level = $Test::Builder::Level + 1;
 
     my $content = $self->get_page_source();
+    my $ret;
 
     if ( not ref $regex eq 'ARRAY' ) {
         $desc = qq{Content is unlike "$regex"} if ( not defined $desc );
-        return unlike_string( $content, $regex, $desc );
+        $ret = unlike_string( $content, $regex, $desc );
+        if ( !$ret ) {
+            $self->error_handler->($self,"Failed to find $regex");
+        }
     }
     elsif ( ref $regex eq 'ARRAY' ) {
         for my $re (@$regex) {
             $desc = qq{Content is unlike "$re"} if ( not defined $desc );
-            unlike_string( $content, $re, $desc );
+            $ret = unlike_string( $content, $re, $desc );
+            if ( !$ret ) {
+                $self->error_handler->($self,"Failed to find $re");
+            }
         }
     }
 }
@@ -501,15 +565,23 @@ sub body_text_like {
     local $Test::Builder::Level = $Test::Builder::Level + 1;
 
     my $text = $self->get_body();
+    my $ret;
 
     if ( not ref $regex eq 'ARRAY' ) {
         $desc = qq{Text is like "$regex"} if ( not defined $desc );
-        return like_string( $text, $regex, $desc );
+        $ret = like_string( $text, $regex, $desc );
+        if ( !$ret ) {
+            $self->error_handler->($self,"Failed to find $regex");
+        }
+        return $ret;
     }
     elsif ( ref $regex eq 'ARRAY' ) {
         for my $re (@$regex) {
             $desc = qq{Text is like "$re"} if ( not defined $desc );
-            like_string( $text, $re, $desc );
+            $ret = like_string( $text, $re, $desc );
+            if ( !$ret ) {
+                $self->error_handler->($self,"Failed to find $re");
+            }
         }
     }
 }
@@ -539,15 +611,24 @@ sub body_text_unlike {
     local $Test::Builder::Level = $Test::Builder::Level + 1;
 
     my $text = $self->get_body();
+    my $ret;
 
     if ( not ref $regex eq 'ARRAY' ) {
         $desc = qq{Text is unlike "$regex"} if ( not defined $desc );
-        return unlike_string( $text, $regex, $desc );
+        $ret = unlike_string( $text, $regex, $desc );
+        if ( !$ret ) {
+            $self->error_handler->($self,"Failed to find $regex");
+        }
+        return $ret;
+
     }
     elsif ( ref $regex eq 'ARRAY' ) {
         for my $re (@$regex) {
             $desc = qq{Text is unlike "$re"} if ( not defined $desc );
-            unlike_string( $text, $re, $desc );
+            $ret = unlike_string( $text, $re, $desc );
+            if ( !$ret ) {
+                $self->error_handler->($self,"Failed to find $re");
+            }
         }
     }
 }
@@ -576,15 +657,24 @@ sub content_contains {
     local $Test::Builder::Level = $Test::Builder::Level + 1;
 
     my $content = $self->get_page_source();
+    my $ret;
 
     if ( not ref $str eq 'ARRAY' ) {
         $desc = qq{Content contains "$str"} if ( not defined $desc );
-        return contains_string( $content, $str, $desc );
+        $ret = contains_string( $content, $str, $desc );
+        if ( !$ret ) {
+            $self->error_handler->($self,"Failed to find $str");
+        }
+        return $ret;
     }
     elsif ( ref $str eq 'ARRAY' ) {
         for my $s (@$str) {
             $desc = qq{Content contains "$s"} if ( not defined $desc );
-            contains_string( $content, $s, $desc );
+            $ret = contains_string( $content, $s, $desc );
+
+            if ( !$ret ) {
+                $self->error_handler->($self,"Failed to find $s");
+            }
         }
     }
 }
@@ -611,15 +701,23 @@ sub content_lacks {
     local $Test::Builder::Level = $Test::Builder::Level + 1;
 
     my $content = $self->get_page_source();
+    my $ret;
 
     if ( not ref $str eq 'ARRAY' ) {
         $desc = qq{Content lacks "$str"} if ( not defined $desc );
-        return lacks_string( $content, $str, $desc );
+        $ret = lacks_string( $content, $str, $desc );
+        if ( !$ret ) {
+            $self->error_handler->($self,"Failed to find $str");
+        }
+        return $ret;
     }
     elsif ( ref $str eq 'ARRAY' ) {
         for my $s (@$str) {
             $desc = qq{Content lacks "$s"} if ( not defined $desc );
-            lacks_string( $content, $s, $desc );
+            $ret = lacks_string( $content, $s, $desc );
+            if ( !$ret ) {
+                $self->error_handler->($self,"Failed to find $s");
+            }
         }
     }
 }
@@ -649,15 +747,23 @@ sub body_text_contains {
     local $Test::Builder::Level = $Test::Builder::Level + 1;
 
     my $text = $self->get_body();
+    my $ret;
 
     if ( not ref $str eq 'ARRAY' ) {
         $desc = qq{Text contains "$str"} if ( not defined $desc );
-        return contains_string( $text, $str, $desc );
+        $ret = contains_string( $text, $str, $desc );
+        if ( !$ret ) {
+            $self->error_handler->($self,"Failed to find $str");
+        }
+        return $ret;
     }
     elsif ( ref $str eq 'ARRAY' ) {
         for my $s (@$str) {
             $desc = qq{Text contains "$s"} if ( not defined $desc );
-            contains_string( $text, $s, $desc );
+            $ret = contains_string( $text, $s, $desc );
+            if ( !$ret ) {
+                $self->error_handler->($self,"Failed to find $s");
+            }
         }
     }
 }
@@ -672,7 +778,7 @@ Tells if the text of the page (as returned by C<< get_body() >>)
 are provided, one 'test' is run for each regex against the content of the
 current page.
 
-A default description of 'Text is lacks "$str"' will be provided if there
+A default description of 'Text lacks "$str"' will be provided if there
 is no description.
 
 To also match the HTML see, C<< content_lacks() >>.
@@ -687,15 +793,23 @@ sub body_text_lacks {
     local $Test::Builder::Level = $Test::Builder::Level + 1;
 
     my $text = $self->get_body();
+    my $ret;
 
     if ( not ref $str eq 'ARRAY' ) {
-        $desc = qq{Text is lacks "$str"} if ( not defined $desc );
-        return lacks_string( $text, $str, $desc );
+        $desc = qq{Text lacks "$str"} if ( not defined $desc );
+        $ret = lacks_string( $text, $str, $desc );
+        if ( !$ret ) {
+            $self->error_handler->($self,"Failed to find $str");
+        }
+        return $ret;
     }
     elsif ( ref $str eq 'ARRAY' ) {
         for my $s (@$str) {
-            $desc = qq{Text is lacks "$s"} if ( not defined $desc );
-            lacks_string( $text, $s, $desc );
+            $desc = qq{Text lacks "$s"} if ( not defined $desc );
+            $ret = lacks_string( $text, $s, $desc );
+            if ( !$ret ) {
+                $self->error_handler->($self,"Failed to find $s");
+            }
         }
     }
 }

t/Test-Selenium-Remote-Driver.t

@@ -2,9 +2,11 @@
 use Test::More;
 use Test::Fatal;
 use Test::Selenium::Remote::Driver;
+use Test::Builder::Tester;
 use Selenium::Remote::WebElement;
 use Selenium::Remote::Mock::Commands;
 use Selenium::Remote::Mock::RemoteConnection;
+use Carp;
 
 my $find_element = sub {
     my ( undef, $searched_item ) = @_;
@@ -93,6 +95,7 @@ my $successful_driver =
     remote_conn => Selenium::Remote::Mock::RemoteConnection->new( spec => $spec, mock_cmds => $mock_commands ),
     commands => $mock_commands,
 );
+$successful_driver->error_handler(sub { my ($self,$msg) = @_; croak "Got message: $msg";}); 
 
 # find element ok tests
 $successful_driver->find_element_ok('q','find_element_ok works');
@@ -134,5 +137,37 @@ $successful_driver->clear_element_ok('q','element is cleared ok');
 $successful_driver->is_element_enabled_ok('p','class','element is enabled');
 $successful_driver->is_element_displayed_ok('q','element is displayed');
 
+test_out('not ok 1 - Content is ok'."\n".'ok 2 - Error callback triggered');
+like(exception { $successful_driver->content_like( qr/nomatch/, 'Content is ok') },qr/^Got message/,'Error callback triggered');
+test_test(title => "Error handler works with 'content_like'",skip_err => 1);
+
+test_out('not ok 1 - Content is ok'."\n".'ok 2 - Error callback triggered');
+like(exception { $successful_driver->content_unlike(qr/matches/, 'Content is ok') },qr/^Got message/,'Error callback triggered');
+test_test(title => "Error handler works with 'content_unlike'",skip_err => 1);
+
+test_out('not ok 1 - Content is ok'."\n".'ok 2 - Error callback triggered');
+like(exception { $successful_driver->content_contains('blah', 'Content is ok') },qr/^Got message/,'Error callback triggered');
+test_test(title => "Error handler works with 'content_contains'",skip_err => 1);
+
+test_out('not ok 1 - Content is ok'."\n".'ok 2 - Error callback triggered');
+like(exception { $successful_driver->content_lacks('matches', 'Content is ok') },qr/^Got message/,'Error callback triggered');
+test_test(title => "Error handler works with 'content_lacks'",skip_err => 1);
+
+test_out('not ok 1 - Body is ok'."\n".'ok 2 - Error callback triggered');
+like(exception { $successful_driver->body_text_like( qr/nomatch/, 'Body is ok') },qr/^Got message/,'Error callback triggered');
+test_test(title => "Error handler works with 'body_text_like'",skip_err => 1);
+
+test_out('not ok 1 - Body is ok'."\n".'ok 2 - Error callback triggered');
+like(exception { $successful_driver->body_text_unlike(qr/matches/, 'Body is ok') },qr/^Got message/,'Error callback triggered');
+test_test(title => "Error handler works with 'body_text_unlike'",skip_err => 1);
+
+test_out('not ok 1 - Text contains "nomatch"'."\n".'ok 2 - Error callback triggered');
+like(exception { $successful_driver->body_text_contains('nomatch') },qr/^Got message/,'Error callback triggered');
+test_test(title => "Error handler works with 'body_text_contains'",skip_err => 1);
+
+test_out('not ok 1 - Text lacks "match"'."\n".'ok 2 - Error callback triggered');
+like(exception { $successful_driver->body_text_lacks(['match','bar']) },qr/^Got message/,'Error callback triggered');
+test_test(title => "Error handler works with 'body_text_lacks'",skip_err => 1);
+
 
 done_testing();