Bug 774293 - Fix issues in devicemanager revealed by pyflakes;r=wlach
authorDominik Oepen <oepen@informatik.hu-berlin.de>
Tue, 14 Aug 2012 10:41:11 -0400
changeset 107775 22d3f22dd62e6255e998ce1a592c498caff9de27
parent 107774 2955526619552bf8fae7cf0b0bae188705cfc4e2
child 107776 227c29b3c0eed6434284a12b44e9ea5611774177
push id1490
push userakeybl@mozilla.com
push dateMon, 08 Oct 2012 18:29:50 +0000
treeherdermozilla-beta@f335e7dacdc1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswlach
bugs774293
milestone17.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 774293 - Fix issues in devicemanager revealed by pyflakes;r=wlach
build/mobile/devicemanager.py
build/mobile/devicemanagerADB.py
build/mobile/devicemanagerSUT.py
build/mobile/emulator.py
build/mobile/remoteautomation.py
--- a/build/mobile/devicemanager.py
+++ b/build/mobile/devicemanager.py
@@ -1,13 +1,12 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this file,
 # You can obtain one at http://mozilla.org/MPL/2.0/.
 
-import time
 import hashlib
 import socket
 import os
 import re
 import StringIO
 
 class FileError(Exception):
   " Signifies an error which occurs while doing a file operation."
--- a/build/mobile/devicemanagerADB.py
+++ b/build/mobile/devicemanagerADB.py
@@ -1,18 +1,18 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this file,
 # You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import subprocess
 from devicemanager import DeviceManager, DMError, _pop_last_line
 import re
 import os
-import sys
 import tempfile
+import time
 
 class DeviceManagerADB(DeviceManager):
 
   def __init__(self, host=None, port=20701, retrylimit=5, packageName='fennec',
                adbPath='adb', deviceSerial=None, deviceRoot=None):
     self.host = host
     self.port = port
     self.retrylimit = retrylimit
@@ -60,17 +60,17 @@ class DeviceManagerADB(DeviceManager):
     except DMError:
       pass
 
     # Can we run things as root? (currently not required)
     useRunAsTmp = self.useRunAs
     self.useRunAs = False
     try:
       self.verifyRoot()
-    except DMError, e:
+    except DMError:
       try:
         self.checkCmd(["root"])
         # The root command does not fail even if ADB cannot get
         # root rights (e.g. due to production builds), so we have
         # to check again ourselves that we have root now.
         self.verifyRoot()
       except DMError:
         if useRunAsTmp:
@@ -389,17 +389,19 @@ class DeviceManagerADB(DeviceManager):
     didKillProcess = False
     for (pid, name, user) in procs:
       if name == appname:
          args = ["shell", "kill"]
          if forceKill:
            args.append("-9")
          args.append(pid)
          p = self.runCmdAs(args)
-         didKillProcess = True
+         p.communicate()
+         if p.returncode == 0:
+             didKillProcess = True
 
     return didKillProcess
 
   # external function
   # returns:
   #  success: filecontents
   #  failure: None
   def catFile(self, remoteFile):
@@ -494,21 +496,21 @@ class DeviceManagerADB(DeviceManager):
     return self.getRemoteHash(remoteFile) == self.getLocalHash(localFile)
 
   # return the md5 sum of a remote file
   # internal function
   # returns:
   #  success: MD5 hash for given filename
   #  failure: None
   def getRemoteHash(self, filename):
-    data = p = self.runCmd(["shell", "ls", "-l", filename]).stdout.read()
+    data = self.runCmd(["shell", "ls", "-l", filename]).stdout.read()
     return data.split()[3]
 
   def getLocalHash(self, filename):
-    data = p = subprocess.Popen(["ls", "-l", filename], stdout=subprocess.PIPE).stdout.read()
+    data = subprocess.Popen(["ls", "-l", filename], stdout=subprocess.PIPE).stdout.read()
     return data.split()[4]
 
   # Internal method to setup the device root and cache its value
   def setupDeviceRoot(self):
     # if self.deviceRoot is already set, create it if necessary, and use it
     if self.deviceRoot:
       if not self.dirExists(self.deviceRoot):
         if not self.mkDir(self.deviceRoot):
@@ -688,17 +690,17 @@ class DeviceManagerADB(DeviceManager):
     if (directive == "process" or directive == "all"):
       ret["process"] = self.runCmd(["shell", "ps"]).stdout.read()
     if (directive == "systime" or directive == "all"):
       ret["systime"] = self.runCmd(["shell", "date"]).stdout.read()
     print ret
     return ret
 
   def runCmd(self, args):
-    # If we are not root but have run-as, and we're trying to execute 
+    # If we are not root but have run-as, and we're trying to execute
     # a shell command then using run-as is the best we can do
     finalArgs = [self.adbPath]
     if self.deviceSerial:
       finalArgs.extend(['-s', self.deviceSerial])
     if (not self.haveRoot and self.useRunAs and args[0] == "shell" and args[1] != "run-as"):
       args.insert(1, "run-as")
       args.insert(2, self.packageName)
     finalArgs.extend(args)
@@ -706,17 +708,17 @@ class DeviceManagerADB(DeviceManager):
 
   def runCmdAs(self, args):
     if self.useRunAs:
       args.insert(1, "run-as")
       args.insert(2, self.packageName)
     return self.runCmd(args)
 
   def checkCmd(self, args):
-    # If we are not root but have run-as, and we're trying to execute 
+    # If we are not root but have run-as, and we're trying to execute
     # a shell command then using run-as is the best we can do
     finalArgs = [self.adbPath]
     if self.deviceSerial:
       finalArgs.extend(['-s', self.deviceSerial])
     if (not self.haveRoot and self.useRunAs and args[0] == "shell" and args[1] != "run-as"):
       args.insert(1, "run-as")
       args.insert(2, self.packageName)
     finalArgs.extend(args)
--- a/build/mobile/devicemanagerSUT.py
+++ b/build/mobile/devicemanagerSUT.py
@@ -1,26 +1,23 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this file,
 # You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import select
 import socket
 import SocketServer
-import time, datetime
+import time
 import os
 import re
-import hashlib
 import posixpath
 import subprocess
 from threading import Thread
-import traceback
-import sys
 import StringIO
-from devicemanager import DeviceManager, DMError, FileError, NetworkTools, _pop_last_line
+from devicemanager import DeviceManager, FileError, NetworkTools, _pop_last_line
 import errno
 
 class AgentError(Exception):
   "SUTAgent-specific exception."
 
   def __init__(self, msg= '', fatal = False):
     self.msg = msg
     self.fatal = fatal
@@ -121,17 +118,16 @@ class DeviceManagerSUT(DeviceManager):
     '''
     a wrapper for _doCmds that loops up to self.retrylimit iterations.
     this allows us to move the retry logic outside of the _doCmds() to make it
     easier for debugging in the future.
     note that since cmdlist is a list of commands, they will all be retried if
     one fails.  this is necessary in particular for pushFile(), where we don't want
     to accidentally send extra data if a failure occurs during data transmission.
     '''
-    done = False
     while self.retries < self.retrylimit:
       try:
         self._doCmds(cmdlist, outputfile, timeout)
         return
       except AgentError, err:
         # re-raise error if it's fatal (i.e. the device got the command but
         # couldn't execute it). retry otherwise
         if err.fatal:
@@ -498,17 +494,17 @@ class DeviceManagerSUT(DeviceManager):
     if (self.debug >= 2): print "FIRE PROC: '" + appname + "'"
 
     if (self.processExist(appname) != None):
       print "WARNING: process %s appears to be running already\n" % appname
       if (failIfRunning):
         return None
 
     try:
-      data = self.runCmds([{ 'cmd': 'exec ' + appname }])
+      self.runCmds([{ 'cmd': 'exec ' + appname }])
     except AgentError:
       return None
 
     # The 'exec' command may wait for the process to start and end, so checking
     # for the process here may result in process = None.
     process = self.processExist(appname)
     if (self.debug >= 4): print "got pid: %s for process: %s" % (process, appname)
 
@@ -542,17 +538,17 @@ class DeviceManagerSUT(DeviceManager):
   # external function
   # returns:
   #  success: True
   #  failure: False
   def killProcess(self, appname, forceKill=False):
     if forceKill:
       print "WARNING: killProcess(): forceKill parameter unsupported on SUT"
     try:
-      data = self.runCmds([{ 'cmd': 'kill ' + appname }])
+      self.runCmds([{ 'cmd': 'kill ' + appname }])
     except AgentError:
       return False
 
     return True
 
   # external function
   # returns:
   #  success: tmpdir, string
@@ -635,17 +631,18 @@ class DeviceManagerSUT(DeviceManager):
     prompt = self.base_prompt + self.prompt_sep
     buffer = ''
 
     # expected return value:
     # <filename>,<filesize>\n<filedata>
     # or, if error,
     # <filename>,-1\n<error message>
     try:
-      data = self.runCmds([{ 'cmd': 'pull ' + remoteFile }])
+      # just send the command first, we read the response inline below
+      self.runCmds([{ 'cmd': 'pull ' + remoteFile }])
     except AgentError:
       return None
 
     # read metadata; buffer the rest
     metadata, sep, buffer = read_until_char('\n', buffer, 'could not find metadata')
     if not metadata:
       return None
     if self.debug >= 3:
@@ -875,17 +872,16 @@ class DeviceManagerSUT(DeviceManager):
   # external function
   # returns:
   #  success: status from test agent
   #  failure: None
   def reboot(self, ipAddr=None, port=30000):
     cmd = 'rebt'
 
     if (self.debug > 3): print "INFO: sending rebt command"
-    callbacksvrstatus = None
 
     if (ipAddr is not None):
     #create update.info file:
       try:
         destname = '/data/data/com.mozilla.SUTAgentAndroid/files/update.info'
         data = "%s,%s\rrebooting\r" % (ipAddr, port)
         self.runCmds([{ 'cmd': 'push %s %s' % (destname, len(data)),
                         'data': data }])
--- a/build/mobile/emulator.py
+++ b/build/mobile/emulator.py
@@ -263,20 +263,19 @@ class Emulator(object):
             using any availble local port, and return the local port.
         """
 
         s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
         s.bind(("", 0))
         local_port = s.getsockname()[1]
         s.close()
 
-        output = self._run_adb(['-s', 'emulator-%d' % self.port,
-                                'forward',
-                                'tcp:%d' % local_port,
-                                'tcp:%d' % remote_port])
+        self._run_adb(['-s', 'emulator-%d' % self.port, 'forward',
+                       'tcp:%d' % local_port,
+                       'tcp:%d' % remote_port])
 
         self.local_port = local_port
 
         return local_port
 
     def wait_for_port(self, timeout=300):
         assert(self.local_port)
         starttime = datetime.datetime.now()
--- a/build/mobile/remoteautomation.py
+++ b/build/mobile/remoteautomation.py
@@ -1,48 +1,47 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import time
-import sys
 import os
-import socket
 import automationutils
 import tempfile
 import shutil
+import subprocess
 
 from automation import Automation
-from devicemanager import DeviceManager, NetworkTools
+from devicemanager import NetworkTools
 
 class RemoteAutomation(Automation):
     _devicemanager = None
-    
+
     def __init__(self, deviceManager, appName = '', remoteLog = None):
         self._devicemanager = deviceManager
         self._appName = appName
         self._remoteProfile = None
         self._remoteLog = remoteLog
 
         # Default our product to fennec
         self._product = "fennec"
         Automation.__init__(self)
 
     def setDeviceManager(self, deviceManager):
         self._devicemanager = deviceManager
-        
+
     def setAppName(self, appName):
         self._appName = appName
 
     def setRemoteProfile(self, remoteProfile):
         self._remoteProfile = remoteProfile
 
     def setProduct(self, product):
         self._product = product
-        
+
     def setRemoteLog(self, logfile):
         self._remoteLog = logfile
 
     # 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:
@@ -83,21 +82,21 @@ class RemoteAutomation(Automation):
         except:
           print "WARNING: unable to remove directory: %s" % (dumpDir)
 
     def buildCommandLine(self, app, debuggerInfo, profileDir, testURL, extraArgs):
         # If remote profile is specified, use that instead
         if (self._remoteProfile):
             profileDir = self._remoteProfile
 
-        # Hack for robocop, if app & testURL == None and extraArgs contains the rest of the stuff, lets 
+        # Hack for robocop, if app & testURL == None and extraArgs contains the rest of the stuff, lets
         # assume extraArgs is all we need
         if app == "am" and extraArgs[0] == "instrument":
             return app, extraArgs
- 
+
         cmd, args = Automation.buildCommandLine(self, app, debuggerInfo, profileDir, testURL, extraArgs)
         # Remove -foreground if it exists, if it doesn't this just returns
         try:
             args.remove('-foreground')
         except:
             pass
 #TODO: figure out which platform require NO_EM_RESTART
 #        return app, ['--environ:NO_EM_RESTART=1'] + args
@@ -108,17 +107,17 @@ class RemoteAutomation(Automation):
         return nettools.getLanIp()
 
     def Process(self, cmd, stdout = None, stderr = None, env = None, cwd = None):
         if stdout == None or stdout == -1 or stdout == subprocess.PIPE:
           stdout = self._remoteLog
 
         return self.RProcess(self._devicemanager, cmd, stdout, stderr, env, cwd)
 
-    # be careful here as this inner class doesn't have access to outer class members    
+    # be careful here as this inner class doesn't have access to outer class members
     class RProcess(object):
         # device manager process
         dm = None
         def __init__(self, dm, cmd, stdout = None, stderr = None, env = None, cwd = None):
             self.dm = dm
             self.stdoutlen = 0
             self.proc = dm.launchProcess(cmd, stdout, cwd, env, True)
             if (self.proc is None):
@@ -157,26 +156,26 @@ class RemoteAutomation(Automation):
             time.sleep(1)
 
         @property
         def pid(self):
             hexpid = self.dm.processExist(self.procName)
             if (hexpid == None):
                 hexpid = "0x0"
             return int(hexpid, 0)
-    
+
         @property
         def stdout(self):
             t = self.dm.getFile(self.proc)
             if t == None: return ''
             tlen = len(t)
             retVal = t[self.stdoutlen:]
             self.stdoutlen = tlen
             return retVal.strip('\n').strip()
- 
+
         def wait(self, timeout = None):
             timer = 0
             interval = 5
 
             if timeout == None:
                 timeout = self.timeout
 
             while (self.dm.processExist(self.procName)):
@@ -185,11 +184,11 @@ class RemoteAutomation(Automation):
                 time.sleep(interval)
                 timer += interval
                 if (timer > timeout):
                     break
 
             if (timer >= timeout):
                 return 1
             return 0
- 
+
         def kill(self):
             self.dm.killProcess(self.procName)