Closed Bug 617815 Opened 9 years ago Closed 9 years ago

remote testing needs environment variables

Categories

(Testing :: General, defect)

ARM
Android
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aki, Assigned: cmtalbert)

References

Details

Attachments

(2 files, 2 obsolete files)

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.
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?
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.
Attachment #496476 - Flags: feedback?(jmaher)
Sorry for the spam, for anyone testing this on android, you need agent version .93 or higher
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?
Attachment #496476 - Flags: feedback?(jmaher) → feedback-
Blocks: 561908
Blocks: 610600
Attached patch Final set of changes (obsolete) — Splinter Review
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.
Assignee: nobody → ctalbert
Attachment #496476 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #498165 - Flags: review?(jmaher)
Comment on attachment 498165 [details] [diff] [review]
Final set of changes

this looks good clint.
Attachment #498165 - Flags: review?(jmaher) → review+
superset of original patch, with a lot of pull and additional environment variables changes.
Attachment #498165 - Attachment is obsolete: true
Attachment #498198 - Flags: review?(ctalbert)
Comment on attachment 498198 [details] [diff] [review]
updated with many additional devicemanager changes (2.0)

Looks good.
Attachment #498198 - Flags: review?(ctalbert) → review+
More final changes taking the remaining changes from mcote on but 614173
Attachment #498224 - Flags: review?(jmaher)
Attachment #498224 - Flags: feedback?(mcote)
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
Attachment #498224 - Flags: review?(jmaher) → review+
Attachment #498224 - Flags: feedback?(mcote) → feedback+
Landed http://hg.mozilla.org/mozilla-central/rev/18004ceae9ce
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Component: New Frameworks → General
You need to log in before you can comment on or make changes to this bug.