1059478 - [mozrunner] Fix environment handling in DeviceRunner

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Andrew Halberstadt [:ahal]]

Currently there are several hacks in DeviceRunner to prevent the BaseRunner from doing its normal thing. E.g:
http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozrunner/mozrunner/base/device.py#36
http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozrunner/mozrunner/base/device.py#82

These are in place to work around limitations in the parent class. We should fix this up properly.

Comment 1

[Julien Pagès (:parkouss)]

Attachment: 1059478.patch

I looked at it, and tought it was good to make only a change in DeviceRunner, not in the parent class. The idea is to not pass any env parameter to the parent, so the real env used will be the copy of os.environ (https://dxr.mozilla.org/mozilla-central/source/testing/mozbase/mozrunner/mozrunner/base/runner.py#39). I think it should be fine - the self._env variable is still used to generate a part of the command line.

Comment 2

[Andrew Halberstadt [:ahal]]

Comment on attachment 8617416 [details] [diff] [review]
1059478.patch

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

This makes sense. I'd like to see some opt and debug emulator tests on try first though.

::: testing/mozbase/mozrunner/mozrunner/base/device.py
@@ +32,5 @@
>              'NO_EM_RESTART': '1', }
>  
>      def __init__(self, device_class, device_args=None, **kwargs):
>          process_log = tempfile.NamedTemporaryFile(suffix='pidlog')
> +        # the env will be passed to the device, it is not a not *real* env

nit: extra "not".

@@ +33,5 @@
>  
>      def __init__(self, device_class, device_args=None, **kwargs):
>          process_log = tempfile.NamedTemporaryFile(suffix='pidlog')
> +        # the env will be passed to the device, it is not a not *real* env
> +        self._env = dict(DeviceRunner.env)

Maybe we should call this self._device_env or something similar to make it slightly less confusing.

Comment 3

[Julien Pagès (:parkouss)]

Attachment: 1059478-2.patch

Eh, good idea; done. What should I run in push try here Andrew ?

Comment 4

[Andrew Halberstadt [:ahal]]

At least the full suite of B2G emulator tests, debug and opt. I'd also like to see crashreporting still working. It looks like Emulator debug mochitest-10 has an intermittent crash, maybe you can just retrigger that a bunch until it shows up.

Comment 5

[Julien Pagès (:parkouss)]

Thanks!

Pushed to try:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a161ba00be0c

Comment 6

[Pulsebot]

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

Comment 7

[Carsten Book [:Tomcat]]

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