830197 - Marionette not waiting long enough before send_keys

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Bob Silverberg [:bsilverberg]]

Attachment: Test case for send_keys issue

We are experiencing intermittent failures with a test for the Clock app on Firefox OS and have narrowed it down to a problem with calling send_keys on an element immediately after finding it.

I have attached a reduced test case to this bug. Running this I am able to generate a failure within 5 iterations most of the time.

Comment 1

[Bob Silverberg [:bsilverberg]]

Note that I tried to create a reduced test case using the UI Tests app, and was unable to replicate the problem with that.

Comment 2

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

Comment on attachment 701654 [details]
Test case for send_keys issue

The attachment is not a patch. I'm sure Bob could make it a patch though if that makes it easier.

Comment 3

[Jonathan Griffin (:jgriffin)]

What is the error you see when this happens?

Comment 4

[Bob Silverberg [:bsilverberg]]

There is no error message per se. The input field is not updated via the send_keys. 

Specifically with the test case attached, it is launching Clock, creating a new alarm, and trying to type in the Alarm Label field. Sometimes it works and the text that we specify _is_ entered into the Alarm Label field, but sometimes the field is not updated. The attached test case uses a loop to try this over and over again, and an assert to check whether the input was populated. If the assert fails this indicates that the send_keys failed and the input was not populated.  Note that we can see this happening on the device when we watch, so it's not just an issue of element.get_attribute('value') not picking up the change - the send_keys does appear to be failing to populate the field.

Comment 5

[Zac C (:zac)]

Now I look at this closely I think it might be just the timing after the asynchronous tap, a problem we have experienced with other tests when using tap().. 

but I also know that marionette suffers from is_displayed problems in this app and that you can't easily wait for elements in this test.

Comment 6

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

This passes every time for me if I add a wait for the alarm label element to be displayed (see below). We could enhance the implicit wait when locating elements to wait for the element to be displayed, but I would generally prefer such an explicit wait.

self.wait_for_condition(lambda m: alarm_label.is_displayed())

Comment 7

[Bob Silverberg [:bsilverberg]]

Ok, so you're saying that if we wait for the element to be displayed, rather than just finding it, then it works. That's fine with me for a workaround, but I believe that we raised this issue because we thought that Marionette shouldn't behave the way it does. If we use send_keys to populate a field and the field is not populated, should we get back an error, or is it ok for it to just fail silently as it is doing in this case?

Comment 8

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

I think if we try to interact with an element that is not displayed, Marionette should probably fail. I've seen this before so maybe this is another use case. I'm happy for this to be reopened, but think the wait would still be needed in the test.

Comment 9

[Bob Silverberg [:bsilverberg]]

I am seeing the same behaviour with the awesome bar in the browser. The attached test case fails intermittently because the send_keys is happening before the element is visible. It doesn't update the awesome bar, but also doesn't throw an exception. If I add a wait_for_element_visible before trying to send_keys then it passes every time.

I believe the issue here is that if the element isn't visible and therefore cannot have keys sent to it, calling send_keys should throw an exception.

Comment 10

[Bob Silverberg [:bsilverberg]]

Attachment: The test that reproduces the issue, albeit intermittently

Comment 11

[David Burns :automatedtester]

Attachment: check visibility before typing

Comment 12

[David Burns :automatedtester]

Desktop try: https://tbpl.mozilla.org/?tree=Try&rev=3110b5d111fa
Mobile try: https://tbpl.mozilla.org/?tree=Try&rev=80d9103b9c83

Comment 13

[David Burns :automatedtester]

landed in https://hg.mozilla.org/integration/mozilla-inbound/rev/82ee824d2ffb

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/82ee824d2ffb

Comment 15

[David Burns :automatedtester]

landed https://hg.mozilla.org/releases/mozilla-b2g18/rev/e2d297ddf01d

Comment 16

[Jonathan Griffin (:jgriffin)]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/895ff1a83015