Closed
Bug 1153832
Opened 9 years ago
Closed 9 years ago
Introduce new dispatching technique to listener
Categories
(Remote Protocol :: Marionette, defect)
Remote Protocol
Marionette
Tracking
(firefox40 fixed)
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: ato, Assigned: ato)
Details
(Keywords: pi-marionette-server)
Attachments
(1 file, 1 obsolete file)
Bug 1107706 consciously did not make any (substantial) changes to the listener, but the ListenerProxy added facilitates the same style of dispatching technique there. Currently when we make a call to a routine in listener space we need to do this.listener.get({url: "https://mozilla.org/"}) which the receiving function then is required to unpack from msg.json. By wrapping command implementations in listener in a try…catch block that unpacks the arguments and apply them to the wrapped function, we could reduce this to this.listener.get("https://mozilla.org/"). By looking at the type of the arguments we can stay backwards compatible, i.e. that we don't have to upgrade all functions at once.
Assignee | ||
Updated•9 years ago
|
Keywords: ateam-marionette-server
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → ato
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•9 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=468d93cd1d25
Assignee | ||
Comment 2•9 years ago
|
||
/r/7061 - Bug 1153832: New dispatch style framework in Marionette listener Pull down this commit: hg pull -r 43ef240735099221cbbddb753eaf9e2802a4de1d https://reviewboard-hg.mozilla.org/gecko/
Attachment #8592758 -
Flags: review?(dburns)
Comment 3•9 years ago
|
||
Comment on attachment 8592758 [details] MozReview Request: bz://1153832/ato https://reviewboard.mozilla.org/r/7059/#review5871 Ship It! ::: testing/marionette/listener.js (Diff revision 1) > - sendError("Error setting cookie", 13, null, msg.json.command_id); > + sendError(new UnknownError("Error setting cookie"), msg.json.command_id); This should be UnableToSetCookie when you do your follow up
Attachment #8592758 -
Flags: review?(dburns) → review+
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to David Burns :automatedtester from comment #3) > ::: testing/marionette/listener.js > (Diff revision 1) > > - sendError("Error setting cookie", 13, null, msg.json.command_id); > > + sendError(new UnknownError("Error setting cookie"), msg.json.command_id); > > This should be UnableToSetCookie when you do your follow up Filed bug 1154757 and made bug 1100545 track fixing up errors.
Assignee | ||
Comment 5•9 years ago
|
||
Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/043a824dd7b7
Comment 6•9 years ago
|
||
Backed out for mass-fail on Mn test suites. https://hg.mozilla.org/integration/mozilla-inbound/rev/e026141c2ce9 https://treeherder.mozilla.org/logviewer.html#?job_id=8886437&repo=mozilla-inbound
Assignee | ||
Comment 7•9 years ago
|
||
There was a bug in sendKeysToElement where the arguments to sendError hadn't been updated. Pushed a new try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b312cadcacdc
Comment 10•9 years ago
|
||
Thanks Pulsebot. Now for some context, it was hitting Gip(a) failures like yesterday's landing. https://treeherder.mozilla.org/logviewer.html#?job_id=8937623&repo=mozilla-inbound
Assignee | ||
Comment 11•9 years ago
|
||
Comment on attachment 8592758 [details] MozReview Request: bz://1153832/ato /r/7061 - Bug 1153832: New dispatch style framework in Marionette listener Pull down this commit: hg pull -r 3d643839885fcf2f2eb335942b1fe00ecb29ab02 https://reviewboard-hg.mozilla.org/gecko/
Attachment #8592758 -
Flags: review+ → review?(dburns)
Comment 12•9 years ago
|
||
Comment on attachment 8592758 [details] MozReview Request: bz://1153832/ato https://reviewboard.mozilla.org/r/7059/#review5963 Ship It!
Attachment #8592758 -
Flags: review?(dburns) → review+
Assignee | ||
Comment 13•9 years ago
|
||
Fixed the Gip(a) failures. It was hilarious. Ask me about it over a beer. Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/0b28f5ff48a6
Comment 14•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0b28f5ff48a6
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Comment 15•9 years ago
|
||
sorry had to back this out for frequent Asan dt failures started with this push (its also in the try run from comment #7 like https://treeherder.mozilla.org/logviewer.html#?job_id=9057650&repo=mozilla-inbound
Status: RESOLVED → REOPENED
Flags: needinfo?(ato)
Resolution: FIXED → ---
Assignee | ||
Comment 16•9 years ago
|
||
I believe Asan dt tests were already flaky before I pushed this commit. If you examine the log of bug 1073442, it looks like the timeout issue is older than this. As far as I can tell, Asan dt tests don't use Marionette at all. It seems to time out in running scripts/scripts/desktop_unittest.py. Does that invoke Marionette at all? How can I run that locally?
Flags: needinfo?(ato) → needinfo?(cbook)
Assignee | ||
Comment 17•9 years ago
|
||
Tomcat: This is test results from the push after your backout of this patch, which shows the problem persists: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=07e55dfe0411
Comment 18•9 years ago
|
||
Backout: https://hg.mozilla.org/mozilla-central/rev/a8e2d2de8d8b
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(cbook)
https://hg.mozilla.org/mozilla-central/rev/2df6451d16bb
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 21•9 years ago
|
||
Attachment #8592758 -
Attachment is obsolete: true
Attachment #8620031 -
Flags: review+
Assignee | ||
Comment 22•9 years ago
|
||
Updated•10 months ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•