617815 - remote testing needs environment variables

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Aki Sasaki (not active)]

Namely, MOZ_CRASHREPORTER_NO_REPORT=1, but there may be others later.
Not having this set a) hangs tests and b) prevents us from getting crash stacks later.  (a) is a more of an immediate need to fix than (b).

I'm currently putting MOZ_CRASHREPORTER_NO_REPORT in the talos run via

diff -r 7a7ec7648e09 remote.config
--- a/remote.config     Tue Nov 23 12:41:06 2010 -0500
+++ b/remote.config     Wed Dec 08 17:13:38 2010 -0800
@@ -68,6 +68,7 @@
 # Environment variables to set during test (use env: {} for none)
 env :
   NO_EM_RESTART : 1
+  MOZ_CRASHREPORTER_NO_REPORT : 1

but if there's a different way to get it into devicemanager, that should be ok too.

Comment 1

[cmtalbert]

So I'm thinking that we simply add another parameter to the devicemanager launchProcess/fireProcess API that allows us to specify env variables for the call.  Then we change the callers of this to add it as needed.  This means every remote-test test runner is going to have to change.

I'll have a patch for devicemanager, reftest, and mochitest in a little bit.  Jmaher, can you handle talos?

Comment 2

[cmtalbert]

Attachment: first pass at devicemanager changes and mochitest support

Joel, take a look at this.  I've edited devicemanager to use the env variables so that the sutagent can understand them (and so that the syntax of env variables will be os agnostic). I've also changed mochitest to support this, and as a result of this I found a bug in special powers, so I moved that code so that it will work remoted.

RemoteReftest actually uses automation.py::environment and needs to use that one to start the webserver (as opposed to the remoteautomation::environment). So, more tweaking needs to be done on reftest to support this.

Comment 3

[cmtalbert]

Sorry for the spam, for anyone testing this on android, you need agent version .93 or higher

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 496476 [details] [diff] [review]
first pass at devicemanager changes and mochitest support

>diff --git a/build/mobile/devicemanager.py b/build/mobile/devicemanager.py
>--- a/build/mobile/devicemanager.py
>+++ b/build/mobile/devicemanager.py
>@@ -390,21 +390,24 @@ class DeviceManager:

>-  def launchProcess(self, cmd, outputFile = "process.txt", cwd = ''):
>+  def launchProcess(self, cmd, outputFile = "process.txt", cwd = '', env = ''):
>     cmdline = subprocess.list2cmdline(cmd)
>     if (outputFile == "process.txt" or outputFile == None):
>       outputFile = self.getDeviceRoot() + '/' + "process.txt"
>       cmdline += " > " + outputFile
>+    
>+    # Prepend our env to the command 
>+    cmdline = ('%s ' % self.formatEnvString(env)) + cmdline
> 
>     self.fireProcess(cmdline)
>     return outputFile
>   


I don't see where we are calling envrun vs exec/run/fire?



>diff --git a/build/mobile/remoteautomation.py b/build/mobile/remoteautomation.py
>--- a/build/mobile/remoteautomation.py
>+++ b/build/mobile/remoteautomation.py
>@@ -67,16 +67,31 @@ class RemoteAutomation(Automation):

>+    # Set up what we need for the remote environment
>+    def environment(self, env = None, xrePath = None, crashreporter = True):
>+        # Because we are running remote, we don't want to mimic the local env
>+        # so no copying of os.environ
>+        if env is None:
>+            env = {}
>+
>+        if crashreporter:
>+            env['MOZ_CRASHREPORTER_NO_REPORT'] = '1'
>+            env['MOZ_CRASHREPORTER'] = '1'
>+        else:
>+            env['MOZ_CRASHREPORTER_DISABLE'] = '1'
>+
>+        return env
>+

Where do we call this RemoteAutomation::environment() ?  What if we pass in env vars via the command line?

Comment 5

[cmtalbert]

Attachment: Final set of changes

This is the set of changes to get mochitest and reftest to use the environment settings to disable the display the crashporter.

There were no changes required to the reftest specific python channels.

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 498165 [details] [diff] [review]
Final set of changes

this looks good clint.

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: updated with many additional devicemanager changes (2.0)

superset of original patch, with a lot of pull and additional environment variables changes.

Comment 8

[cmtalbert]

Comment on attachment 498198 [details] [diff] [review]
updated with many additional devicemanager changes (2.0)

Looks good.

Comment 9

[cmtalbert]

Attachment: Final changes combined with mcote's latest

More final changes taking the remaining changes from mcote on but 614173

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 498224 [details] [diff] [review]
Final changes combined with mcote's latest

>diff --git a/build/mobile/devicemanager.py b/build/mobile/devicemanager.py
>--- a/build/mobile/devicemanager.py
>+++ b/build/mobile/devicemanager.py
>@@ -884,16 +910,19 @@ class DeviceManager:
> 
>       if (self.debug > 3): print "updateApp using command: " + str(cmd)
> 
>       # Set up our callback server
>       callbacksvr = callbackServer(ip, port, self.debug)
>       data = self.sendCMD([cmd])
>       status = callbacksvr.disconnect()
>       if (self.debug > 3): print "got status back: " + str(status)
>+    else:
>+      if (self.debug > 3): print "updateApp using command: " + str(cmd)
>+      status = self.sendCMD([cmd])
> 
>     return status
> 
>   """
>     return the current time on the device
>   """
>   def getCurrentTime(self):
>     data = self.sendCMD(['clok'])

it seems like we are duplicating a debug print statement here.  r+ with that cleaned up

Comment 11

[cmtalbert]

Landed http://hg.mozilla.org/mozilla-central/rev/18004ceae9ce