Closed Bug 1034044 Opened 6 years ago Closed 6 years ago

get_device_manager relies on a session existing


(Testing :: Marionette, defect)

Not set


(Not tracked)



(Reporter: zcampbell, Assigned: davehunt)




(3 files)

Since bug 1019043 landed, get_device_manager in B2GTestCaseMixin relies on a marionette session being open, but generally the test framework relies on the device manager to be able to stop/start/manipulate the device into the state of being ready to start a session.

If get_device_manager could work without requiring a session (ie it could decipher the desktopb2g from the command line args) then we'd reduce this inter-dependency.
Can we pass a device_manager instance to the TestCase object (if appropriate), so we don't have to instantiate one in the test itself?
Blocks: 1033975
Dave, now you're back, do you have an ideas about this?

It blocks fixing of the crash handling of gaiatest (which is currently broken).

I've also worked around this in gaiatest but it's not pretty (see me for the commit), it'd definitely be better to fix this higher up in the framework.
Flags: needinfo?(dave.hunt)
The short-term solution could be to assume we're running against a device if we can't determine otherwise. Something like the following should work:

> def get_device_manager(self, *args, **kwargs):
>     capabilities = self.marionette.session and self.marionette.session_capabilities or {}
>     if not self._device_manager and capabilities.get('device') != 'desktop':
>         self._device_manager = get_dm(self.marionette, **kwargs)
>     return self._device_manager

Longer-term, I'm wondering if we should switch to the new B2GDeviceRunner in mozrunner for managing our devices. This doesn't have a dependency on Marionette, and should be able to replace some of our GaiaDevice methods. Andrew: I'm keen to hear your thoughts.
Flags: needinfo?(dave.hunt) → needinfo?(ahalberstadt)
Yeah, using B2GDeviceRunner seems like a good idea to me. It should be useable now, though there's a few patches I have yet to land that should fix some profile related setup problems (e.g bug 1036982).

For now, why can't you just create a new dm instance in that file instead of grabbing it from marionette? The wouldn't know about the detecting whether it's 'desktop' or not part though.
Flags: needinfo?(ahalberstadt)
Here's the short-term fix. This works well for me locally with a test I put together to cause a crash. The longer-term fix should be covered by bug 1038868 and bug 1038870.
Assignee: nobody → dave.hunt
Attachment #8456366 - Flags: review?(jgriffin)
Attached file
Test for simulating a crash. Run at least one test after this to test the crash recovery, or run with --repeat=1.
This patch fixes the crash recovery in gaiatest, and can be used to test the marionette client patch.
Comment on attachment 8456366 [details] [diff] [review]
Remove dependency on Marionette session from get_device_manager. v1.0

Review of attachment 8456366 [details] [diff] [review]:

I'm not sure how this will impact B2G desktop Gip runs, but if it works there, this seems like a good short-term fix.
Attachment #8456366 - Flags: review?(jgriffin) → review+
Did you do a Try run Dave?
Not yet, I wanted a review first. I'll kick off a try run shortly.
Flags: needinfo?(dave.hunt)
Try was green.
Flags: needinfo?(dave.hunt)
Keywords: checkin-needed
Closed: 6 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.