Speed of selection

[p14n]

Hi Nic,

Page attached, running this code:

    WebDriver driver = new ChromeDriver();
    RedsniffWebDriverTester browser = new RedsniffWebDriverTester(driver);
    browser.goTo("file:///Users/p14n/dev/jhc/redsniff/CreateEvent.html");
    MFinder ownerSelectContainer = list().that(hasAttribute("data-ref", "users-list"));
    browser.clickOn(listItem().that(hasText("DART1")).withinA(ownerSelectContainer));
    driver.quit();

Will give you this warning:
!!!WARNING!!! has data-ref "users-list" would throw UnoptimizedNotPossibleException
!!!WARNING!!! took 7.552 - to filter 87 elements down to 1 - has data-ref "users-list"

Any pointers or suggestions on either optimising the selection or where to look to put together a PR gratefully received.

Cheers,
Dean
createevent.zip

[nico78]

Hey,
I'll take a proper look later when i can run it, but in the meantime - TLDR; it's retrieving every <ul> then looping through each one and querying the data-ref attribute. So quick answer for now might be to replace list().that(hasAttribute("data-ref", "users-list")) with css selector - $("ul[data-ref=users-list]") or use css class instead of data-ref.
Or see what selenium/webdriver commands were coming out of protractor when you searched by dataref and use whatever that is to create a hasDataRef() method. (I suspect it's using css selector).
Then i can put that into the optimisation.

Longer answer: thought i had something on the wiki about the optimisation logic but looks like not.

The (fairly crude) optimisation is about whether it chooses to use your selection criteria directly to issue a specific webdriver command to get straight to the element (which is obviously quicker since it uses the browser's own indexing of the dom by id/name/class etc), or if it retrieves a list of elements then locally checks each one for whether they match the critieria (each check might also result in another webdriver command, depending on what you're checking).

If it thinks it can find a quicker way of running other than looping then it tries that first. At the moment the quick ways it tries are for anything where you've used a literal String for an id, class or name - hasId("myId") .. anything that can be turned into a By.id(..), By.name(..) or By.cssClass(..) .

It can't currently do that with hasAttribute(..) so didn't run in this optimised mode. We can probably add something that does.

That warning "took 7.552 - to filter 87 elements down to 1 " is saying that it's spent a long time doing this kind of manual filtering - it retrieved all <ul>s and queried the data-ref on each one to find yours, rather than get the browser to find it.

The "would throw unoptimized" thing : Finding things the quickest way has the drawback that it won't give nice feedback if the thing isn't found, so there's a fallback to use the cruder way if the quick way fails.

eg, if you useddiv().that(hasId("myid")) the optimisation would first switch it around to be equivalent to something like id("myid").that(hasTagname("div)) and so issue adriver.find(By.Id("myid")) and check the returned element is a div.
But if your page didn't have such an element, then, rather than just say "myid not found", it falls back to running the query again "unoptimized", knowing it would fail, so it could give you a pointer to what was wrong - something like : "n divs found but none matched id:myid - did find these: [his]Id, [her]Id etc"

That's the idea, but there's some complication in implementing the fallback that's not quite right - that error message about UnoptimizedNotPossibleException was originally actuallly throwing the exception as a sort of "shouldn't get to here" assertion - but it was getting to there - so I turned it into a (badly worded) warning.
It needs a bit of refactoring to get rid of it. Basically it needs to distinguish between "running unoptimised as a fallback" and "running unoptimised because there was no better way".
And probably a more helpful "your query is taking too long" message

Nic

[nico78]

Ok, have put a change in to make hasAttribute(..) use optimization now - so in your case it will use By.cssSelector("[data-ref=users-list]") and then check the element is a <ul>. It's much faster now for that first find.

(At the moment the optimization only works at individual matcher level - ideally it would be good to do the optimization across the whole finder - ie include the list() in the css selection and turn list().that(hasAttribute("...") into By.cssSelector("ul[data-ref=users-list") but i think it's a bit complicated.)

It's still slow on the second bit - finding the DART1 item in the list - 2.5 seconds on my machine. That's because for finding based on text content there's not really an alternative to looping through and calling .getText on each element in the list. By.partialLinkText doesn't work as it's not a link.
If you can add the text as an attribute somewhere on the element then you could use css selector.

BTW Have also updated to use latest version of selenium, which meant a few other changes. if you don't want that just use the penultimate commit
Cheers

Nic

[p14n]

Read this in the middle of the night and then forgot I had. Thanks again.
I'll get the latest and we can try it out.

On 31 January 2016 at 14:22, nico78 notifications@github.com wrote:

Ok, have put a change in to make hasAttribute(..) use optimization now -
so in your case it will use By.cssSelector("[data-ref=users-list]") and
then check the element is a

. It's much faster now for that first
find.

(At the moment the optimization only works at individual matcher level -
ideally it would be good to do the optimization across the whole finder -
ie include the list() in the css selection and turn
list().that(hasAttribute("...") into
By.cssSelector("ul[data-ref=users-list") but i think it's a bit
complicated.)

It's still slow on the second bit - finding the DART1 item in the list -
2.5 seconds on my machine. That's because for finding based on text content
there's not really an alternative to looping through and calling .getText
on each element in the list. By.partialLinkText doesn't work as it's not
a link.

If you can add the text as an attribute somewhere on the element then you
could use css selector.

BTW Have also updated to use latest version of selenium, which meant a few
other changes. if you don't want that just use the penultimate commit
Cheers

Nic

—
Reply to this email directly or view it on GitHub
#2 (comment).