1034044 - get_device_manager relies on a session existing

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Zac C (:zac)]

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.

Comment 1

[Jonathan Griffin (:jgriffin)]

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?

Comment 2

[Zac C (:zac)]

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.

Comment 3

[Dave Hunt [:davehunt] [he/him] ⌚BST]

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.

Comment 4

[Andrew Halberstadt [:ahal]]

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.

Comment 5

[Andrew Halberstadt [:ahal]]

s/The/I

Comment 6

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Remove dependency on Marionette session from get_device_manager. v1.0

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.

Comment 7

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: test_crash.py

Test for simulating a crash. Run at least one test after this to test the crash recovery, or run with --repeat=1.

Comment 8

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Fix crash recovery in gaiatest.

This patch fixes the crash recovery in gaiatest, and can be used to test the marionette client patch.

Comment 9

[Jonathan Griffin (:jgriffin)]

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.

Comment 10

[Zac C (:zac)]

Did you do a Try run Dave?

Comment 11

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Not yet, I wanted a review first. I'll kick off a try run shortly.

Comment 12

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Try: https://tbpl.mozilla.org/?tree=Try&rev=50b4aab1b6dd

Comment 13

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Try was green.

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b76e4fdff4da

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/a0364999f064

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a0364999f064

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b76e4fdff4da