Bug 1440674 - Simplify some code in remoteautomation.py; r=jmaher
authorGeoff Brown <gbrown@mozilla.com>
Fri, 23 Feb 2018 09:47:57 -0700
changeset 405096 b494bb88b01ca0c28db17cb0f5fbaf56ffe561eb
parent 405095 a5fdaf6da588414deb6eda32d1f27345dfa7a17e
child 405097 e379efa4ff54e48138e28de17e2d7b59e9724789
push id33502
push userarchaeopteryx@coole-files.de
push dateSat, 24 Feb 2018 00:59:26 +0000
treeherdermozilla-central@1056e048072c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjmaher
bugs1440674
milestone60.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 1440674 - Simplify some code in remoteautomation.py; r=jmaher remove RemoteAutomation._product and product options; remove some unused code paths; remove "blobber" references; remove incorrect and/or unhelpful comments; s/_devicemanager/_dm/ (shorter, improve readability)
build/mobile/remoteautomation.py
layout/tools/reftest/reftestcommandline.py
layout/tools/reftest/remotereftest.py
testing/mochitest/mochitest_options.py
testing/mochitest/runrobocop.py
testing/mochitest/runtestsremote.py
--- a/build/mobile/remoteautomation.py
+++ b/build/mobile/remoteautomation.py
@@ -5,62 +5,52 @@
 import datetime
 import glob
 import time
 import re
 import os
 import posixpath
 import tempfile
 import shutil
-import subprocess
 import sys
 
 from automation import Automation
 from mozdevice import DMError, DeviceManager
 from mozlog import get_default_logger
 from mozscreenshot import dump_screen
 import mozcrash
 
 # signatures for logcat messages that we don't care about much
 fennecLogcatFilters = [ "The character encoding of the HTML document was not declared",
                         "Use of Mutation Events is deprecated. Use MutationObserver instead.",
                         "Unexpected value from nativeGetEnabledTags: 0" ]
 
 class RemoteAutomation(Automation):
     _devicemanager = None
 
-    # Part of a hack for Robocop: "am COMMAND" is handled specially if COMMAND
-    # is in this set. See usages below.
-    _specialAmCommands = ('instrument', 'start')
-
     def __init__(self, deviceManager, appName = '', remoteLog = None,
                  processArgs=None):
-        self._devicemanager = deviceManager
+        self._dm = deviceManager
         self._appName = appName
         self._remoteProfile = None
         self._remoteLog = remoteLog
         self._processArgs = processArgs or {};
 
-        # Default our product to fennec
-        self._product = "fennec"
         self.lastTestSeen = "remoteautomation.py"
         Automation.__init__(self)
 
     def setDeviceManager(self, deviceManager):
-        self._devicemanager = deviceManager
+        self._dm = 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, debugger=False, lsanPath=None, ubsanPath=None):
         # Because we are running remote, we don't want to mimic the local env
         # so no copying of os.environ
         if env is None:
@@ -106,17 +96,17 @@ class RemoteAutomation(Automation):
             If maxTime seconds elapse or no output is detected for timeout
             seconds, kill the process and fail the test.
         """
         proc.utilityPath = utilityPath
         # maxTime is used to override the default timeout, we should honor that
         status = proc.wait(timeout = maxTime, noOutputTimeout = timeout)
         self.lastTestSeen = proc.getLastTestSeen
 
-        topActivity = self._devicemanager.getTopActivity()
+        topActivity = self._dm.getTopActivity()
         if topActivity == proc.procName:
             print "Browser unexpectedly found running. Killing..."
             proc.kill(True)
         if status == 1:
             if maxTime:
                 print "TEST-UNEXPECTED-FAIL | %s | application ran for longer than " \
                       "allowed maximum time of %s seconds" % (self.lastTestSeen, maxTime)
             else:
@@ -128,75 +118,73 @@ class RemoteAutomation(Automation):
 
         return status
 
     def deleteANRs(self):
         # empty ANR traces.txt file; usually need root permissions
         # we make it empty and writable so we can test the ANR reporter later
         traces = "/data/anr/traces.txt"
         try:
-            self._devicemanager.shellCheckOutput(['echo', '', '>', traces], root=True,
-                                                 timeout=DeviceManager.short_timeout)
-            self._devicemanager.shellCheckOutput(['chmod', '666', traces], root=True,
-                                                 timeout=DeviceManager.short_timeout)
+            self._dm.shellCheckOutput(['echo', '', '>', traces], root=True,
+                                       timeout=DeviceManager.short_timeout)
+            self._dm.shellCheckOutput(['chmod', '666', traces], root=True,
+                                       timeout=DeviceManager.short_timeout)
         except DMError:
             print "Error deleting %s" % traces
             pass
 
     def checkForANRs(self):
         traces = "/data/anr/traces.txt"
-        if self._devicemanager.fileExists(traces):
+        if self._dm.fileExists(traces):
             try:
-                t = self._devicemanager.pullFile(traces)
+                t = self._dm.pullFile(traces)
                 if t:
                     stripped = t.strip()
                     if len(stripped) > 0:
                         print "Contents of %s:" % traces
                         print t
                 # Once reported, delete traces
                 self.deleteANRs()
             except DMError:
                 print "Error pulling %s" % traces
             except IOError:
                 print "Error pulling %s" % traces
         else:
             print "%s not found" % traces
 
     def deleteTombstones(self):
-        # delete any existing tombstone files from device
+        # delete any tombstone files from device
         tombstones = "/data/tombstones/*"
         try:
-            self._devicemanager.shellCheckOutput(['rm', '-r', tombstones], root=True,
-                                                 timeout=DeviceManager.short_timeout)
+            self._dm.shellCheckOutput(['rm', '-r', tombstones], root=True,
+                                       timeout=DeviceManager.short_timeout)
         except DMError:
             # This may just indicate that the tombstone directory is missing
             pass
 
     def checkForTombstones(self):
         # pull any tombstones from device and move to MOZ_UPLOAD_DIR
         remoteDir = "/data/tombstones"
-        blobberUploadDir = os.environ.get('MOZ_UPLOAD_DIR', None)
-        if blobberUploadDir:
-            if not os.path.exists(blobberUploadDir):
-                os.mkdir(blobberUploadDir)
-            if self._devicemanager.dirExists(remoteDir):
-                # copy tombstone files from device to local blobber upload directory
+        uploadDir = os.environ.get('MOZ_UPLOAD_DIR', None)
+        if uploadDir:
+            if not os.path.exists(uploadDir):
+                os.mkdir(uploadDir)
+            if self._dm.dirExists(remoteDir):
+                # copy tombstone files from device to local upload directory
                 try:
-                    self._devicemanager.shellCheckOutput(['chmod', '777', remoteDir], root=True,
-                                                 timeout=DeviceManager.short_timeout)
-                    self._devicemanager.shellCheckOutput(['chmod', '666', os.path.join(remoteDir, '*')], root=True,
-                                                 timeout=DeviceManager.short_timeout)
-                    self._devicemanager.getDirectory(remoteDir, blobberUploadDir, False)
+                    self._dm.shellCheckOutput(['chmod', '777', remoteDir], root=True,
+                                               timeout=DeviceManager.short_timeout)
+                    self._dm.shellCheckOutput(['chmod', '666', os.path.join(remoteDir, '*')],
+                                               root=True, timeout=DeviceManager.short_timeout)
+                    self._dm.getDirectory(remoteDir, uploadDir, False)
                 except DMError:
                     # This may just indicate that no tombstone files are present
                     pass
                 self.deleteTombstones()
-                # add a .txt file extension to each tombstone file name, so
-                # that blobber will upload it
-                for f in glob.glob(os.path.join(blobberUploadDir, "tombstone_??")):
+                for f in glob.glob(os.path.join(uploadDir, "tombstone_??")):
                     # add a unique integer to the file name, in case there are
                     # multiple tombstones generated with the same name, for
                     # instance, after multiple robocop tests
                     for i in xrange(1, sys.maxint):
                         newname = "%s.%d.txt" % (f, i)
                         if not os.path.exists(newname):
                             os.rename(f, newname)
                             break
@@ -204,113 +192,96 @@ class RemoteAutomation(Automation):
                 print "%s does not exist; tombstone check skipped" % remoteDir
         else:
             print "MOZ_UPLOAD_DIR not defined; tombstone check skipped"
 
     def checkForCrashes(self, directory, symbolsPath):
         self.checkForANRs()
         self.checkForTombstones()
 
-        logcat = self._devicemanager.getLogcat(filterOutRegexps=fennecLogcatFilters)
+        logcat = self._dm.getLogcat(filterOutRegexps=fennecLogcatFilters)
 
         javaException = mozcrash.check_for_java_exception(logcat, test_name=self.lastTestSeen)
         if javaException:
             return True
 
         # If crash reporting is disabled (MOZ_CRASHREPORTER!=1), we can't say
         # anything.
         if not self.CRASHREPORTER:
             return False
 
         try:
             dumpDir = tempfile.mkdtemp()
             remoteCrashDir = posixpath.join(self._remoteProfile, 'minidumps')
-            if not self._devicemanager.dirExists(remoteCrashDir):
+            if not self._dm.dirExists(remoteCrashDir):
                 # If crash reporting is enabled (MOZ_CRASHREPORTER=1), the
                 # minidumps directory is automatically created when Fennec
                 # (first) starts, so its lack of presence is a hint that
                 # something went wrong.
                 print "Automation Error: No crash directory (%s) found on remote device" % remoteCrashDir
-                # Whilst no crash was found, the run should still display as a failure
                 return True
-            self._devicemanager.getDirectory(remoteCrashDir, dumpDir)
+            self._dm.getDirectory(remoteCrashDir, dumpDir)
 
             logger = get_default_logger()
-            if logger is not None:
-                crashed = mozcrash.log_crashes(logger, dumpDir, symbolsPath, test=self.lastTestSeen)
-            else:
-                crashed = Automation.checkForCrashes(self, dumpDir, symbolsPath)
+            crashed = mozcrash.log_crashes(logger, dumpDir, symbolsPath, test=self.lastTestSeen)
 
         finally:
             try:
                 shutil.rmtree(dumpDir)
             except:
                 print "WARNING: unable to remove directory: %s" % dumpDir
         return crashed
 
     def buildCommandLine(self, app, debuggerInfo, profileDir, testURL, extraArgs):
         # If remote profile is specified, use that instead
-        if (self._remoteProfile):
+        if self._remoteProfile:
             profileDir = self._remoteProfile
 
         # 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] in RemoteAutomation._specialAmCommands:
+        if app == "am" and extraArgs[0] in ('instrument', 'start'):
             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
         return app, args
 
     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, self._appName,
+        return self.RProcess(self._dm, cmd, self._remoteLog, env, cwd, self._appName,
                              **self._processArgs)
 
-    # 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, app=None,
+        def __init__(self, dm, cmd, stdout=None, env=None, cwd=None, app=None,
                      messageLogger=None, counts=None):
             self.dm = dm
             self.stdoutlen = 0
             self.lastTestSeen = "remoteautomation.py"
             self.proc = dm.launchProcess(cmd, stdout, cwd, env, True)
             self.messageLogger = messageLogger
             self.utilityPath = None
 
             self.counts = counts
             if self.counts is not None:
                 self.counts['pass'] = 0
                 self.counts['fail'] = 0
                 self.counts['todo'] = 0
 
-            if (self.proc is None):
-                if cmd[0] == 'am':
-                    self.proc = stdout
-                else:
-                    raise Exception("unable to launch process")
-            self.procName = cmd[0].split('/')[-1]
-            if cmd[0] == 'am' and cmd[1] in RemoteAutomation._specialAmCommands:
+            if self.proc is None:
+                self.proc = stdout
+            self.procName = cmd[0].split(posixpath.sep)[-1]
+            if cmd[0] == 'am' and cmd[1] in ('instrument', 'start'):
                 self.procName = app
 
             # Setting timeout at 1 hour since on a remote device this takes much longer.
             # Temporarily increased to 90 minutes because no more chunks can be created.
             self.timeout = 5400
-            # The benefit of the following sleep is unclear; it was formerly 15 seconds
-            time.sleep(1)
 
             # Used to buffer log messages until we meet a line break
             self.logBuffer = ""
 
         @property
         def pid(self):
             pid = self.dm.processExist(self.procName)
             # HACK: we should probably be more sophisticated about monitoring
--- a/layout/tools/reftest/reftestcommandline.py
+++ b/layout/tools/reftest/reftestcommandline.py
@@ -435,24 +435,16 @@ class RemoteArgumentsParser(ReftestArgum
 
         self.add_argument("--devicePort",
                           action="store",
                           type=str,
                           default="20701",
                           dest="devicePort",
                           help="port of remote device to test")
 
-        self.add_argument("--remote-product-name",
-                          action="store",
-                          type=str,
-                          dest="remoteProductName",
-                          default="fennec",
-                          help="Name of product to test - either fennec or firefox, "
-                               "defaults to fennec")
-
         self.add_argument("--remote-webserver",
                           action="store",
                           type=str,
                           dest="remoteWebServer",
                           help="IP Address of the webserver hosting the reftest content")
 
         self.add_argument("--http-port",
                           action="store",
@@ -498,17 +490,17 @@ class RemoteArgumentsParser(ReftestArgum
                           action="store_false",
                           dest="printDeviceInfo",
                           default=True,
                           help="do not display verbose diagnostics about the remote device")
 
     def validate_remote(self, options, automation):
         # Ensure our defaults are set properly for everything we can infer
         if not options.remoteTestRoot:
-            options.remoteTestRoot = automation._devicemanager.deviceRoot + \
+            options.remoteTestRoot = automation._dm.deviceRoot + \
                 '/reftest'
         options.remoteProfile = options.remoteTestRoot + "/profile"
 
         if options.remoteWebServer is None:
             options.remoteWebServer = self.get_ip()
 
         # Verify that our remotewebserver is set properly
         if options.remoteWebServer == '127.0.0.1':
@@ -563,17 +555,17 @@ class RemoteArgumentsParser(ReftestArgum
         # httpd-path is specified by standard makefile targets and may be specified
         # on the command line to select a particular version of httpd.js. If not
         # specified, try to select the one from hostutils.zip, as required in
         # bug 882932.
         if not options.httpdPath:
             options.httpdPath = os.path.join(options.utilityPath, "components")
 
         if not options.ignoreWindowSize:
-            parts = automation._devicemanager.getInfo(
+            parts = automation._dm.getInfo(
                 'screen')['screen'][0].split()
             width = int(parts[0].split(':')[1])
             height = int(parts[1].split(':')[1])
             if (width < 1366 or height < 1050):
                 self.error("ERROR: Invalid screen resolution %sx%s, "
                            "please adjust to 1366x1050 or higher" % (
                             width, height))
 
--- a/layout/tools/reftest/remotereftest.py
+++ b/layout/tools/reftest/remotereftest.py
@@ -197,18 +197,17 @@ class RemoteReftest(RefTest):
             localAutomation.IS_MAC = True
         elif (hostos == 'linux' or hostos == 'linux2'):
             localAutomation.IS_LINUX = True
             localAutomation.UNIXISH = True
         elif (hostos == 'win32' or hostos == 'win64'):
             localAutomation.BIN_SUFFIX = ".exe"
             localAutomation.IS_WIN32 = True
 
-        paths = [options.xrePath, localAutomation.DIST_BIN, self.automation._product,
-                 os.path.join('..', self.automation._product)]
+        paths = [options.xrePath, localAutomation.DIST_BIN]
         options.xrePath = self.findPath(paths)
         if options.xrePath is None:
             print ("ERROR: unable to find xulrunner path for %s, "
                    "please specify with --xre-path" % (os.name))
             return 1
         paths.append("bin")
         paths.append(os.path.join("..", "bin"))
 
@@ -410,19 +409,16 @@ def run_test_harness(parser, options):
         traceback.print_exc()
         print ("Automation Error: exception while initializing devicemanager.  "
                "Most likely the device is not in a testable state.")
         return 1
 
     automation = RemoteAutomation(None)
     automation.setDeviceManager(dm)
 
-    if options.remoteProductName:
-        automation.setProduct(options.remoteProductName)
-
     # Set up the defaults and ensure options are set
     parser.validate_remote(options, automation)
 
     # Check that Firefox is installed
     expected = options.app.split('/')[-1]
     installed = dm.shellCheckOutput(['pm', 'list', 'packages', expected])
     if expected not in installed:
         print "%s is not installed on this device" % expected
--- a/testing/mochitest/mochitest_options.py
+++ b/testing/mochitest/mochitest_options.py
@@ -874,23 +874,16 @@ class AndroidArguments(ArgumentContainer
           "suppress": True,
           }],
         [["--devicePort"],
          {"dest": "devicePort",
           "type": int,
           "default": 20701,
           "help": "port of remote device to test",
           }],
-        [["--remote-product-name"],
-         {"dest": "remoteProductName",
-          "default": "fennec",
-          "help": "The executable's name of remote product to test - either \
-                   fennec or firefox, defaults to fennec",
-          "suppress": True,
-          }],
         [["--remote-logfile"],
          {"dest": "remoteLogFile",
           "default": None,
           "help": "Name of log file on the device relative to the device \
                    root. PLEASE ONLY USE A FILENAME.",
           }],
         [["--remote-webserver"],
          {"dest": "remoteWebServer",
--- a/testing/mochitest/runrobocop.py
+++ b/testing/mochitest/runrobocop.py
@@ -59,21 +59,16 @@ class RobocopTestRunner(MochitestDesktop
         self.remoteLog = options.remoteLogFile
         self.auto.setRemoteLog(self.remoteLog)
         self.remoteScreenshots = "/mnt/sdcard/Robotium-Screenshots"
         self.remoteMozLog = os.path.join(options.remoteTestRoot, "mozlog")
         self.auto.setServerInfo(
             self.options.webServer, self.options.httpPort, self.options.sslPort)
         self.localLog = options.logFile
         self.localProfile = None
-        productPieces = self.options.remoteProductName.split('.')
-        if (productPieces is not None):
-            self.auto.setProduct(productPieces[0])
-        else:
-            self.auto.setProduct(self.options.remoteProductName)
         self.auto.setAppName(self.options.remoteappname)
         self.certdbNew = True
         self.remoteCopyAvailable = True
         self.passed = 0
         self.failed = 0
         self.todo = 0
 
     def startup(self):
@@ -176,19 +171,17 @@ class RobocopTestRunner(MochitestDesktop
         """
            Setup xrePath and utilityPath and verify xpcshell.
 
            This is similar to switchToLocalPaths in runtestsremote.py.
         """
         localAutomation = self.makeLocalAutomation()
         paths = [
             self.options.xrePath,
-            localAutomation.DIST_BIN,
-            self.auto._product,
-            os.path.join('..', self.auto._product)
+            localAutomation.DIST_BIN
         ]
         self.options.xrePath = self.findPath(paths)
         if self.options.xrePath is None:
             self.log.error(
                 "unable to find xulrunner path for %s, please specify with --xre-path" %
                 os.name)
             sys.exit(1)
         self.log.debug("using xre path %s" % self.options.xrePath)
--- a/testing/mochitest/runtestsremote.py
+++ b/testing/mochitest/runtestsremote.py
@@ -113,18 +113,16 @@ class MochiRemote(MochitestDesktop):
         remoteXrePath = options.xrePath
         remoteProfilePath = options.profilePath
         remoteUtilityPath = options.utilityPath
 
         localAutomation = self.makeLocalAutomation()
         paths = [
             options.xrePath,
             localAutomation.DIST_BIN,
-            self._automation._product,
-            os.path.join('..', self._automation._product)
         ]
         options.xrePath = self.findPath(paths)
         if options.xrePath is None:
             self.log.error(
                 "unable to find xulrunner path for %s, please specify with --xre-path" %
                 os.name)
             sys.exit(1)
 
@@ -337,21 +335,16 @@ def run_test_harness(parser, options):
 
     # Check that Firefox is installed
     expected = options.app.split('/')[-1]
     installed = dm.shellCheckOutput(['pm', 'list', 'packages', expected])
     if expected not in installed:
         log.error("%s is not installed on this device" % expected)
         return 1
 
-    productPieces = options.remoteProductName.split('.')
-    if (productPieces is not None):
-        auto.setProduct(productPieces[0])
-    else:
-        auto.setProduct(options.remoteProductName)
     auto.setAppName(options.remoteappname)
 
     logParent = os.path.dirname(options.remoteLogFile)
     dm.removeDir(logParent)
     dm.mkDir(logParent)
     auto.setRemoteLog(options.remoteLogFile)
     auto.setServerInfo(options.webServer, options.httpPort, options.sslPort)