Bring automationutils.py up to date with the fixes from bug 808410, bug 813577, bug 850681; a=test-only

build/automationutils.py

@@ -146,24 +146,23 @@ def checkForCrashes(dumpDir, symbolsPath, testName=None):
   if len(dumps) == 0:
     return False
 
-  foundCrash = False
-  removeSymbolsPath = False
-
-  # If our symbols are at a remote URL, download them now
-  if symbolsPath and isURL(symbolsPath):
-    print "Downloading symbols from: " + symbolsPath
-    removeSymbolsPath = True
-    # Get the symbols and write them to a temporary zipfile
-    data = urllib2.urlopen(symbolsPath)
-    symbolsFile = tempfile.TemporaryFile()
-    symbolsFile.write(data.read())
-    # extract symbols to a temporary directory (which we'll delete after
-    # processing all crashes)
-    symbolsPath = tempfile.mkdtemp()
-    zfile = ZipFileReader(symbolsFile)
-    zfile.extractall(symbolsPath)
-
   try:
+    removeSymbolsPath = False
+
+    # If our symbols are at a remote URL, download them now
+    if symbolsPath and isURL(symbolsPath):
+      print "Downloading symbols from: " + symbolsPath
+      removeSymbolsPath = True
+      # Get the symbols and write them to a temporary zipfile
+      data = urllib2.urlopen(symbolsPath)
+      symbolsFile = tempfile.TemporaryFile()
+      symbolsFile.write(data.read())
+      # extract symbols to a temporary directory (which we'll delete after
+      # processing all crashes)
+      symbolsPath = tempfile.mkdtemp()
+      zfile = ZipFileReader(symbolsFile)
+      zfile.extractall(symbolsPath)
+
     for d in dumps:
       stackwalkOutput = []
       stackwalkOutput.append("Crash dump filename: " + d)
@@ -216,12 +215,11 @@ def checkForCrashes(dumpDir, symbolsPath, testName=None):
       extra = os.path.splitext(d)[0] + ".extra"
       if os.path.exists(extra):
         os.remove(extra)
-      foundCrash = True
   finally:
     if removeSymbolsPath:
       shutil.rmtree(symbolsPath)
 
-  return foundCrash
+  return True
 
 def getFullPath(directory, path):
   "Get an absolute path relative to 'directory'."
@@ -285,9 +283,8 @@ def dumpLeakLog(leakLogFile, filter = False):
   if not os.path.exists(leakLogFile):
     return
 
-  leaks = open(leakLogFile, "r")
+  with open(leakLogFile, "r") as leaks:
-  leakReport = leaks.read()
+    leakReport = leaks.read()
-  leaks.close()
 
   # Only |XPCOM_MEM_LEAK_LOG| reports can be actually filtered out.
   # Only check whether an actual leak was reported.
@@ -297,9 +294,8 @@ def dumpLeakLog(leakLogFile, filter = False):
   # Simply copy the log.
   log.info(leakReport.rstrip("\n"))
 
-def processSingleLeakFile(leakLogFileName, PID, processType, leakThreshold):
+def processSingleLeakFile(leakLogFileName, processType, leakThreshold):
-  """Process a single leak log, corresponding to the specified
+  """Process a single leak log.
-  process PID and type.
   """
 
   #                  Per-Inst  Leaked      Total  Rem ...
@@ -310,85 +306,75 @@ def processSingleLeakFile(leakLogFileName, PID, processType, leakThreshold):
                       r"-?\d+\s+(?P<numLeaked>-?\d+)")
 
   processString = ""
-  if PID and processType:
+  if processType:
-    processString = "| %s process %s " % (processType, PID)
+    # eg 'plugin'
-  leaks = open(leakLogFileName, "r")
+    processString = " %s process:" % processType
-  for line in leaks:
-    matches = lineRe.match(line)
-    if (matches and
-        int(matches.group("numLeaked")) == 0 and
-        matches.group("name") != "TOTAL"):
-      continue
-    log.info(line.rstrip())
-  leaks.close()
 
-  leaks = open(leakLogFileName, "r")
-  seenTotal = False
   crashedOnPurpose = False
-  prefix = "TEST-PASS"
+  totalBytesLeaked = None
-  numObjects = 0
+  leakAnalysis = []
-  for line in leaks:
+  leakedObjectNames = []
-    if line.find("purposefully crash") > -1:
+  with open(leakLogFileName, "r") as leaks:
-      crashedOnPurpose = True
+    for line in leaks:
-    matches = lineRe.match(line)
+      if line.find("purposefully crash") > -1:
-    if not matches:
+        crashedOnPurpose = True
-      continue
+      matches = lineRe.match(line)
-    name = matches.group("name")
+      if not matches:
-    size = int(matches.group("size"))
+        # eg: the leak table header row
-    bytesLeaked = int(matches.group("bytesLeaked"))
+        log.info(line.rstrip())
-    numLeaked = int(matches.group("numLeaked"))
+        continue
-    if size < 0 or bytesLeaked < 0 or numLeaked < 0:
+      name = matches.group("name")
-      log.info("TEST-UNEXPECTED-FAIL %s| automationutils.processLeakLog() | negative leaks caught!" %
+      size = int(matches.group("size"))
-               processString)
+      bytesLeaked = int(matches.group("bytesLeaked"))
+      numLeaked = int(matches.group("numLeaked"))
+      # Output the raw line from the leak log table if it is the TOTAL row,
+      # or is for an object row that has been leaked.
+      if numLeaked != 0 or name == "TOTAL":
+        log.info(line.rstrip())
+      # Analyse the leak log, but output later or it will interrupt the leak table
       if name == "TOTAL":
-        seenTotal = True
+        totalBytesLeaked = bytesLeaked
-    elif name == "TOTAL":
+      if size < 0 or bytesLeaked < 0 or numLeaked < 0:
-      seenTotal = True
+        leakAnalysis.append("TEST-UNEXPECTED-FAIL | leakcheck |%s negative leaks caught!"
-      # Check for leaks.
+                            % processString)
-      if bytesLeaked < 0 or bytesLeaked > leakThreshold:
+        continue
-        prefix = "TEST-UNEXPECTED-FAIL"
+      if name != "TOTAL" and numLeaked != 0:
-        leakLog = "TEST-UNEXPECTED-FAIL %s| automationutils.processLeakLog() | leaked" \
+        leakedObjectNames.append(name)
-                  " %d bytes during test execution" % (processString, bytesLeaked)
+        leakAnalysis.append("TEST-INFO | leakcheck |%s leaked %d %s (%s bytes)"
-      elif bytesLeaked > 0:
+                            % (processString, numLeaked, name, bytesLeaked))
-        leakLog = "TEST-PASS %s| automationutils.processLeakLog() | WARNING leaked" \
+  log.info('\n'.join(leakAnalysis))
-                  " %d bytes during test execution" % (processString, bytesLeaked)
-      else:
-        leakLog = "TEST-PASS %s| automationutils.processLeakLog() | no leaks detected!" \
-                  % processString
-      # Remind the threshold if it is not 0, which is the default/goal.
-      if leakThreshold != 0:
-        leakLog += " (threshold set at %d bytes)" % leakThreshold
-      # Log the information.
-      log.info(leakLog)
-    else:
-      if numLeaked != 0:
-        if numLeaked > 1:
-          instance = "instances"
-          rest = " each (%s bytes total)" % matches.group("bytesLeaked")
-        else:
-          instance = "instance"
-          rest = ""
-        numObjects += 1
-        if numObjects > 5:
-          # don't spam brief tinderbox logs with tons of leak output
-          prefix = "TEST-INFO"
-        log.info("%(prefix)s %(process)s| automationutils.processLeakLog() | leaked %(numLeaked)d %(instance)s of %(name)s "
-                 "with size %(size)s bytes%(rest)s" %
-                 { "prefix": prefix,
-                   "process": processString,
-                   "numLeaked": numLeaked,
-                   "instance": instance,
-                   "name": name,
-                   "size": matches.group("size"),
-                   "rest": rest })
-  if not seenTotal:
-    if crashedOnPurpose:
-      log.info("INFO | automationutils.processLeakLog() | process %s was " \
-               "deliberately crashed and thus has no leak log" % PID)
-    else:
-      log.info("WARNING | automationutils.processLeakLog() | missing output line for total leaks!")
-  leaks.close()
 
+  if totalBytesLeaked is None:
+    # We didn't see a line with name 'TOTAL'
+    if crashedOnPurpose:
+      log.info("TEST-INFO | leakcheck |%s deliberate crash and thus no leak log"
+               % processString)
+    else:
+      # TODO: This should be a TEST-UNEXPECTED-FAIL, but was changed to a warning
+      # due to too many intermittent failures (see bug 831223).
+      log.info("WARNING | leakcheck |%s missing output line for total leaks!"
+               % processString)
+    return
+
+  if totalBytesLeaked == 0:
+    log.info("TEST-PASS | leakcheck |%s no leaks detected!" % processString)
+    return
+
+  # totalBytesLeaked was seen and is non-zero.
+  if totalBytesLeaked > leakThreshold:
+    # Fail the run if we're over the threshold (which defaults to 0)
+    prefix = "TEST-UNEXPECTED-FAIL"
+  else:
+    prefix = "WARNING"
+  # Create a comma delimited string of the first N leaked objects found,
+  # to aid with bug summary matching in TBPL. Note: The order of the objects
+  # had no significance (they're sorted alphabetically).
+  maxSummaryObjects = 5
+  leakedObjectSummary = ', '.join(leakedObjectNames[:maxSummaryObjects])
+  if len(leakedObjectNames) > maxSummaryObjects:
+    leakedObjectSummary += ', ...'
+  log.info("%s | leakcheck |%s %d bytes leaked (%s)"
+           % (prefix, processString, totalBytesLeaked, leakedObjectSummary))
 
 def processLeakLog(leakLogFile, leakThreshold = 0):
   """Process the leak log, including separate leak logs created
@@ -399,25 +385,26 @@ def processLeakLog(leakLogFile, leakThreshold = 0):
   """
 
   if not os.path.exists(leakLogFile):
-    log.info("WARNING | automationutils.processLeakLog() | refcount logging is off, so leaks can't be detected!")
+    log.info("WARNING | leakcheck | refcount logging is off, so leaks can't be detected!")
     return
 
+  if leakThreshold != 0:
+    log.info("TEST-INFO | leakcheck | threshold set at %d bytes" % leakThreshold)
+
   (leakLogFileDir, leakFileBase) = os.path.split(leakLogFile)
-  pidRegExp = re.compile(r".*?_([a-z]*)_pid(\d*)$")
+  fileNameRegExp = re.compile(r".*?_([a-z]*)_pid\d*$")
   if leakFileBase[-4:] == ".log":
     leakFileBase = leakFileBase[:-4]
-    pidRegExp = re.compile(r".*?_([a-z]*)_pid(\d*).log$")
+    fileNameRegExp = re.compile(r".*?_([a-z]*)_pid\d*.log$")
 
   for fileName in os.listdir(leakLogFileDir):
     if fileName.find(leakFileBase) != -1:
       thisFile = os.path.join(leakLogFileDir, fileName)
-      processPID = 0
       processType = None
-      m = pidRegExp.search(fileName)
+      m = fileNameRegExp.search(fileName)
       if m:
         processType = m.group(1)
-        processPID = m.group(2)
+      processSingleLeakFile(thisFile, processType, leakThreshold)
-      processSingleLeakFile(thisFile, processPID, processType, leakThreshold)
 
 def replaceBackSlashes(input):
   return input.replace('\\', '/')