867220 - [B2G] Implicit wait on find_element does not work

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Zac C (:zac)]

Attachment: test case

With these two test cases I demonstrate that the Implicit wait does not work in Marionette.

The test page adds an element to the page after setTimeout/5000ms.

The first test will try to find the element immediately and it will fail with a NoSuchElementException.

The second test includes a hardcoded sleep(6) 

Set_search_timeout is set to 10000 in the setUp of both tests.

As you can see this is conclusive proof that there was not a second shooter on the grassy knoll.

Comment 1

[Jonathan Griffin (:jgriffin)]

I'll take a look at this.

Comment 2

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

The following reduced test case works for me. This might imply it's a gaiatest issue rather than Marionette:

from marionette import Marionette
m = Marionette()
m.start_session()
m.set_search_timeout(10000)
m.navigate('http://zacc.github.io/impltest.html')
m.find_element('id', 'clickme').click()
m.find_element('id','find').is_displayed()

Comment 3

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

Or perhaps it's specific to the browser app in Gaia...

Comment 4

[Jonathan Griffin (:jgriffin)]

This occurs because the current implementation of set_search_timeout delivers the timeout to the active process, and this test switches processes.

We need to make set_search_timeout behave globally.

Comment 5

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

Yeah, I can confirm that adding self.marionette.set_search_timeout(10000) after switching to the browser app's content makes the test pass.

Comment 6

[Zac C (:zac)]

What is active process? Is that per iframe (in layman's terms)?

Comment 7

[Jonathan Griffin (:jgriffin)]

(In reply to Zac C (:zac) from comment #6)
> What is active process? Is that per iframe (in layman's terms)?

Generally, yes.

Comment 8

[Jonathan Griffin (:jgriffin)]

Attachment: Make setSearchTimeout behave globally,

Comment 9

[Jonathan Griffin (:jgriffin)]

try: https://tbpl.mozilla.org/?tree=Try&rev=98bf9d1cd9e5

Comment 10

[Malini Das [:mdas] - Away, not checking bugmail]

Comment on attachment 743857 [details] [diff] [review]
Make setSearchTimeout behave globally,

Review of attachment 743857 [details] [diff] [review]:
-----------------------------------------------------------------

1 potential issue. The rest looks good

::: testing/marionette/marionette-elements.js
@@ +290,4 @@
>        }
>        return;
>      } else {
> +      if (searchTimeout == 0 || new Date().getTime() - startTime > searchTimeout) {

We should catch if !searchTimeout since we set it to null by default in this patch

Comment 11

[Jonathan Griffin (:jgriffin)]

Attachment: Make setSearchTimeout behave globally,

Good catch, updated.

Comment 12

[Jonathan Griffin (:jgriffin)]

Comment on attachment 743887 [details] [diff] [review]
Make setSearchTimeout behave globally,

carry r+ forward

Comment 13

[Jonathan Griffin (:jgriffin)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/95993aae5d96

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/95993aae5d96

Comment 15

[Jonathan Griffin (:jgriffin)]

checkin-needed to mozilla-b2g18 please

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/c161ceec6480

Comment 17

[Jonathan Griffin (:jgriffin)]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/1cafe915804f