Closed
Bug 1081043
Opened 10 years ago
Closed 10 years ago
Reduce how often we check for crashes
Categories
(Remote Protocol :: Marionette, defect)
Remote Protocol
Marionette
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla36
People
(Reporter: davehunt, Assigned: davehunt)
References
Details
Attachments
(1 file, 1 obsolete file)
We currently have a decorator [1] that checks for crashes (by default only when an exception is raised). This is attached to _send_message in the client [2]. As seen in bug 1038868 we have an excessive amount of crash checks, which I suspect are caused by Waits, which we expect to raise exceptions a number of times before they complete. Most exceptions, including these commonly used in waits: NoSuchElementException, StaleElementException, ElementNotVisibleException are not indicative of a crash, so there's not a lot of value in checking for one. [1] http://hg.mozilla.org/mozilla-central/file/50b689feab5f/testing/marionette/client/marionette/decorators.py#l18 [2] http://hg.mozilla.org/mozilla-central/file/50b689feab5f/testing/marionette/client/marionette/marionette.py#l604
Assignee | ||
Comment 1•10 years ago
|
||
One thought would be to limit the check for crashes to exception of type MarionetteException (ignoring inheritance) and IOError. If there are other explicit exceptions that might indicate a crash then we can include those. Malini: Do you think this sounds reasonable?
Assignee: nobody → dave.hunt
Status: NEW → ASSIGNED
Flags: needinfo?(mdas)
Assignee | ||
Comment 2•10 years ago
|
||
In my testing this has considerably reduced the number of times we check for crashes. For example, running test_kill.py went from 155 checks to just 6.
Attachment #8503141 -
Flags: feedback?(mdas)
Comment 3•10 years ago
|
||
Comment on attachment 8503141 [details] [diff] [review] Only check for crashes if we get a MarionetteException or an instance of IOError. v1.0 Review of attachment 8503141 [details] [diff] [review]: ----------------------------------------------------------------- looks great, thanks! ::: testing/marionette/client/marionette/decorators.py @@ +33,5 @@ > > try: > return func(*args, **kwargs) > + except Exception as e: > + if type(e) is MarionetteException or isinstance(e, IOError): I think a socket.error may also be thrown
Attachment #8503141 -
Flags: feedback?(mdas) → feedback+
Updated•10 years ago
|
Flags: needinfo?(mdas)
Assignee | ||
Comment 4•10 years ago
|
||
Also catch socket.error, and took a slightly different approach.
Attachment #8503141 -
Attachment is obsolete: true
Attachment #8503970 -
Flags: review?(mdas)
Assignee | ||
Comment 5•10 years ago
|
||
Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=d49789096063 https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ba3b9d7dcd84
Comment 6•10 years ago
|
||
Comment on attachment 8503970 [details] [diff] [review] Only check for crashes if we get a MarionetteException or an instance of IOError. v1.1 Review of attachment 8503970 [details] [diff] [review]: ----------------------------------------------------------------- thanks!
Attachment #8503970 -
Flags: review?(mdas) → review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 7•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c52ec780e67
Keywords: checkin-needed
Comment 8•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2c52ec780e67
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Updated•1 year ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•