Browse Source

Coerce timeouts in various set*timeout subroutines

The tests for these aren't perfect - that is, we only test the
successful coercion for `set_implicit_wait_timeout`, but not for all of
them. We _do_ test the failed coercion for all of the subroutines.

The reason is because I'm too lazy to re-record the recordings, and
refraining from adding additional successful set*timeout calls means we
don't make any additional requests to the server, so we don't need to
change the recordings.

In theory, this shouldn't cause any issues...
Daniel Gempesaw 10 years ago
parent
commit
e5fb913e85
2 changed files with 23 additions and 8 deletions
  1. 8 5
      lib/Selenium/Remote/Driver.pm
  2. 15 3
      t/01-driver.t

+ 8 - 5
lib/Selenium/Remote/Driver.pm

@@ -968,9 +968,11 @@ sub get_capabilities {
 
 
 sub set_timeout {
 sub set_timeout {
     my ( $self, $type, $ms ) = @_;
     my ( $self, $type, $ms ) = @_;
-    if ( not defined $type or not defined $ms ) {
-        croak "Expecting type & timeout in ms";
+    if ( not defined $type  ) {
+        croak "Expecting type";
     }
     }
+    $ms = _coerce_timeout_ms( $ms );
+
     my $res = { 'command' => 'setTimeout' };
     my $res = { 'command' => 'setTimeout' };
     my $params = { 'type' => $type, 'ms' => $ms };
     my $params = { 'type' => $type, 'ms' => $ms };
     return $self->_execute_command( $res, $params );
     return $self->_execute_command( $res, $params );
@@ -994,9 +996,8 @@ sub set_timeout {
 
 
 sub set_async_script_timeout {
 sub set_async_script_timeout {
     my ( $self, $ms ) = @_;
     my ( $self, $ms ) = @_;
-    if ( not defined $ms ) {
-        croak "Expecting timeout in ms";
-    }
+    $ms = _coerce_timeout_ms( $ms );
+
     my $res    = { 'command' => 'setAsyncScriptTimeout' };
     my $res    = { 'command' => 'setAsyncScriptTimeout' };
     my $params = { 'ms'      => $ms };
     my $params = { 'ms'      => $ms };
     return $self->_execute_command( $res, $params );
     return $self->_execute_command( $res, $params );
@@ -1026,6 +1027,8 @@ sub set_async_script_timeout {
 
 
 sub set_implicit_wait_timeout {
 sub set_implicit_wait_timeout {
     my ( $self, $ms ) = @_;
     my ( $self, $ms ) = @_;
+    $ms = _coerce_timeout_ms( $ms );
+
     my $res    = { 'command' => 'setImplicitWaitTimeout' };
     my $res    = { 'command' => 'setImplicitWaitTimeout' };
     my $params = { 'ms'      => $ms };
     my $params = { 'ms'      => $ms };
     return $self->_execute_command( $res, $params );
     return $self->_execute_command( $res, $params );

+ 15 - 3
t/01-driver.t

@@ -195,10 +195,22 @@ WINDOW: {
 
 
     $ret = $driver->get_page_source();
     $ret = $driver->get_page_source();
     ok($ret =~ m/^<html/i, 'Received page source');
     ok($ret =~ m/^<html/i, 'Received page source');
-    eval {$driver->set_implicit_wait_timeout(20001);};
-    ok(!$@,"Set implicit wait timeout");
+
+    # Using a string instead of an int exercises the
+    # _coerce_timeout_ms functionality
+    eval {$driver->set_implicit_wait_timeout("20001");};
+    ok(!$@,"Set implicit wait timeout with string");
     eval {$driver->set_implicit_wait_timeout(0);};
     eval {$driver->set_implicit_wait_timeout(0);};
-    ok(!$@,"Reset implicit wait timeout");
+    ok(!$@,"Reset implicit wait timeout with integer");
+
+    my $invalid_ms = 'invalid timeout ms';
+    ok( exception{ $driver->set_timeout('script', $invalid_ms) },
+        'Coerce ms arguments for set_timeout' );
+    ok( exception{ $driver->set_async_script_timeout( $invalid_ms ) },
+        'Coerce ms arguments for set_async_script_timeout' );
+    ok( exception{ $driver->set_implicit_wait_timeout( $invalid_ms ) },
+        'Coerce ms arguments for set_implicit_wait_timeout' );
+
     $ret = $driver->get("$website/frameset.html");
     $ret = $driver->get("$website/frameset.html");
     $ret = $driver->switch_to_frame('second');
     $ret = $driver->switch_to_frame('second');