938201 - Take a search_timeout argument when attempting to locate elements

Status: RESOLVED WONTFIX

(Remote Protocol :: Marionette, defect)

Description

[Dave Hunt [:davehunt] [he/him] ⌚BST]

We have a cool script_timeout on execute_script and execute_async_script that allows us to set the timeout for a specific script rather than using the timeout set using set_script_timeout, which is really handy when we need to extend the timeout without having to then set it back.

It would be great to have this for find_element too, so we can specify (for example) to not wait for the element to exist if a search timeout has already been set. We're doing this currently by forcing the search timeout to 0 and then setting it back to 10 seconds, but this kind of hard-coding is bad.

If we had something like marionette.find_element('id', 'myElement', search_timeout=0) we'd be able to do what we need without affecting the server set timeout.

Comment 1

[David Burns :automatedtester]

Why isn't doing explicit timeouts on the local end sufficient?

Comment 2

[Zac C (:zac)]

Basically we want to not implicitly wait when doing a "is not present" find element.

If you do find_element expecting an exception then you'll have to wait for the search_timeout before you get that exception. You can't just get the results immediately without, as dave mentioned, setting the timeout to 0 and then setting it back again.

Comment 3

[David Burns :automatedtester]

Note: The following comes from a place of hating implicit waiting as the scourge of the earth!

My opinion would be to still be rather explicit here. When someone does

    marionette.set_search_timeout(0)
    marionette.find_element(by, value)
    marionette.set_search_timeout(10)

They are showing the intent of the code that you want it to fail fast if it doesnt find the element

If we leave it at 

    marionette.find_element(by, value) 

and there is no searchTime because someone forgot to add it there then we never know if it should/shouldnt be there without having to read a lot more of the code. This, and is only my opinion, is why implicit waits shouldnt be in WebDriver

Comment 4

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to David Burns :automatedtester from comment #3)
> Note: The following comes from a place of hating implicit waiting as the
> scourge of the earth!

I tend to agree, however the fact is that the implicit waits are very popular with test writers and many tests (including lots of ours) have come to rely on them.

> My opinion would be to still be rather explicit here. When someone does
> 
>     marionette.set_search_timeout(0)
>     marionette.find_element(by, value)
>     marionette.set_search_timeout(10)
> 
> They are showing the intent of the code that you want it to fail fast if it
> doesnt find the element

Indeed, but the issue is the hard-coded 10 second implicit wait. This doesn't take into account what the previous value for the implicit wait was. My suggestion would make this:

    marionette.find_element(by, value, search_timeout=0)

Therefore preserving the implicit wait value.

> If we leave it at 
> 
>     marionette.find_element(by, value) 
> 
> and there is no searchTime because someone forgot to add it there then we
> never know if it should/shouldnt be there without having to read a lot more
> of the code. This, and is only my opinion, is why implicit waits shouldnt be
> in WebDriver

We allow setting a script timeout and overriding this on individual scripts. I feel bringing this to the search timeout would be a natural progression. If the answer is to use explicit waits everywhere, then we should be discouraging use of search timeout altogether. I think this would be a challenge, given its popularity.

Comment 5

[David Burns :automatedtester]

>We allow setting a script timeout and overriding this on individual scripts. I feel bringing this to the >search timeout would be a natural progression. If the answer is to use explicit waits everywhere, then we >should be discouraging use of search timeout altogether. I think this would be a challenge, given its >popularity.

I agree this would be a huge challenge! I personally think it is as big a challenge as getting people to only Selenium IDE for noddy scripts, which is near impossible. 

The webdriver project has done a number of things wrong and I _really_ think this is one of them. I personally would discourage anyone from doing implicit waits and I know there are other WebDriver developers agree.

I agree this does look like a natural progression but because it removes the intent of the code I am -1 this change. It also looks like a micro optimisation of the API which doesn't feel like it is needed.

Comment 6

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Okay, we can certainly live with the hard-coded value. Do we need any further opinions here? I'm happy to close this as WONTFIX.

Comment 7

[David Burns :automatedtester]

I don't think so, left it open in case someone wanted to chip in.

Closing now.