Bug 1076969 - processLeakLog should come up with reasonable defaults itself. r=jmaher
authorAndrew McCreight <continuation@gmail.com>
Mon, 06 Oct 2014 14:23:17 -0700
changeset 209075 1b74418f424588c415839396ec6a0111ed4517aa
parent 209074 abd8b7b6dbd1608965ce0057c5347f46c57b0a0a
child 209076 fbb1b7c89a32334396fbe995fdd2db88179e540f
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersjmaher
bugs1076969
milestone35.0a1
Bug 1076969 - processLeakLog should come up with reasonable defaults itself. r=jmaher Instead of grabbing attributes off options at every call site, pass in the options object to processLeakLog, and attempt to get the attributes there. If not present, use a restrictive default value. This will prevent silent harness failures if one of the many ways to invoke processLeakLog fails to set up these options, and makes it so they don't have to set it up if they don't care.
build/automationutils.py
layout/tools/reftest/remotereftest.py
layout/tools/reftest/runreftest.py
layout/tools/reftest/runreftestb2g.py
testing/mochitest/runtests.py
testing/mochitest/runtestsb2g.py
--- a/build/automationutils.py
+++ b/build/automationutils.py
@@ -304,41 +304,51 @@ def processSingleLeakFile(leakLogFileNam
 
   if logAsWarning:
     log.warning("%s | leakcheck | %s %d bytes leaked (%s)"
                 % (prefix, processString, totalBytesLeaked, leakedObjectSummary))
   else:
     log.info("%s | leakcheck | %s %d bytes leaked (%s)"
              % (prefix, processString, totalBytesLeaked, leakedObjectSummary))
 
-def processLeakLog(leakLogFile, leakThresholds, ignoreMissingLeaks):
+def processLeakLog(leakLogFile, options):
   """Process the leak log, including separate leak logs created
   by child processes.
 
   Use this function if you want an additional PASS/FAIL summary.
   It must be used with the |XPCOM_MEM_BLOAT_LOG| environment variable.
 
   The base of leakLogFile for a non-default process needs to end with
     _proctype_pid12345.log
   "proctype" is a string denoting the type of the process, which should
   be the result of calling XRE_ChildProcessTypeToString(). 12345 is
   a series of digits that is the pid for the process. The .log is
   optional.
 
   All other file names are treated as being for default processes.
 
+  The options argument is checked for two optional attributes,
+  leakThresholds and ignoreMissingLeaks.
+
   leakThresholds should be a dict mapping process types to leak thresholds,
   in bytes. If a process type is not present in the dict the threshold
   will be 0.
+
+  ignoreMissingLeaks should be a list of process types. If a process
+  creates a leak log without a TOTAL, then we report an error if it isn't
+  in the list ignoreMissingLeaks.
   """
 
   if not os.path.exists(leakLogFile):
     log.info("WARNING | leakcheck | refcount logging is off, so leaks can't be detected!")
     return
 
+  leakThresholds = getattr(options, 'leakThresholds', {})
+  ignoreMissingLeaks = getattr(options, 'ignoreMissingLeaks', [])
+
   # This list is based on kGeckoProcessTypeString. ipdlunittest processes likely
   # are not going to produce leak logs we will ever see.
   knownProcessTypes = ["default", "plugin", "tab", "geckomediaplugin"]
 
   for processType in knownProcessTypes:
     log.info("TEST-INFO | leakcheck | %s process: leak threshold set at %d bytes"
              % (processType, leakThresholds.get(processType, 0)))
 
--- a/layout/tools/reftest/remotereftest.py
+++ b/layout/tools/reftest/remotereftest.py
@@ -157,20 +157,16 @@ class RemoteOptions(ReftestOptions):
             f.close()
 
         # 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")
 
-        # Android does not run leak tests, but set some reasonable defaults to avoid errors.
-        options.leakThresholds = {}
-        options.ignoreMissingLeaks = []
-
         # TODO: Copied from main, but I think these are no longer used in a post xulrunner world
         #options.xrePath = options.remoteTestRoot + self.automation._product + '/xulrunner'
         #options.utilityPath = options.testRoot + self.automation._product + '/bin'
         return options
 
 class ReftestServer:
     """ Web server used to serve Reftests, for closer fidelity to the real web.
         It is virtually identical to the server used in mochitest and will only
--- a/layout/tools/reftest/runreftest.py
+++ b/layout/tools/reftest/runreftest.py
@@ -339,17 +339,17 @@ class RefTest(object):
                                  cmdlineArgs,
                                  utilityPath = options.utilityPath,
                                  xrePath=options.xrePath,
                                  debuggerInfo=debuggerInfo,
                                  symbolsPath=options.symbolsPath,
                                  # give the JS harness 30 seconds to deal
                                  # with its own timeouts
                                  timeout=options.timeout + 30.0)
-      processLeakLog(self.leakLogFile, options.leakThresholds, options.ignoreMissingLeaks)
+      processLeakLog(self.leakLogFile, options)
       self.automation.log.info("\nREFTEST INFO | runreftest.py | Running tests: end.")
     finally:
       self.cleanup(profileDir)
     return status
 
   def copyExtraFilesToProfile(self, options, profile):
     "Copy extra files or dirs specified on the command line to the testing profile."
     profileDir = profile.profile
@@ -507,17 +507,16 @@ class ReftestOptions(OptionParser):
       if options.totalChunks is not None and options.thisChunk is None:
         self.error("cannot specify thisChunk or totalChunks with parallel tests")
       if options.focusFilterMode != "all":
         self.error("cannot specify focusFilterMode with parallel tests")
       if options.debugger is not None:
         self.error("cannot specify a debugger with parallel tests")
 
     options.leakThresholds = {"default": options.defaultLeakThreshold}
-    options.ignoreMissingLeaks = []
 
     return options
 
 def main():
   automation = Automation()
   parser = ReftestOptions(automation)
   reftest = RefTest(automation)
 
--- a/layout/tools/reftest/runreftestb2g.py
+++ b/layout/tools/reftest/runreftestb2g.py
@@ -198,20 +198,16 @@ class B2GOptions(ReftestOptions):
             f.close()
 
         # 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 from the xre bundle, as required in bug 882932.
         if not options.httpdPath:
             options.httpdPath = os.path.join(options.xrePath, "components")
 
-        # B2G reftests do not do leak checking, but set some reasonable defaults to avoid errors.
-        options.leakThresholds = {}
-        options.ignoreMissingLeaks = []
-
         return options
 
 
 class ProfileConfigParser(ConfigParser.RawConfigParser):
     """Subclass of RawConfigParser that outputs .ini files in the exact
        format expected for profiles.ini, which is slightly different
        than the default format.
     """
--- a/testing/mochitest/runtests.py
+++ b/testing/mochitest/runtests.py
@@ -1840,17 +1840,17 @@ class Mochitest(MochitestUtilsMixin):
         self.log.error("Automation Error: Received unexpected exception while running application\n")
         status = 1
 
     finally:
       if options.vmwareRecording:
         self.stopVMwareRecording();
       self.stopServers()
 
-    processLeakLog(self.leak_report_file, options.leakThresholds, options.ignoreMissingLeaks)
+    processLeakLog(self.leak_report_file, options)
 
     if self.nsprLogs:
       with zipfile.ZipFile("%s/nsprlog.zip" % browserEnv["MOZ_UPLOAD_DIR"], "w", zipfile.ZIP_DEFLATED) as logzip:
         for logfile in glob.glob("%s/nspr*.log*" % tempfile.gettempdir()):
           logzip.write(logfile)
           os.remove(logfile)
 
     self.log.info("runtests.py | Running tests: end.")
--- a/testing/mochitest/runtestsb2g.py
+++ b/testing/mochitest/runtestsb2g.py
@@ -197,17 +197,17 @@ class B2GMochitest(MochitestUtilsMixin):
             if status is None:
                 # the runner has timed out
                 status = 124
 
             local_leak_file = tempfile.NamedTemporaryFile()
             self.app_ctx.dm.getFile(self.leak_report_file, local_leak_file.name)
             self.app_ctx.dm.removeFile(self.leak_report_file)
 
-            processLeakLog(local_leak_file.name, options.leakThresholds, options.ignoreMissingLeaks)
+            processLeakLog(local_leak_file.name, options)
         except KeyboardInterrupt:
             self.log.info("runtests.py | Received keyboard interrupt.\n");
             status = -1
         except:
             traceback.print_exc()
             self.log.error("Automation Error: Received unexpected exception while running application\n")
             if hasattr(self, 'runner'):
                 self.runner.check_for_crashes()