1032136 - Make new mozrunner work with devices

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[James Graham [:jgraham]]

The new mozrunner release regresses support for running B2G on actual devices. This is needed for wptrunner / fxos certsuite.

Comment 1

[James Graham [:jgraham]]

Attachment: Make mozrunner 6 work for on-device B2G testing.

Initial cut at a patch. This is lightly tested and seems to work somewhat, but
may have remaining issues. 

jgriffin: it would be greta if you could use this along with HEAD wptrunner and report
if you get network connection issues.

Comment 2

[Andrew Halberstadt [:ahal]]

Comment on attachment 8447928 [details] [diff] [review]
Make mozrunner 6 work for on-device B2G testing.

Review of attachment 8447928 [details] [diff] [review]:
-----------------------------------------------------------------

Looks mostly good, but I don't think it's land-able just yet. Let me know if you want me to take this.

::: testing/mozbase/mozrunner/mozrunner/application.py
@@ +107,5 @@
>              sys.path.insert(0, self.bindir)
>  
> +        rv = find_executable(binary, os.pathsep.join(sys.path))
> +        if not rv:
> +            rv = find_executable(binary, os.environ.get("PATH"))

I think adding self.bindir to os.environ['PATH'] and only looking there should be good enough. Using sys.path in the first place was probably a bug.

@@ +116,5 @@
> +            # Assume this is a real device
> +            print("Restarting device")
> +            self.dm.reboot(wait=True)
> +            import subprocess
> +            print(subprocess.call(["adb", "wait-for-device"]))

This logic doesn't have anything b2g specific in it.. I think it could actually go in a reboot() method on the base Device class. We could then either use isinstance(Emulator) in runner.start() to differentiate or the Emulator subclass could override it to do nothing.

That still seems yucky, but this current assumption isn't really valid. Also, passing wait=True into dm.reboot calls 'wait-for-device' already.

::: testing/mozbase/mozrunner/mozrunner/base/device.py
@@ +60,5 @@
>          return cmd
>  
>      def start(self, *args, **kwargs):
> +        self.device.connect()
> +        print("Installing device profile")

Leftover debug?

::: testing/mozbase/mozrunner/mozrunner/devices/emulator.py
@@ +126,3 @@
>          """
> +        if not self.serial:
> +            self.start()

I mentioned previously that this will break Marionette's 'connect_to_running_emulator' option. If you want I can look into not breaking this.

Comment 3

[Andrew Halberstadt [:ahal]]

Attachment: Make mozrunner 6 work for on-device B2G testing.

I think this should work.. though haven't tested (so as discussed on irc, please make sure it works before giving the r+). I was trying to avoid any modifications to marionette because last time we bumped mozrunner dependencies there it was a massive pain to update for davehunt. This patch seems like a not-too-terrible way of maintaining backwards compatibility there.

Comment 4

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8448028 [details] [diff] [review]
Make mozrunner 6 work for on-device B2G testing.

Review of attachment 8448028 [details] [diff] [review]:
-----------------------------------------------------------------

This works as well as mozrunner 5.37 with the cert suite, but not as well as 5.37.1.  What needs to change:

::: testing/mozbase/mozrunner/mozrunner/base/device.py
@@ +65,2 @@
>              self.device.start()
> +        self.device.connect()

Need another call to self.device.wait_for_net() here, like http://hg.mozilla.org/releases/mozilla-aurora/file/6cd5f4c61978/testing/mozbase/mozrunner/mozrunner/remote.py#l170

::: testing/mozbase/mozrunner/mozrunner/devices/base.py
@@ +185,5 @@
> +            proc = subprocess.Popen([self.dm._adbPath, 'shell', '/system/bin/netcfg'], stdout=subprocess.PIPE)
> +            proc.stdout.readline() # ignore first line
> +            line = proc.stdout.readline()
> +            while line != "":
> +                if (re.search(r'UP\s+(?:[0-9]{1,3}\.){3}[0-9]{1,3}', line)):

Need the regex change from http://hg.mozilla.org/releases/mozilla-aurora/file/6cd5f4c61978/testing/mozbase/mozrunner/mozrunner/remote.py#l295

Comment 5

[James Graham [:jgraham]]

Attachment: Make mozrunner 6 work for on-device B2G testing.

Updated with jgriffin's changes and so that it actually works on a device.

On thing I have noticed is that in start() the wait_for_net() after rebooting
produces a lot of "device not found" errors, which is a little bit surprising
because reboot() is supposed to have done wait-for-device. I wonder if there is a
race condition here where we don't actually reboot until after wait-for-device
has returned. I have no idea why this would affect this patch when it didn't seem
to before the mozrunner upgrade, however.

Comment 6

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8448702 [details] [diff] [review]
Make mozrunner 6 work for on-device B2G testing.

Review of attachment 8448702 [details] [diff] [review]:
-----------------------------------------------------------------

This works well for me with my 1.3 Hamachi.  I didn't get any timeouts when restarting the phone due to test errors.   I also didn't see any "device not found" errors, so I'm not sure what was going on there.

I'll let ahal do the real review.

Comment 7

[Andrew Halberstadt [:ahal]]

Comment on attachment 8448702 [details] [diff] [review]
Make mozrunner 6 work for on-device B2G testing.

Review of attachment 8448702 [details] [diff] [review]:
-----------------------------------------------------------------

Lgtm!

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/88419653aac6