Closed
Bug 1081043
Opened 11 years ago
Closed 11 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•11 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•11 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•11 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•11 years ago
|
Flags: needinfo?(mdas)
Assignee | ||
Comment 4•11 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•11 years ago
|
||
Comment 6•11 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•11 years ago
|
Keywords: checkin-needed
Comment 7•11 years ago
|
||
Keywords: checkin-needed
Comment 8•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Updated•2 years ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•