Closed Bug 1293991 Opened 3 years ago Closed 3 years ago

Add remaining process/crash checks for methods of the Marionette class

Categories

(Testing :: Marionette, defect)

Product:
Testing
Component:
Marionette
Version:
Version 3
Type:
defect
Priority:
Not set

Tracking

(firefox50 fixed, firefox51 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla51
Tracking Flags:
Tracking Status
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

Details

Attachments

(1 file)

Bug 1293991 - Add remaining process/crash checks for methods of the Marionette class.
58 bytes, text/x-review-board-request
automatedtester
: review+
Details 
Just noticed that the restart() method doesn't have a `@do_process_check` decorator:

https://dxr.mozilla.org/mozilla-central/source/testing/marionette/client
/marionette_driver/marionette.py#1059

In case of non in_app restarts, we call `raise_for_port()` in the else case, which then could cause an IOError in case no connection can be made to the socket server.
Ok, so it's also `enforce_gecko_prefs()` which is not doing a proper process check given that it is restarting the instance and setting up a new socket connection.
Summary: Marionette.restart() isn't doing a process_check for not in_app triggered restarts → `enforce_gecko_prefs()` and `restart()` aren't doing a process_check
Well, __init__ had the same problem because it restarts the application in case of it was terminated.
Summary: `enforce_gecko_prefs()` and `restart()` aren't doing a process_check → Add remaining process/crash checks for methods of the Marionette class
Comment on attachment 8779729 [details]
Bug 1293991 - Add remaining process/crash checks for methods of the Marionette class.

https://reviewboard.mozilla.org/r/70658/#review68468
Attachment #8779729 - Flags: review?(dburns) → review+
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8e31e563790a
Add remaining process/crash checks for methods of the Marionette class. r=automatedtester
Sorry for that. The usage of the do_process_check decorator doesn't work for __init__() because at this time no Marionette instance is available. It means that as long as we start the application there, we might not catch a crash. The risk should be low because it's just some milliseconds.

I'm going to remove the decorator from __init__() which will make it work.
Flags: needinfo?(hskupin)
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/903c21dc1481
Add remaining process/crash checks for methods of the Marionette class. r=automatedtester
https://hg.mozilla.org/mozilla-central/rev/903c21dc1481
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
We would also need this test-only change on mozilla-aurora.
status-firefox50: --- → affected
Whiteboard: [checkin-needed-aurora]
You need to log in before you can comment on or make changes to this bug.