1233529 - Intermittent TEST-UNEXPECTED-FAIL | apps/homescreen/test/marionette/pinning_the_web_test.js | Homescreen - Pin the web Pinning a site from the browser context menu

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Homescreen, defect)

Description

[Michael Henretty [:mikehenrty][:mhenretty]]

Attachment: [screenshot] failure state

TEST-UNEXPECTED-FAIL | apps/homescreen/test/marionette/pinning_the_web_test.js | Homescreen - Pin the web Pinning a site from the browser context menu
Error: timeout exceeded!
    at Object.Client.waitForSync (node_modules/marionette-client/lib/marionette/client.js:760:16)
    at Object.Client.waitFor (node_modules/marionette-client/lib/marionette/client.js:726:60)
    at Object.MarionetteHelper.waitForElement (node_modules/marionette-helper/index.js:142:12)
    at Object.pinDialog (apps/system/test/marionette/lib/pinning_the_web.js:21:31)
    at Object.PinningTheWeb._clickPinContextMenu (apps/system/test/marionette/lib/pinning_the_web.js:119:43)
    at Object.openAndPinSite [as openAndPinSiteFromBrowser] (apps/system/test/marionette/lib/pinning_the_web.js:72:10)
    at Context.<anonymous> (apps/homescreen/test/marionette/pinning_the_web_test.js:54:13)
    at Test.MarionetteTest.run (node_modules/marionette-js-runner/lib/ui.js:25:31)
    at node_modules/marionette-js-runner/node_modules/promise/lib/core.js:33:15
    at flush (node_modules/marionette-js-runner/node_modules/promise/node_modules/asap/asap.js:27:13)

Comment 1

[Michael Henretty [:mikehenrty][:mhenretty]]

disabled in master: https://github.com/mozilla-b2g/gaia/commit/af827aed4b3f8cb2e4e4434ea7d7ea96e24878bc

Comment 2

[GitHub Autolander Bot]

Attachment: [gaia] albertopq:1233529-pin-intermitent > mozilla-b2g:master

Comment 3

[Alberto Pastor [:albertopq]]

Comment on attachment 8710468 [details] [review]
[gaia] albertopq:1233529-pin-intermitent > mozilla-b2g:master

This seems to help:

https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=2afa4d8c842a5ffc0b2d0f91046fa5c7f07f1bef&selectedJob=3417122

Comment 4

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8710468 [details] [review]
[gaia] albertopq:1233529-pin-intermitent > mozilla-b2g:master

I'm worried about this change, since the default timeout for this is currently 60000ms [1], so this is actually decreasing the timeout here (and I've seen this line in the pinning library failing on a number of tests with that timeout. If anything, I would say we should increase this timeout beyond 1 minute.

1.) https://github.com/mozilla-b2g/gaia/blob/9d8f052fe836bb83557c8a6c8d87b8fe1b066424/tests/jsmarionette/client/marionette-client/lib/marionette/client.js#L13

Comment 5

[Alberto Pastor [:albertopq]]

Comment on attachment 8710468 [details] [review]
[gaia] albertopq:1233529-pin-intermitent > mozilla-b2g:master

Yeah, you are right. I thought the timeout was 1 second. I increased it a little bit. It seems to help, but I think the last changes we did to the library (in another bug) helped this bug too.
100 green runs :)

Comment 6

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8710468 [details] [review]
[gaia] albertopq:1233529-pin-intermitent > mozilla-b2g:master

Ok, let's give it a try!

Also, make sure not to land the testing-only changes :)

Comment 7

[Alberto Pastor [:albertopq]]

master: https://github.com/mozilla-b2g/gaia/commit/dc72acb44096f012b6df6ee12efa192c5adc1e9a

Comment 8

[Michael Henretty [:mikehenrty][:mhenretty]]

Alberto, this change ended up having no effect. The additional timeout you add is after you call this.pinDialog [1], which is a getter that does a waitForElement on the pinDialog selector with the normal timeout. You should change this to the selector rather than using the getter. Conversely, you can increase the waitForElement timeout in the getter itself.

1.) https://github.com/mozilla-b2g/gaia/commit/dc72acb44096f012b6df6ee12efa192c5adc1e9a#diff-f87c6634998bcca584f441fdda6dd5c2R121

Comment 9

[GitHub Autolander Bot]

Attachment: [gaia] albertopq:1233529-pin-intermitent2 > mozilla-b2g:master

Comment 10

[Alberto Pastor [:albertopq]]

I'm going to carry over the r+, as is the same fix in the right place.

Follow up:
master: https://github.com/mozilla-b2g/gaia/commit/2107a9299ff50db25507a3ac455eebdbbed0da2e