988382 - mozrunner.remote.B2GRunner changes needed to support web-platform-tests

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[James Graham [:jgraham]]

Web-platform-tests has some different requirements compared to mochitest, so a small refactor of the runner to make fewer assumptions was required.

Comment 1

[James Graham [:jgraham]]

Attachment: Allow mozrunner.remote.B2GRunner to start without connecting to Marionette.

Comment 2

[James Graham [:jgraham]]

Comment on attachment 8397994 [details] [diff] [review]
Allow mozrunner.remote.B2GRunner to start without connecting to Marionette.

I haven't got this to fail locally but it seemed pretty busted on try [1], so any idea what I'm missing would be helpful.

[1] https://tbpl.mozilla.org/?tree=Try&rev=356bed415f45

Comment 3

[Andrew Halberstadt [:ahal]]

Comment on attachment 8397994 [details] [diff] [review]
Allow mozrunner.remote.B2GRunner to start without connecting to Marionette.

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

I don't really understand why you can't connect to marionette? Is it just because you don't need to? If that's the case then could we create the emulator object in B2GRunner instead of receiving it as an argument?

I think that orange might be a known intermittent. I re-triggered a couple times to verify.

::: testing/mozbase/mozrunner/mozrunner/remote.py
@@ +125,2 @@
>          self.marionette = marionette
> +        if marionette_port is None and self.marionette is not None:

nit: 'self.marionette is not None' is a double negative, just use 'if self.marionette:'. Same for the conditionals below.

Comment 4

[James Graham [:jgraham]]

(In reply to Andrew Halberstadt [:ahal] from comment #3)
> Comment on attachment 8397994 [details] [diff] [review]
> Allow mozrunner.remote.B2GRunner to start without connecting to Marionette.
> 
> Review of attachment 8397994 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I don't really understand why you can't connect to marionette? Is it just
> because you don't need to? If that's the case then could we create the
> emulator object in B2GRunner instead of receiving it as an argument?

So I don't have an emulator object at the moment; I've concentrated on running on a real device since that's what the certtest needs.

The reason I can't connect to marionette here is that the architecture of web-platform-tests splits what Mochitest does as a single step into two seperate concerns represented by two objects; a "Browser" object that is just responsible for launching the browser with the right settings and an Executor object that is actually responsible for managing the marionette connection and running the tests. The way that parallelisation work in the harness, these objects actually live in separate processes. It's the Browser object that owns the Runner instance so the Runner can't take responsibility for connecting to marionette.

> I think that orange might be a known intermittent. I re-triggered a couple
> times to verify.

It looks like it's broken :(
 
> ::: testing/mozbase/mozrunner/mozrunner/remote.py
> @@ +125,2 @@
> >          self.marionette = marionette
> > +        if marionette_port is None and self.marionette is not None:
> 
> nit: 'self.marionette is not None' is a double negative, just use 'if
> self.marionette:'. Same for the conditionals below.

I don't really understand. Why is it preferable to do the implicit type conversion rather than comparing to the specific sentinal value that indicates no object was passed in?

Comment 5

[James Graham [:jgraham]]

Attachment: Allow B2GRunner to be created without starting a marionette session

Comment 6

[James Graham [:jgraham]]

Comment on attachment 8398564 [details] [diff] [review]
Allow B2GRunner to be created without starting a marionette session

Try run is in progress [1], so obviously that also needs to pass.

[1] https://tbpl.mozilla.org/?tree=Try&rev=187378bff329

Comment 7

[James Graham [:jgraham]]

Try run was green :)

Comment 8

[Andrew Halberstadt [:ahal]]

Comment on attachment 8398564 [details] [diff] [review]
Allow B2GRunner to be created without starting a marionette session

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

::: testing/mozbase/mozrunner/mozrunner/remote.py
@@ +161,5 @@
>          self._setup_remote_profile()
>          # reboot device so it starts up with the proper profile
> +        if not self.emulator:
> +            t0 = time.time()
> +            self.dm.reboot(wait=True)

Please remove _reboot_device. Almost positive nothing else is using it.

@@ +177,5 @@
>          self.process_handler.run(timeout=timeout, outputTimeout=outputTimeout)
>  
>          # Set up port forwarding again for Marionette, since any that
>          # existed previously got wiped out by the reboot.
> +        if not self.emulator:

If we're going to use is None/is not None above, we might as well be consistent :). To answer your question, I find this way more readable, but apparently using is not None is part of PEP 8, so I concede. Please also update below.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6eccefd21990