[B2G] Implicit wait on find_element does not work

RESOLVED FIXED in Firefox 23

Status

Testing
Marionette
--
major
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: zac, Assigned: jgriffin)

Tracking

unspecified
mozilla23
Other
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

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

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 743676 [details]
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.
Blocks: 801898
(Assignee)

Comment 1

5 years ago
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...
(Assignee)

Comment 4

5 years ago
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.
(Reporter)

Comment 6

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

Comment 7

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

Generally, yes.
(Assignee)

Comment 8

5 years ago
Created attachment 743857 [details] [diff] [review]
Make setSearchTimeout behave globally,
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+
(Assignee)

Comment 11

5 years ago
Created attachment 743887 [details] [diff] [review]
Make setSearchTimeout behave globally,

Good catch, updated.
(Assignee)

Updated

5 years ago
Attachment #743857 - Attachment is obsolete: true
(Assignee)

Comment 12

5 years ago
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
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 15

5 years ago
checkin-needed to mozilla-b2g18 please
Keywords: checkin-needed
https://hg.mozilla.org/releases/mozilla-b2g18/rev/c161ceec6480
status-b2g18: --- → fixed
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → wontfix
status-firefox21: --- → wontfix
status-firefox22: --- → wontfix
status-firefox23: --- → fixed
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.