https://treeherder.mozilla.org/#/jobs?repo=try&revision=701a987d4fcddd24a4f67f08df6f9ee2b9b81f3a&selectedJob=161714284 https://treeherder.mozilla.org/logviewer.html#?job_id=161714284&repo=try&lineNumber=7140 This might be related to bug 1437712

The timeout happened at the place waiting for moveMouseOver promise [1]. CCing Patrick who are the reviewer for the patch that introduced this test in bug 1120111. [1] https://hg.mozilla.org/mozilla-central/file/6d8f470b2579/devtools/client/inspector/test/browser_inspector_highlighter-keybinding_03.js#l25

I started looking into this (honestly just run the test locally).

We have to wait for 'picker-stopped' event after doKeyPick() there before calling startPicker() in the next test case, otherwise the state of the picker will be something broken. From the failure test; info("Testing enter/return key as pick-node command"); yield doKeyPick({key: "VK_RETURN", options: {}}); is(inspector.selection.nodeFront.id, "another", "The #another node was selected. Passed."); In this case, we don't wait for 'picker-stopped'. Whereas; info("Testing escape key as cancel-picker command"); yield startPicker(toolbox); yield moveMouseOver("#ahoy"); yield doKeyStop({key: "VK_ESCAPE", options: {}}); is(inspector.selection.nodeFront.id, "another", "The #another DIV is still selected. Passed."); in this case we do wait for 'picker-stopped' in doKeyStop(). I haven't checked all code but as per the try in comment 0, this is the only place that we do care about.

A try based on the patch for bug 1193394; https://treeherder.mozilla.org/#/jobs?repo=try&revision=386ad407dad9835b899e576905ffd3eff4b25a56 And a try based on bare m-c; https://treeherder.mozilla.org/#/jobs?repo=try&revision=70ee94daa809379472d91043fc0a4069baa8502f

Gah, the patch causes timeout if the patch for bug 1193394 is not applied.

We have to wait the event in doKeyPick(). https://treeherder.mozilla.org/#/jobs?repo=try&revision=100c5ce01addc23872cfb32a29d622ac33d0f713 Try based on bare m-c; https://treeherder.mozilla.org/#/jobs?repo=try&revision=d99adf2ddcbd913d10696a3c5da1ca91ae5f45e4

Both tries look green. Patrick, Though I haven't dug into detail how the test proceeds before 'picker-stopped' event is dispatched, does it make sense to wait for 'picker-stopped' event in doKeyPick() along with 'inspector-updated' and 'new-node-front'? If it doesn't I'd like to leave devtools guys to investigate this issue.

(In reply to Hiroyuki Ikezoe (:hiro) from comment #7) > Both tries look green. > > Patrick, Though I haven't dug into detail how the test proceeds before > 'picker-stopped' event is dispatched, does it make sense to wait for > 'picker-stopped' event in doKeyPick() along with 'inspector-updated' and > 'new-node-front'? If it doesn't I'd like to leave devtools guys to > investigate this issue. Yes it does! This change looks great to me, and both tries are green, so I think we're good!

Thank you Patrick!

Hmm, MozReview doesn't allow to push review requests once the requests specified reviewers that does not accept review request even after the specified reviewers were dropped.

Leaving NI instead of review request.

Comment on attachment 8951781 [details] Bug 1437714 - Wait for 'picker-stopped' event before starting the new picker in the next test case. :jlast would you mind reviewing this on behalf of :pbro?

Comment on attachment 8951781 [details] Bug 1437714 - Wait for 'picker-stopped' event before starting the new picker in the next test case. https://reviewboard.mozilla.org/r/221046/#review228660 Thanks Hiro.

Pushed by hikezoe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5f7512c0b66a Wait for 'picker-stopped' event before starting the new picker in the next test case. r=pbro