Closed Bug 1153832 Opened 7 years ago Closed 7 years ago

Introduce new dispatching technique to listener

Categories

(Testing :: Marionette, defect)

defect
Not set
normal

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: nobody → ato
Status: NEW → ASSIGNED
Attached file MozReview Request: bz://1153832/ato (obsolete) —
/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 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+
(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.
Blocks: 1100545
No longer blocks: 1100545
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
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
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 on attachment 8592758 [details]
MozReview Request: bz://1153832/ato

https://reviewboard.mozilla.org/r/7059/#review5963

Ship It!
Attachment #8592758 - Flags: review?(dburns) → review+
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
https://hg.mozilla.org/mozilla-central/rev/0b28f5ff48a6
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
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 → ---
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)
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
Flags: needinfo?(cbook)
https://hg.mozilla.org/mozilla-central/rev/2df6451d16bb
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Attachment #8592758 - Attachment is obsolete: true
Attachment #8620031 - Flags: review+
You need to log in before you can comment on or make changes to this bug.