Closed Bug 1235884 Opened 8 years ago Closed 8 years ago

Intermittent TEST-UNEXPECTED-FAIL | apps/system/test/marionette/notification_events_test.js | Notification events custom data available via mozSetMessageHandler

Categories

(Firefox OS Graveyard :: Gaia::System::Status bar, Utility tray, Notification, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mikehenrty, Assigned: u520299)

References

Details

(Keywords: intermittent-failure, Whiteboard: [MJS][systemsfe])

Attachments

(2 files, 1 obsolete file)

TEST-UNEXPECTED-FAIL | apps/system/test/marionette/notification_events_test.js | Notification events custom data available via mozSetMessageHandler
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 Context.<anonymous> (apps/system/test/marionette/notification_events_test.js:406:12)
    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)

https://treeherder.mozilla.org/logviewer.html#?job_id=3248555&repo=gaia
What is the failure rate on this?
Robert, this is your test, if you have any idea :)
Flags: needinfo?(robertbindar)
To be honest, I'm not sure if client.waitFor() and executeScript() are good friends
Hey! I'd first check if the test races on line 348, I.e. try add the eventlistner for mozchromenotificationevent before the notification is created, though it shouldn't race.

Also check if that event's name hasn't been changed in the meantime by someone and it forgot to update the test.

Let me know if any of these work, unfortunately I don't have access to a laptop right now, but I'll do my best to help.
Flags: needinfo?(robertbindar)
Assignee: nobody → tilman
Status: NEW → ASSIGNED
Depends on: 818000
Attachment #8711756 - Flags: review?(mhenretty)
Comment on attachment 8711756 [details] [review]
[gaia] tilmankamp:u1235884 > mozilla-b2g:master

I love the idea of using a fake app with a real mozSetMessageHandler (rather then the other way around which is what we had before). I left a couple of comments on github. Feel free to re-flag me when you've answered the questions are addressed it in the code.
Attachment #8711756 - Flags: review?(mhenretty)
Attachment #8711756 - Attachment is obsolete: true
Comment on attachment 8714326 [details] [review]
[gaia] tilmankamp:u1235884 > mozilla-b2g:master

This new approach of "direct" clicking (under use of notification.js helpers) simplifies the test code even more. It seems that neither deferred clicking nor app launch delaying is required anymore.
Please review another time. Thanks.

50/50 green test runs:
https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=d7e8b1ed33f32e2cf34859fb880ea6e8afa62605
Attachment #8714326 - Flags: review?(mhenretty)
Comment on attachment 8714326 [details] [review]
[gaia] tilmankamp:u1235884 > mozilla-b2g:master

I left a comment on github. Feel free to land once you have addressed it in some way. Overall the test looks to be much better. Thanks!
Attachment #8714326 - Flags: review?(mhenretty) → review+
Merged into master:
https://github.com/tilmankamp/gaia/commit/15b2633ca77d983c72d733ebc9400c48ad2fe567
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: