Skip to content
This repository has been archived by the owner on Aug 20, 2018. It is now read-only.

Commit

Permalink
Bug 810096 - Clean up internal variable usage in mozdevice;r=ahal
Browse files Browse the repository at this point in the history
  • Loading branch information
wlach committed Nov 13, 2012
1 parent f193623 commit 2061295
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 88 deletions.
6 changes: 3 additions & 3 deletions mozdevice/mozdevice/devicemanager.py
Expand Up @@ -33,7 +33,7 @@ def not_implemented(*args, **kwargs):

class DeviceManager:

logcatNeedsRoot = True
_logcatNeedsRoot = True

@abstractmethod
def shell(self, cmd, outputfile, env=None, cwd=None, timeout=None, root=False):
Expand Down Expand Up @@ -491,7 +491,7 @@ def recordLogcat(self):
#TODO: spawn this off in a separate thread/process so we can collect all the logcat information

# Right now this is just clearing the logcat so we can only see what happens after this call.
self.shellCheckOutput(['/system/bin/logcat', '-c'], root=self.logcatNeedsRoot)
self.shellCheckOutput(['/system/bin/logcat', '-c'], root=self._logcatNeedsRoot)

def getLogcat(self, filterSpecs=["dalvikvm:S", "ConnectivityService:S",
"WifiMonitor:S", "WifiStateTracker:S",
Expand All @@ -503,7 +503,7 @@ def getLogcat(self, filterSpecs=["dalvikvm:S", "ConnectivityService:S",
"""
cmdline = ["/system/bin/logcat", "-v", format, "-d"] + filterSpecs
lines = self.shellCheckOutput(cmdline,
root=self.logcatNeedsRoot).split('\r')
root=self._logcatNeedsRoot).split('\r')

for regex in filterOutRegexps:
lines = [line for line in lines if not re.search(regex, line)]
Expand Down
147 changes: 73 additions & 74 deletions mozdevice/mozdevice/devicemanagerADB.py
Expand Up @@ -12,39 +12,38 @@

class DeviceManagerADB(DeviceManager):

_haveRootShell = False
_haveSu = False
_useRunAs = False
_useDDCopy = False
_useZip = False
_logcatNeedsRoot = False
_pollingInterval = 0.01
_packageName = None
_tempDir = None
default_timeout = 300

def __init__(self, host=None, port=20701, retrylimit=5, packageName='fennec',
adbPath='adb', deviceSerial=None, deviceRoot=None, **kwargs):
self.host = host
self.port = port
self.retrylimit = retrylimit
self.retries = 0
self._sock = None
self.haveRootShell = False
self.haveSu = False
self.useRunAs = False
self.useDDCopy = False
self.useZip = False
self.logcatNeedsRoot = False
self.packageName = None
self.tempDir = None
self.deviceRoot = deviceRoot
self.default_timeout = 300
self.pollingInterval = 0.01

# the path to adb, or 'adb' to assume that it's on the PATH
self.adbPath = adbPath
self._adbPath = adbPath

# The serial number of the device to use with adb, used in cases
# where multiple devices are being managed by the same adb instance.
self.deviceSerial = deviceSerial
self._deviceSerial = deviceSerial

if packageName == 'fennec':
if os.getenv('USER'):
self.packageName = 'org.mozilla.fennec_' + os.getenv('USER')
self._packageName = 'org.mozilla.fennec_' + os.getenv('USER')
else:
self.packageName = 'org.mozilla.fennec_'
self._packageName = 'org.mozilla.fennec_'
elif packageName:
self.packageName = packageName
self._packageName = packageName

# verify that we can run the adb command. can't continue otherwise
self._verifyADB()
Expand Down Expand Up @@ -97,7 +96,7 @@ def shell(self, cmd, outputfile, env=None, cwd=None, timeout=None, root=False):
# always. :(

# If requested to run as root, check that we can actually do that
if root and not self.haveRootShell and not self.haveSu:
if root and not self._haveRootShell and not self._haveSu:
raise DMError("Shell command '%s' requested to run as root but root "
"is not available on this device. Root your device or "
"refactor the test/harness to not require root." %
Expand All @@ -106,7 +105,7 @@ def shell(self, cmd, outputfile, env=None, cwd=None, timeout=None, root=False):
# Getting the return code is more complex than you'd think because adb
# doesn't actually return the return code from a process, so we have to
# capture the output to get it
if root and not self.haveRootShell:
if root and not self._haveRootShell:
cmdline = "su -c \"%s\"" % self._escapedCommandLine(cmd)
else:
cmdline = self._escapedCommandLine(cmd)
Expand All @@ -120,9 +119,9 @@ def shell(self, cmd, outputfile, env=None, cwd=None, timeout=None, root=False):
cmdline = envstr + "; " + cmdline

# all output should be in stdout
args=[self.adbPath]
if self.deviceSerial:
args.extend(['-s', self.deviceSerial])
args=[self._adbPath]
if self._deviceSerial:
args.extend(['-s', self._deviceSerial])
args.extend(["shell", cmdline])
proc = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)

Expand All @@ -134,7 +133,7 @@ def shell(self, cmd, outputfile, env=None, cwd=None, timeout=None, root=False):
start_time = time.time()
ret_code = proc.poll()
while ((time.time() - start_time) <= timeout) and ret_code == None:
time.sleep(self.pollingInterval)
time.sleep(self._pollingInterval)
ret_code = proc.poll()
if ret_code == None:
proc.kill()
Expand Down Expand Up @@ -171,10 +170,10 @@ def pushFile(self, localname, destname):
raise DMError("Attempted to push a file (%s) to a directory (%s)!" %
(localname, destname))

if self.useRunAs:
if self._useRunAs:
remoteTmpFile = self.getTempDir() + "/" + os.path.basename(localname)
self._checkCmd(["push", os.path.realpath(localname), remoteTmpFile])
if self.useDDCopy:
if self._useDDCopy:
self.shellCheckOutput(["dd", "if=" + remoteTmpFile, "of=" + destname])
else:
self.shellCheckOutput(["cp", remoteTmpFile, destname])
Expand All @@ -200,7 +199,7 @@ def pushDir(self, localDir, remoteDir):
# one to get around this limitation
if not self.dirExists(remoteDir):
self.mkDirs(remoteDir+"/x")
if self.useZip:
if self._useZip:
try:
localZip = tempfile.mktemp() + ".zip"
remoteZip = remoteDir + "/adbdmtmp.zip"
Expand All @@ -213,7 +212,7 @@ def pushDir(self, localDir, remoteDir):
raise Exception("unzip failed, or permissions error")
except:
print "zip/unzip failure: falling back to normal push"
self.useZip = False
self._useZip = False
self.pushDir(localDir, remoteDir)
else:
tmpDir = tempfile.mkdtemp()
Expand Down Expand Up @@ -410,7 +409,7 @@ def _runPull(self, remoteFile, localFile):
if (len(errl) == 1):
if (((errl[0].find("Permission denied") != -1)
or (errl[0].find("does not exist") != -1))
and self.useRunAs):
and self._useRunAs):
# If we lack permissions to read but have run-as, then we should try
# to copy the file to a world-readable location first before attempting
# to pull it again.
Expand Down Expand Up @@ -536,11 +535,11 @@ def getTempDir(self):
"""
# Cache result to speed up operations depending
# on the temporary directory.
if not self.tempDir:
self.tempDir = self.getDeviceRoot() + "/tmp"
self.mkDir(self.tempDir)
if not self._tempDir:
self._tempDir = self.getDeviceRoot() + "/tmp"
self.mkDir(self._tempDir)

return self.tempDir
return self._tempDir

def getAppRoot(self, packageName):
"""
Expand All @@ -553,10 +552,10 @@ def getAppRoot(self, packageName):
return None

if (packageName and self.dirExists('/data/data/' + packageName)):
self.packageName = packageName
self._packageName = packageName
return '/data/data/' + packageName
elif (self.packageName and self.dirExists('/data/data/' + self.packageName)):
return '/data/data/' + self.packageName
elif (self._packageName and self.dirExists('/data/data/' + self._packageName)):
return '/data/data/' + self._packageName

# Failure (either not installed or not a recognized platform)
raise DMError("Failed to get application root for: %s" % packageName)
Expand Down Expand Up @@ -666,27 +665,27 @@ def _runCmd(self, args):
returns: returncode from subprocess.Popen
"""
finalArgs = [self.adbPath]
if self.deviceSerial:
finalArgs.extend(['-s', self.deviceSerial])
finalArgs = [self._adbPath]
if self._deviceSerial:
finalArgs.extend(['-s', self._deviceSerial])
# use run-as to execute commands as the package we're testing if
# possible
if not self.haveRootShell and self.useRunAs and args[0] == "shell" and args[1] != "run-as":
if not self._haveRootShell and self._useRunAs and args[0] == "shell" and args[1] != "run-as":
args.insert(1, "run-as")
args.insert(2, self.packageName)
args.insert(2, self._packageName)
finalArgs.extend(args)
return subprocess.Popen(finalArgs, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)

def _runCmdAs(self, args):
"""
Runs a command using adb
If self.useRunAs is True, the command is run-as user specified in self.packageName
If self._useRunAs is True, the command is run-as user specified in self._packageName
returns: returncode from subprocess.Popen
"""
if self.useRunAs:
if self._useRunAs:
args.insert(1, "run-as")
args.insert(2, self.packageName)
args.insert(2, self._packageName)
return self._runCmd(args)

# timeout is specified in seconds, and if no timeout is given,
Expand All @@ -700,12 +699,12 @@ def _checkCmd(self, args, timeout=None):
"""
# use run-as to execute commands as the package we're testing if
# possible
finalArgs = [self.adbPath]
if self.deviceSerial:
finalArgs.extend(['-s', self.deviceSerial])
if not self.haveRootShell and self.useRunAs and args[0] == "shell" and args[1] != "run-as":
finalArgs = [self._adbPath]
if self._deviceSerial:
finalArgs.extend(['-s', self._deviceSerial])
if not self._haveRootShell and self._useRunAs and args[0] == "shell" and args[1] != "run-as":
args.insert(1, "run-as")
args.insert(2, self.packageName)
args.insert(2, self._packageName)
finalArgs.extend(args)
if not timeout:
# We are asserting that all commands will complete in this time unless otherwise specified
Expand All @@ -716,7 +715,7 @@ def _checkCmd(self, args, timeout=None):
start_time = time.time()
ret_code = proc.poll()
while ((time.time() - start_time) <= timeout) and ret_code == None:
time.sleep(self.pollingInterval)
time.sleep(self._pollingInterval)
ret_code = proc.poll()
if ret_code == None:
proc.kill()
Expand All @@ -726,14 +725,14 @@ def _checkCmd(self, args, timeout=None):
def _checkCmdAs(self, args, timeout=None):
"""
Runs a command using adb and waits for command to finish
If self.useRunAs is True, the command is run-as user specified in self.packageName
If self._useRunAs is True, the command is run-as user specified in self._packageName
If timeout is specified, the process is killed after <timeout> seconds
returns: returncode from subprocess.Popen
"""
if (self.useRunAs):
if (self._useRunAs):
args.insert(1, "run-as")
args.insert(2, self.packageName)
args.insert(2, self._packageName)
return self._checkCmd(args, timeout)

def chmodDir(self, remoteDir, mask="777"):
Expand All @@ -759,9 +758,9 @@ def _verifyADB(self):
"""
Check to see if adb itself can be executed.
"""
if self.adbPath != 'adb':
if not os.access(self.adbPath, os.X_OK):
raise DMError("invalid adb path, or adb not executable: %s", self.adbPath)
if self._adbPath != 'adb':
if not os.access(self._adbPath, os.X_OK):
raise DMError("invalid adb path, or adb not executable: %s", self._adbPath)

try:
self._checkCmd(["version"])
Expand All @@ -772,20 +771,20 @@ def _verifyADB(self):

def _verifyDevice(self):
# If there is a device serial number, see if adb is connected to it
if self.deviceSerial:
if self._deviceSerial:
deviceStatus = None
proc = subprocess.Popen([self.adbPath, "devices"],
proc = subprocess.Popen([self._adbPath, "devices"],
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
for line in proc.stdout:
m = re.match('(.+)?\s+(.+)$', line)
if m:
if self.deviceSerial == m.group(1):
if self._deviceSerial == m.group(1):
deviceStatus = m.group(2)
if deviceStatus == None:
raise DMError("device not found: %s" % self.deviceSerial)
raise DMError("device not found: %s" % self._deviceSerial)
elif deviceStatus != "device":
raise DMError("bad status for device %s: %s" % (self.deviceSerial, deviceStatus))
raise DMError("bad status for device %s: %s" % (self._deviceSerial, deviceStatus))

# Check to see if we can connect to device and run a simple command
try:
Expand All @@ -806,7 +805,7 @@ def _isCpAvailable(self):
data = self._runCmd(["shell", "dd", "-"]).stdout.read()
if (re.search('unknown operand', data)):
print "'cp' not found, but 'dd' was found as a replacement"
self.useDDCopy = True
self._useDDCopy = True
return True
print "unable to execute 'cp' on device; consider installing busybox from Android Market"
return False
Expand All @@ -819,28 +818,28 @@ def _verifyRunAs(self):
# echoing conditions encountered by Fennec at run time.
# Check to see if run-as can be used here, by verifying a
# file copy via run-as.
self.useRunAs = False
self._useRunAs = False
devroot = self.getDeviceRoot()
if (self.packageName and self._isCpAvailable() and devroot):
if (self._packageName and self._isCpAvailable() and devroot):
tmpDir = self.getTempDir()

# The problem here is that run-as doesn't cause a non-zero exit code
# when failing because of a non-existent or non-debuggable package :(
runAsOut = self._runCmd(["shell", "run-as", self.packageName, "mkdir", devroot + "/sanity"]).communicate()[0]
runAsOut = self._runCmd(["shell", "run-as", self._packageName, "mkdir", devroot + "/sanity"]).communicate()[0]
if runAsOut.startswith("run-as:") and ("not debuggable" in runAsOut or "is unknown" in runAsOut):
raise DMError("run-as failed sanity check")

tmpfile = tempfile.NamedTemporaryFile()
self._checkCmd(["push", tmpfile.name, tmpDir + "/tmpfile"])
if self.useDDCopy:
self._checkCmd(["shell", "run-as", self.packageName, "dd", "if=" + tmpDir + "/tmpfile", "of=" + devroot + "/sanity/tmpfile"])
if self._useDDCopy:
self._checkCmd(["shell", "run-as", self._packageName, "dd", "if=" + tmpDir + "/tmpfile", "of=" + devroot + "/sanity/tmpfile"])
else:
self._checkCmd(["shell", "run-as", self.packageName, "cp", tmpDir + "/tmpfile", devroot + "/sanity"])
self._checkCmd(["shell", "run-as", self._packageName, "cp", tmpDir + "/tmpfile", devroot + "/sanity"])
if (self.fileExists(devroot + "/sanity/tmpfile")):
print "will execute commands via run-as " + self.packageName
self.useRunAs = True
print "will execute commands via run-as " + self._packageName
self._useRunAs = True
self._checkCmd(["shell", "rm", devroot + "/tmp/tmpfile"])
self._checkCmd(["shell", "run-as", self.packageName, "rm", "-r", devroot + "/sanity"])
self._checkCmd(["shell", "run-as", self._packageName, "rm", "-r", devroot + "/sanity"])

def _checkForRoot(self):
# Check whether we _are_ root by default (some development boards work
Expand All @@ -849,7 +848,7 @@ def _checkForRoot(self):
proc = self._runCmd(["shell", "id"])
data = proc.stdout.read()
if data.find('uid=0(root)') >= 0:
self.haveRootShell = True
self._haveRootShell = True
# if this returns true, we don't care about su
return

Expand All @@ -868,7 +867,7 @@ def _checkForRoot(self):

data = proc.stdout.read()
if data.find('uid=0(root)') >= 0:
self.haveSu = True
self._haveSu = True

def _isUnzipAvailable(self):
data = self._runCmdAs(["shell", "unzip"]).stdout.read()
Expand All @@ -888,9 +887,9 @@ def _verifyZip(self):
# If "zip" can be run locally, and "unzip" can be run remotely, then pushDir
# can use these to push just one file per directory -- a significant
# optimization for large directories.
self.useZip = False
self._useZip = False
if (self._isUnzipAvailable() and self._isLocalZipAvailable()):
print "will use zip to push directories"
self.useZip = True
self._useZip = True
else:
raise DMError("zip not available")

0 comments on commit 2061295

Please sign in to comment.