Fix #329 -- Uninitialized value warnings in S::R::Commands #332
Conversation
It just wasn't setting sane defaults before running regex replace. This commit additionally adds test coverage for the modified subroutine, hopefully in the correct place. I also added a block to properly include the libdir when running the affected test via prove (so that you don't have to use -Ilib or something like that) whenever we're not running the test via 'dzil test' or anything else where we've plopped the libs and tests in a temporary '.build' directory.
t/04-commands-implemented.t
Outdated
|
|
||
| unless($ENV{RELEASE_TESTING}) { | ||
| plan(skip_all=>"Author tests not required for installation."); | ||
| # Assume we're not using dzil test unless we're doing that |
There was a problem hiding this comment.
If you are going to do this, just use FindBin.
t/04-commands-implemented.t
Outdated
| or /{'?commands'?}->\{'?$command'?}/) { | ||
| pass("find $command"); | ||
| $found_command = 1; | ||
| require_ok( "Selenium::Remote::Commands" ) || die; |
There was a problem hiding this comment.
would prefer a message with the die
t/04-commands-implemented.t
Outdated
| } | ||
| } | ||
| if (!$found_command && $command !~ /Gecko/) { |
There was a problem hiding this comment.
This seems profoundly silly to use pass/fail when one could simply fall through and do ok($found_command,"found $command") unless $command =~ /Gecko/
There was a problem hiding this comment.
I don't recall writing that bit... maybe I moved it?
|
K, updated the test for review commentary. Speeds up t/04-commands-implemented.t by 1s due to not reading in all three files every time you searched for one command. |
| is($elems->[0]->get_value(),$array_elems[0]->get_value(), 'and the elements returned are the same'); | ||
| { | ||
| no warnings 'redefine'; | ||
| local *Selenium::Remote::WebElement::get_value = sub { return "zippy"; }; |
There was a problem hiding this comment.
Not a fan of redefining the method you then immediately call. Would be better to mock dependent methods in the method called.
| is($elem->get_attribute('id'),'checky','Execute found proper element'); | ||
| { | ||
| no warnings 'redefine'; | ||
| local *Selenium::Remote::WebElement::get_attribute = sub { return "checky"; }; |
| @@ -23,6 +23,8 @@ my $ret; | |||
| my $elem; | |||
|
|
|||
| LINK: { | |||
| no warnings 'redefine'; | |||
| local *Selenium::Remote::WebElement::click = sub { return; }; | |||
t/04-commands-implemented.t
Outdated
| fail("find $command"); | ||
| } | ||
| my @cmds2see = keys( %{ Selenium::Remote::Commands->new->get_cmds } ); | ||
| # You won't find the Gecko command in Selenium::Rmeote::Commands. This is intentional, so remove it from consideration. |
| @@ -31,10 +31,15 @@ my %selenium_args = ( | |||
| $driver->get('https://www.google.com'); | |||
| my $input_text = $driver->find_element("//input[\@type='text']"); | |||
|
|
|||
| use Carp::Always; | |||
There was a problem hiding this comment.
Do we need this, or was this thrown in for debugging?
| use Test::Selenium::Remote::Driver; | ||
| use Selenium::Remote::Mock::RemoteConnection; | ||
|
|
||
| use Carp::Always; |
It just wasn't setting sane defaults before running regex replace.
This commit additionally adds test coverage for the modified
subroutine, hopefully in the correct place.
I also added a block to properly include the libdir when running
the affected test via prove (so that you don't have to use -Ilib
or something like that) whenever we're not running the test via
'dzil test' or anything else where we've plopped the libs and
tests in a temporary '.build' directory.