Closed Bug 1154681 Opened 9 years ago Closed 9 years ago

Use static lookup in errors.py

Categories

(Testing :: Marionette Client and Harness, defect)

defect
Not set
normal

Tracking

(firefox40 fixed)

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: ato, Assigned: ato)

Details

(Keywords: pi-marionette-client)

Attachments

(1 file, 1 obsolete file)

errors.lookup currently iterates using anonymous functions every time you do a lookup.  This is inefficient and better practice would be to use a pre-defined lookup table.
Assignee: nobody → ato
Status: NEW → ASSIGNED
Attached file MozReview Request: bz://1154681/ato (obsolete) —
/r/7353 - Bug 1154681: Use static lookups in errors.py

Pull down this commit:

hg pull -r 93e29eb4cc59299ce873edd781e097e39c8ed529 https://reviewboard-hg.mozilla.org/gecko/
Attachment #8595310 - Flags: review?(james)
Attachment #8595310 - Flags: review?(james) → review+
Comment on attachment 8595310 [details]
MozReview Request: bz://1154681/ato

https://reviewboard.mozilla.org/r/7351/#review6111

Ship It!
Attachment #8595310 - Flags: review+ → review?(james)
Comment on attachment 8595310 [details]
MozReview Request: bz://1154681/ato

/r/7353 - Bug 1154681: Use static lookups in errors.py

Pull down this commit:

hg pull -r 9840c07fa813f7b44ea305669683efffd2dfe94c https://reviewboard-hg.mozilla.org/gecko/
Comment on attachment 8595310 [details]
MozReview Request: bz://1154681/ato

https://reviewboard.mozilla.org/r/7351/#review6113

::: testing/marionette/driver/marionette_driver/errors.py:186
(Diff revision 2)
> -    by_code = lambda exc: identifier in exc.code
> +    lookup = None

I'm not really sure that setting lookup to None here helps; I would just raise a ValueError in an "else" clause below.
Attachment #8595310 - Flags: review?(james)
Comment on attachment 8595310 [details]
MozReview Request: bz://1154681/ato

/r/7353 - Bug 1154681: Use static lookups in errors.py

Pull down this commit:

hg pull -r 990d55c253c89f822557023676f84371b3dfb3df https://reviewboard-hg.mozilla.org/gecko/
Attachment #8595310 - Flags: review?(james)
https://reviewboard.mozilla.org/r/7351/#review6119

> I'm not really sure that setting lookup to None here helps; I would just raise a ValueError in an "else" clause below.

Actually I've changed it so that it defaults to `by_string` so that in case where `identifier` is unknown, it will always end up returning `MarionetteException`.
Comment on attachment 8595310 [details]
MozReview Request: bz://1154681/ato

https://reviewboard.mozilla.org/r/7351/#review6121

Ship It!
Attachment #8595310 - Flags: review?(james) → review+
https://hg.mozilla.org/mozilla-central/rev/d90c697eead2
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Attachment #8595310 - Attachment is obsolete: true
Attachment #8620046 - Flags: review+
Product: Testing → Remote Protocol

Moving bugs for Marionette client due to component changes.

Component: Marionette → Marionette Client and Harness
Product: Remote Protocol → Testing
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: