'find_element'-like functions in Test::Selenium::Remote::Driver should support a locator strategy as an argument #179

gempesaw · 2015-01-22T16:31:16Z

I raise this issue mainly because I found myself writing a lot lately :

$driver->default_finder('id');
$driver->find_element_ok('some_id', 'my element was found'); 
$driver->default_finder('xpath'); 
$driver->find_element_ok('//some_xpath', 'my other element was found');

If we could pass the locator strategy as an argument, it would allow one to write :

$driver->find_element_ok('some_id','id','my element was found'); 
$driver->find_element_ok('//some_xpath','xpath', 'my other element was found');

There might be some issues doing this, but overall it would improve the readability of code written with T::S::R::D.

gempesaw · 2015-01-21T14:12:16Z

Sure, this makes a good amount of sense.

Other language bindings do things like find_element_by_id and find_element_by_css, embedding the strategy in the function name. Is that interesting?

Are you planning to support both the two and three argument versions of find_element_ok, or just break the previous usage?

peroumal1 · 2015-01-21T14:14:29Z

I guess I'll go for the simpler one 😊

gempesaw · 2015-01-21T15:47:24Z

Actually this could be combined with solving the croaking issues we have, if you're amenable. For example,

sub find_element_by_id {
    my ($self, $locator) = @_;
    my $finder = 'id';
    return try {
        return $self->find_element($locator, $finder);
    }
    catch {
        carp 'hope you notice we didn\'t find an element!'
        return 0;

        # or we could even try
        return Selenium::Remote::Null->new;
    };
}

I think we should be able to generate appropriate functions based on our list of FINDERS and Sub::Install them or so. If we return that weird Null object, they won't even throw when blindly chained.

And then we could propagate them similarly into Test::Driver.pm ?

peroumal1 · 2015-01-21T15:57:54Z

So, your idea here is to introduce a new non-croaking family of functions ?
If this is it, I guess this should be addressed in a separate issue in order to keep things clean :)
So far on what I am doing, there are some issues already on T::S::R::D implementation, I don't feel like over-complicating things by also doing this at the same time.

gempesaw · 2015-01-21T16:02:59Z

Fair enough, don't worry about it then :)

peroumal1 · 2015-01-21T16:05:30Z

👍
Those bugs I am finding also make me wonder if T::S::R::D is of any use at all...

peroumal1 · 2015-01-21T17:48:27Z

So I have something working so far, but it might need some refactoring (https://github.com/gempesaw/Selenium-Remote-Driver/tree/fix-179-locator-strategy-in-T-S-R-D) :

the _check_ok function has become really ugly
type_element_ok does not support a locator yet

peroumal1 · 2015-01-22T11:18:13Z

Another thing, I come to think that using positional parameters everywhere is kind of a PITA, mainly when we want to add new parameters (like here, adding the finding strategy).
@gempesaw, maybe we should at some point migrate to named parameters instead. This would be a breaking change, for sure, but this would better for the implementation.

peroumal1 · 2015-01-22T12:50:40Z

OK I guess I am done, no need to update the mocks, the tests were put in t/Test-Selenium-Remote-Driver.t using the other mocking technique (adding a sub that returns an expected response).
~~@gempesaw can you tell if this looks good to you ?~~ sorry went too fast, there are still some subs missing.

…ement

peroumal1 · 2015-01-22T16:12:15Z

OK done this time ! Waiting for approval :)

gempesaw · 2015-01-22T16:33:04Z

t/Test-Selenium-Remote-Driver.t

@@ -5,22 +5,84 @@ use Test::Selenium::Remote::Driver;
 use Selenium::Remote::WebElement;
 use Selenium::Remote::Mock::Commands;
 use Selenium::Remote::Mock::RemoteConnection;
+use DDP; 


nitpicking, pls remove :D

peroumal1 · 2015-01-22T16:36:09Z

Done, thanks 👍

gempesaw · 2015-01-22T16:43:25Z

lib/Test/Selenium/Remote/Driver.pm

+        'find_child_element'     => 3,
+        'find_child_elements'     => 3,
+        'find_element'     => 2,
+        'find_elements'     => 2,
        'compare_elements' => 2,


ah wow I see what you mean about complicated. yeah the named parameter version seems more and more attractive. this is good for now!

Yeah :)
We could try and migrate T::S::R::D on a named parameter version, since this class seems to be used by few. Something to add on the roadmap at some point ?

peroumal1 · 2015-01-22T17:09:47Z

Hmm is it me or are some Travis jobs that are failing with no good reasons at all ?

gempesaw · 2015-01-22T17:16:03Z

lib/Test/Selenium/Remote/Role/DoesTesting.pm

+        if ($method eq 'find_no_element') { 
+            $real_method = $method;
+            $method = 'find_element'; 
+        }


sorry I can't see where we're inverting the return in the case that it's find_no_element. I understand that we need to actually be asking for find_element, since there's no find_no_element on Driver.pm, but I must be missing where we say "find_element found nothing, that means we pass" ? Oh is 0 a successful return and 1 is error-ful? fair enough

Yes, that's it. I hacked this a bit quickly, I guess there's room for improvement for sure.

ah yeah i'm not concerned about the "quickly" nature, I was just super confused as to how it was working :P

gempesaw · 2015-01-22T17:34:26Z

Yes the travisCI is not always helpful - basically it just checks that the Linux mocks work for the tests, but it doesn't actually run the tests against a live webdriver server. I've been meaning to switch that over, actually - download webdriver and do a record run on travis instead of just checking against mocks. Maybe even use travisCI to commit updated linux mocks to a branch that we could pick and choose from as necessary....

…nctions, added some tests

…-R-D 'find_element'-like functions in Test::Selenium::Remote::Driver should support a locator strategy as an argument

[NEW FEATURES] - #178 Fix upload_file's return value & add test suite - #179 Add optional middle finder-strategy argument to T:S:R:D's find_element-like functions - #174 Add new endpoints: cache status, geolocation, log types, orientation, etc [BUG FIXES] - #175 Fix find_elements - #180 Stop overwriting body_text_unlike test descriptions - #141 Explicitly cast height and weight to integers for set_window_size

gempesaw · 2015-01-25T22:23:33Z

Thanks! Now on cpan as v0.23

peroumal1 self-assigned this Jan 21, 2015

find_element(s)_ok functions are covered

ecbf21a

peroumal1 added 4 commits January 21, 2015 17:28

Fixed things up for find_child_elements

8c42792

Fixed find_no_element_ok, now uses the same logic as other _ok functions

5c16eac

Fixed content_unlike, context issue on 'desc' var

5ae7043

Doc update for find_element(s)_ok

c1386ac

peroumal1 added 4 commits January 22, 2015 10:57

simplified the code of check_ok

b461bbd

Cleaned up 'find_no_element_ok'

4412d75

Fixed some non-sense in the mock part

a3493c4

#179 totally fixed, took care of 'type_elements_ok'

a7a8bc8

#179 Covered the functions using find_element + _ok on an S::R::D::El…

a0a2604

…ement

gempesaw reviewed Jan 22, 2015
View reviewed changes

#179 Removed debug helpers

3da8eb9

gempesaw reviewed Jan 22, 2015
View reviewed changes

Fixed doc + FINDER constant test

9552870

gempesaw reviewed Jan 22, 2015
View reviewed changes

gempesaw added this to the 0.23 milestone Jan 22, 2015

peroumal1 mentioned this pull request Jan 23, 2015

body_text_unlike in T::S::R::D #180

Closed

Fix #180 : removed the 'my' before some desc in body_ and content_ fu…

c30b087

…nctions, added some tests

gempesaw added a commit that referenced this pull request Jan 24, 2015

Merge pull request #179 from gempesaw/fix-179-locator-strategy-in-T-S…

8d9fefd

…-R-D 'find_element'-like functions in Test::Selenium::Remote::Driver should support a locator strategy as an argument

gempesaw merged commit 8d9fefd into master Jan 24, 2015

gempesaw deleted the fix-179-locator-strategy-in-T-S-R-D branch January 24, 2015 18:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

'find_element'-like functions in Test::Selenium::Remote::Driver should support a locator strategy as an argument #179

'find_element'-like functions in Test::Selenium::Remote::Driver should support a locator strategy as an argument #179

gempesaw commented Jan 22, 2015

gempesaw commented Jan 21, 2015

peroumal1 commented Jan 21, 2015

gempesaw commented Jan 21, 2015

peroumal1 commented Jan 21, 2015

gempesaw commented Jan 21, 2015

peroumal1 commented Jan 21, 2015

peroumal1 commented Jan 21, 2015

peroumal1 commented Jan 22, 2015

peroumal1 commented Jan 22, 2015

peroumal1 commented Jan 22, 2015

gempesaw Jan 22, 2015

peroumal1 commented Jan 22, 2015

gempesaw Jan 22, 2015

peroumal1 Jan 22, 2015

peroumal1 commented Jan 22, 2015

gempesaw Jan 22, 2015

peroumal1 Jan 22, 2015

gempesaw Jan 22, 2015

gempesaw commented Jan 22, 2015

gempesaw commented Jan 25, 2015

'find_element'-like functions in Test::Selenium::Remote::Driver should support a locator strategy as an argument #179

'find_element'-like functions in Test::Selenium::Remote::Driver should support a locator strategy as an argument #179

Conversation

gempesaw commented Jan 22, 2015

gempesaw commented Jan 21, 2015

peroumal1 commented Jan 21, 2015

gempesaw commented Jan 21, 2015

peroumal1 commented Jan 21, 2015

gempesaw commented Jan 21, 2015

peroumal1 commented Jan 21, 2015

peroumal1 commented Jan 21, 2015

peroumal1 commented Jan 22, 2015

peroumal1 commented Jan 22, 2015

peroumal1 commented Jan 22, 2015

gempesaw Jan 22, 2015

Choose a reason for hiding this comment

peroumal1 commented Jan 22, 2015

gempesaw Jan 22, 2015

Choose a reason for hiding this comment

peroumal1 Jan 22, 2015

Choose a reason for hiding this comment

peroumal1 commented Jan 22, 2015

gempesaw Jan 22, 2015

Choose a reason for hiding this comment

peroumal1 Jan 22, 2015

Choose a reason for hiding this comment

gempesaw Jan 22, 2015

Choose a reason for hiding this comment

gempesaw commented Jan 22, 2015

gempesaw commented Jan 25, 2015