Closed Bug 867220 Opened 8 years ago Closed 8 years ago

[B2G] Implicit wait on find_element does not work

Categories

(Testing :: Marionette, defect)

Other
Gonk (Firefox OS)
defect
Not set
major

Tracking

(firefox21 wontfix, firefox22 wontfix, firefox23 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

RESOLVED FIXED
mozilla23
Tracking Status
firefox21 --- wontfix
firefox22 --- wontfix
firefox23 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- fixed

People

(Reporter: zcampbell, Assigned: jgriffin)

References

Details

Attachments

(2 files, 1 obsolete file)

Attached file 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.
I'll take a look at this.
Assignee: nobody → jgriffin
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()
Or perhaps it's specific to the browser app in Gaia...
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.
Yeah, I can confirm that adding self.marionette.set_search_timeout(10000) after switching to the browser app's content makes the test pass.
What is active process? Is that per iframe (in layman's terms)?
(In reply to Zac C (:zac) from comment #6)
> What is active process? Is that per iframe (in layman's terms)?

Generally, yes.
Attachment #743857 - Flags: review?(mdas)
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
Attachment #743857 - Flags: review?(mdas) → review+
Good catch, updated.
Attachment #743857 - Attachment is obsolete: true
Comment on attachment 743887 [details] [diff] [review]
Make setSearchTimeout behave globally,

carry r+ forward
Attachment #743887 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/95993aae5d96
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
checkin-needed to mozilla-b2g18 please
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.