Bug 498128 - xpcshell-tests: add option to enable/disable file logging; (Av3a) Support --[no-]logfiles in xpcst suite, improve dir/file handling in 3 suites

r=ted.mielczarek

layout/tools/reftest/runreftest.py

@@ -57,18 +57,15 @@ def getFullPath(path):
   return os.path.normpath(os.path.join(oldcwd, os.path.expanduser(path)))
 
 def createReftestProfile(options, profileDir):
-  "Sets up a clean profile for reftest."
+  "Sets up a profile for reftest."
 
-  # Start with a clean slate.
+  # Set preferences.
-  shutil.rmtree(profileDir, True)
-  os.mkdir(profileDir)
-  # reftest should only need the dump pref set
   prefsFile = open(os.path.join(profileDir, "user.js"), "w")
   prefsFile.write("""user_pref("browser.dom.window.dump.enabled", true);
 """)
   prefsFile.write('user_pref("reftest.timeout", %d);' % options.timeout)
-
   prefsFile.close()
+
   # install the reftest extension bits into the profile
   profileExtensionsPath = os.path.join(profileDir, "extensions")
   os.mkdir(profileExtensionsPath)
@@ -170,7 +167,7 @@ Are you executing $objdir/_tests/reftest/runreftest.py?""" \
     processLeakLog(leakLogFile, options.leakThreshold)
     automation.log.info("\nREFTEST INFO | runreftest.py | Running tests: end.")
   finally:
-    if profileDir is not None:
+    if profileDir:
       shutil.rmtree(profileDir)
   sys.exit(status)
 

testing/mochitest/runtests.py.in

@@ -85,7 +85,7 @@ os.chdir(SCRIPT_DIRECTORY)
 
 PROFILE_DIRECTORY = os.path.abspath("./mochitesttestingprofile")
 
-LEAK_REPORT_FILE = PROFILE_DIRECTORY + "/" + "leaks-report.log"
+LEAK_REPORT_FILE = os.path.join(PROFILE_DIRECTORY, "runtests_leaks.log")
 
 # Map of debugging programs to information about them, like default arguments
 # and whether or not they are interactive.
@@ -421,7 +421,6 @@ Are you executing $objdir/_tests/testing/mochitest/runtests.py?"""
   # we'll try to kill the server and exit with an error.
   server.ensureReady(SERVER_STARTUP_TIMEOUT)
 
-
   # URL parameters to test URL:
   #
   # autorun -- kick off tests automatically

testing/testsuite-targets.mk

@@ -101,6 +101,7 @@ xpcshell-tests:
 	  -I$(topsrcdir)/build \
 	  $(topsrcdir)/testing/xpcshell/runxpcshelltests.py \
 	  --manifest=$(DEPTH)/_tests/xpcshell/all-test-dirs.list \
+	  --no-logfiles \
 	  --symbols-path=$(DIST)/crashreporter-symbols \
 	  $(TEST_PATH_ARG) $(EXTRA_TEST_ARGS) \
 	  $(DIST)/bin/xpcshell

testing/xpcshell/runxpcshelltests.py

@@ -38,7 +38,6 @@
 # ***** END LICENSE BLOCK ***** */
 
 import re, sys, os, os.path, logging, shutil
-import tempfile
 from glob import glob
 from optparse import OptionParser
 from subprocess import Popen, PIPE, STDOUT
@@ -69,18 +68,24 @@ def readManifest(manifest):
     pass # just eat exceptions
   return testdirs
 
-def runTests(xpcshell, testdirs=[], xrePath=None, testPath=None,
+def runTests(xpcshell, xrePath=None, symbolsPath=None,
-             manifest=None, interactive=False, symbolsPath=None):
+             manifest=None, testdirs=[], testPath=None,
-  """Run the tests in |testdirs| using the |xpcshell| executable.
+             interactive=False, logfiles=True):
+  """Run xpcshell tests.
 
+  |xpcshell|, is the xpcshell executable to use to run the tests.
   |xrePath|, if provided, is the path to the XRE to use.
-  |testPath|, if provided, indicates a single path and/or test to run.
-  |manifest|, if provided, is a file containing a list of
-    test directories to run.
-  |interactive|, if set to True, indicates to provide an xpcshell prompt
-    instead of automatically executing the test.
   |symbolsPath|, if provided is the path to a directory containing
     breakpad symbols for processing crashes in tests.
+  |manifest|, if provided, is a file containing a list of
+    test directories to run.
+  |testdirs|, if provided, is a list of absolute paths of test directories.
+    No-manifest only option.
+  |testPath|, if provided, indicates a single path and/or test to run.
+  |interactive|, if set to True, indicates to provide an xpcshell prompt
+    instead of automatically executing the test.
+  |logfiles|, if set to False, indicates not to save output to log files.
+    Non-interactive only option.
   """
 
   if not testdirs and not manifest:
@@ -102,11 +107,6 @@ def runTests(xpcshell, testdirs=[], xrePath=None, testPath=None,
   # Don't launch the crash reporter client
   env["MOZ_CRASHREPORTER_NO_REPORT"] = "1"
 
-  # Enable leaks (only) detection to its own log file.
-  # Each test will overwrite it.
-  leakLogFile = os.path.join(tempfile.gettempdir(), "runxpcshelltests_leaks.log")
-  env["XPCOM_MEM_LEAK_LOG"] = leakLogFile
-
   if xrePath is None:
     xrePath = os.path.dirname(xpcshell)
   else:
@@ -163,6 +163,7 @@ def runTests(xpcshell, testdirs=[], xrePath=None, testPath=None,
       # Simply remove optional ending separator.
       testPath = testPath.rstrip("/")
 
+  # Override testdirs.
   if manifest is not None:
     testdirs = readManifest(os.path.abspath(manifest))
 
@@ -206,22 +207,27 @@ def runTests(xpcshell, testdirs=[], xrePath=None, testPath=None,
       # The test file will have to be loaded after the head files.
       cmdT = ['-e', 'const _TEST_FILE = ["%s"];' %
                       os.path.join(testdir, test).replace('\\', '/')]
+
       # create a temp dir that the JS harness can stick a profile in
-      profd = mkdtemp()
+      profileDir = None
-      env["XPCSHELL_TEST_PROFILE_DIR"] = profd
+      try:
+        profileDir = mkdtemp()
+        env["XPCSHELL_TEST_PROFILE_DIR"] = profileDir
+
+        # Enable leaks (only) detection to its own log file.
+        leakLogFile = os.path.join(profileDir, "runxpcshelltests_leaks.log")
+        env["XPCOM_MEM_LEAK_LOG"] = leakLogFile
 
         proc = Popen(cmdH + cmdT + xpcsRunArgs,
                      stdout=pStdout, stderr=pStderr, env=env, cwd=testdir)
         # |stderr == None| as |pStderr| was either |None| or redirected to |stdout|.
         stdout, stderr = proc.communicate()
 
-      shutil.rmtree(profd, True)
-
         if interactive:
-        # not sure what else to do here...
+          # Not sure what else to do here...
           return True
 
-      if proc.returncode != 0 or (stdout is not None and re.search("^TEST-UNEXPECTED-FAIL", stdout, re.MULTILINE)):
+        if proc.returncode != 0 or (stdout and re.search("^TEST-UNEXPECTED-FAIL", stdout, re.MULTILINE)):
           print """TEST-UNEXPECTED-FAIL | %s | test failed (with xpcshell return code: %d), see following log:
   >>>>>>>
   %s
@@ -234,9 +240,9 @@ def runTests(xpcshell, testdirs=[], xrePath=None, testPath=None,
 
         dumpLeakLog(leakLogFile, True)
 
-      if stdout is not None:
+        if logfiles and stdout:
           try:
-          f = open(test + '.log', 'w')
+            f = open(test + ".log", "w")
             f.write(stdout)
 
             if os.path.exists(leakLogFile):
@@ -246,11 +252,9 @@ def runTests(xpcshell, testdirs=[], xrePath=None, testPath=None,
           finally:
             if f:
               f.close()
-
+      finally:
-      # Remove the leak detection file (here) so it can't "leak" to the next test.
+        if profileDir:
-      # The file is not there if leak logging was not enabled in the xpcshell build.
+          shutil.rmtree(profileDir)
-      if os.path.exists(leakLogFile):
-        os.remove(leakLogFile)
 
   if passCount == 0 and failCount == 0:
     print "TEST-UNEXPECTED-FAIL | runxpcshelltests.py | No tests run. Did you pass an invalid --test-path?"
@@ -267,15 +271,21 @@ def main():
   parser = OptionParser()
 
   addCommonOptions(parser)
-  parser.add_option("--test-path",
-                    action="store", type="string", dest="testPath",
-                    default=None, help="single path and/or test filename to test")
   parser.add_option("--interactive",
                     action="store_true", dest="interactive", default=False,
                     help="don't automatically run tests, drop to an xpcshell prompt")
+  parser.add_option("--logfiles",
+                    action="store_true", dest="logfiles", default=True,
+                    help="create log files (default, only used to override --no-logfiles)")
   parser.add_option("--manifest",
-                    action="store", type="string", dest="manifest",
+                    type="string", dest="manifest", default=None,
-                    default=None, help="Manifest of test directories to use")
+                    help="Manifest of test directories to use")
+  parser.add_option("--no-logfiles",
+                    action="store_false", dest="logfiles",
+                    help="don't create log files")
+  parser.add_option("--test-path",
+                    type="string", dest="testPath", default=None,
+                    help="single path and/or test filename to test")
   options, args = parser.parse_args()
 
   if len(args) < 2 and options.manifest is None or \
@@ -289,12 +299,14 @@ def main():
     print >>sys.stderr, "Error: You must specify a test filename in interactive mode!"
     sys.exit(1)
 
-  if not runTests(args[0], testdirs=args[1:],
+  if not runTests(args[0],
                   xrePath=options.xrePath,
+                  symbolsPath=options.symbolsPath,
+                  manifest=options.manifest,
+                  testdirs=args[1:],
                   testPath=options.testPath,
                   interactive=options.interactive,
-                  manifest=options.manifest,
+                  logfiles=options.logfiles):
-                  symbolsPath=options.symbolsPath):
     sys.exit(1)
 
 if __name__ == '__main__':