Bug 1293295 - Replace all mochitest 'flavor' options with a single --…

…flavor argument, r=jmaher

This accomplishes three things:

1) Easier to use CLI when running without the benefit of testing/mochitest/mach_commands.py
2) Guarantees these arguments are mutually exclusive
3) Simplifies a bunch of logic in the test harness

The primary motivation for this change is to slightly improve the UX when running mochitest
from a taskcluster interactive loaner. However, this is more of a bandaid solution that was
easy to implement before the proper fix in bug 1293259 can be landed.

MozReview-Commit-ID: IeHBGrJ0Sji

b2g/build.mk

@@ -21,7 +21,7 @@ ifdef ENABLE_TESTS
 # Implemented in testing/testsuite-targets.mk
 
 mochitest-browser-chrome:
-	$(RUN_MOCHITEST) --browser-chrome
+	$(RUN_MOCHITEST) --flavor=browser
 	$(CHECK_TEST_ERROR)
 
 mochitest:: mochitest-browser-chrome

browser/build.mk

@@ -45,7 +45,7 @@ ifdef ENABLE_TESTS
 # Implemented in testing/testsuite-targets.mk
 
 mochitest-browser-chrome:
-	$(RUN_MOCHITEST) --browser-chrome
+	$(RUN_MOCHITEST) --flavor=browser
 	$(CHECK_TEST_ERROR)
 
 mochitest:: mochitest-browser-chrome

mobile/android/build.mk

@@ -53,7 +53,7 @@ ifdef ENABLE_TESTS
 # Implemented in testing/testsuite-targets.mk
 
 mochitest-browser-chrome:
-	$(RUN_MOCHITEST) --browser-chrome
+	$(RUN_MOCHITEST) --flavor=browser
 	$(CHECK_TEST_ERROR)
 
 mochitest:: mochitest-browser-chrome

testing/mochitest/mach_commands.py

@@ -90,39 +90,39 @@
         'aliases': ('chrome', 'mochitest-chrome'),
         'enabled_apps': ('firefox', 'mulet', 'b2g', 'android'),
         'extra_args': {
-            'chrome': True,
+            'flavor': 'chrome',
         }
     },
     'browser-chrome': {
         'suite': 'browser',
         'aliases': ('browser', 'browser-chrome', 'mochitest-browser-chrome', 'bc'),
         'enabled_apps': ('firefox',),
         'extra_args': {
-            'browserChrome': True,
+            'flavor': 'browser',
         }
     },
     'jetpack-package': {
         'suite': 'jetpack-package',
         'aliases': ('jetpack-package', 'mochitest-jetpack-package', 'jpp'),
         'enabled_apps': ('firefox',),
         'extra_args': {
-            'jetpackPackage': True,
+            'flavor': 'jetpack-package',
         }
     },
     'jetpack-addon': {
         'suite': 'jetpack-addon',
         'aliases': ('jetpack-addon', 'mochitest-jetpack-addon', 'jpa'),
         'enabled_apps': ('firefox',),
         'extra_args': {
-            'jetpackAddon': True,
+            'flavor': 'jetpack-addon',
         }
     },
     'a11y': {
         'suite': 'a11y',
         'aliases': ('a11y', 'mochitest-a11y', 'accessibility'),
         'enabled_apps': ('firefox',),
         'extra_args': {
-            'a11y': True,
+            'flavor': 'a11y',
         }
     },
 }

testing/mochitest/mochitest_options.py

@@ -86,8 +86,8 @@ def get_full_path(self, path, cwd):
 class MochitestArguments(ArgumentContainer):
     """General mochitest arguments."""
 
+    FLAVORS = ('a11y', 'browser', 'chrome', 'jetpack-addon', 'jetpack-package', 'plain')
     LOG_LEVELS = ("DEBUG", "INFO", "WARNING", "ERROR", "FATAL")
-    LEVEL_STRING = ", ".join(LOG_LEVELS)
 
     args = [
         [["test_paths"],
@@ -97,6 +97,12 @@ class MochitestArguments(ArgumentContainer):
           "help": "Test to run. Can be a single test file or a directory of tests "
                   "(to run recursively). If omitted, the entire suite is run.",
           }],
+        [["-f", "--flavor"],
+         {"default": "plain",
+          "choices": FLAVORS,
+          "help": "Mochitest flavor to run, one of {}. Defaults to 'plain'.".format(FLAVORS),
+          "suppress": build_obj is not None,
+          }],
         [["--keep-open"],
          {"nargs": "?",
           "type": strtobool,
@@ -182,13 +188,8 @@ class MochitestArguments(ArgumentContainer):
          {"dest": "consoleLevel",
           "choices": LOG_LEVELS,
           "default": "INFO",
-          "help": "One of %s to determine the level of console logging." % LEVEL_STRING,
-          "suppress": True,
-          }],
-        [["--chrome"],
-         {"action": "store_true",
-          "default": False,
-          "help": "Run chrome mochitests.",
+          "help": "One of {} to determine the level of console logging.".format(
+                  ', '.join(LOG_LEVELS)),
           "suppress": True,
           }],
         [["--bisect-chunk"],
@@ -207,38 +208,11 @@ class MochitestArguments(ArgumentContainer):
           "default": "",
           "help": "Stop running the test sequence at this test.",
           }],
-        [["--browser-chrome"],
-         {"action": "store_true",
-          "dest": "browserChrome",
-          "default": False,
-          "help": "run browser chrome Mochitests",
-          "suppress": True,
-          }],
         [["--subsuite"],
          {"default": None,
           "help": "Subsuite of tests to run. Unlike tags, subsuites also remove tests from "
                   "the default set. Only one can be specified at once.",
           }],
-        [["--jetpack-package"],
-         {"action": "store_true",
-          "dest": "jetpackPackage",
-          "help": "Run jetpack package tests.",
-          "default": False,
-          "suppress": True,
-          }],
-        [["--jetpack-addon"],
-         {"action": "store_true",
-          "dest": "jetpackAddon",
-          "help": "Run jetpack addon tests.",
-          "default": False,
-          "suppress": True,
-          }],
-        [["--a11y"],
-         {"action": "store_true",
-          "help": "Run accessibility Mochitests.",
-          "default": False,
-          "suppress": True,
-          }],
         [["--setenv"],
          {"action": "append",
           "dest": "environment",

testing/mochitest/redirect.html

@@ -5,8 +5,8 @@
   <script type="text/javascript">
     function redirect(aURL)
     {
-      // We create a listener for this event in browser-test.js 
-      // which will get picked up when specifying --chrome or --a11y
+      // We create a listener for this event in browser-test.js which will
+      // get picked up when specifying --flavor=chrome or --flavor=a11y
       var event = new CustomEvent("contentEvent", {
         bubbles: true,
         detail: {

testing/mochitest/runtests.py

@@ -619,8 +619,7 @@ def buildURLOptions(self, options, env):
         if options.logFile:
             options.logFile = self.getLogFilePath(options.logFile)
 
-        if options.browserChrome or options.chrome or \
-           options.a11y or options.jetpackPackage or options.jetpackAddon:
+        if options.flavor in ('a11y', 'browser', 'chrome', 'jetpack-addon', 'jetpack-package'):
             self.makeTestConfig(options)
         else:
             if options.autorun:
@@ -689,32 +688,31 @@ def buildURLOptions(self, options, env):
             if options.debugger:
                 self.urlOpts.append("interactiveDebugger=true")
 
-    def getTestFlavor(self, options):
-        if options.browserChrome:
-            return "browser-chrome"
-        elif options.jetpackPackage:
-            return "jetpack-package"
-        elif options.jetpackAddon:
-            return "jetpack-addon"
-        elif options.chrome:
-            return "chrome"
-        elif options.a11y:
-            return "a11y"
-        else:
-            return "mochitest"
+    def normflavor(self, flavor):
+        """
+        In some places the string 'browser-chrome' is expected instead of
+        'browser' and 'mochitest' instead of 'plain'. Normalize the flavor
+        strings for those instances.
+        """
+        # TODO Use consistent flavor strings everywhere and remove this
+        if flavor == 'browser':
+            return 'browser-chrome'
+        elif flavor == 'plain':
+            return 'mochitest'
+        return flavor
 
     # This check can be removed when bug 983867 is fixed.
     def isTest(self, options, filename):
         allow_js_css = False
-        if options.browserChrome:
+        if options.flavor == 'browser':
             allow_js_css = True
             testPattern = re.compile(r"browser_.+\.js")
-        elif options.jetpackPackage:
+        elif options.flavor == 'jetpack-package':
             allow_js_css = True
             testPattern = re.compile(r"test-.+\.js")
-        elif options.jetpackAddon:
+        elif options.flavor == 'jetpack-addon':
             testPattern = re.compile(r".+\.xpi")
-        elif options.chrome or options.a11y:
+        elif options.flavor in ('a11y', 'chrome'):
             testPattern = re.compile(r"(browser|test)_.+\.(xul|html|js|xhtml)")
         else:
             testPattern = re.compile(r"test_")
@@ -728,19 +726,11 @@ def isTest(self, options, filename):
                 not re.search(r'\^headers\^$', filename))
 
     def setTestRoot(self, options):
-        if options.browserChrome:
-            if options.immersiveMode:
+        if options.flavor != 'plain':
+            self.testRoot = options.flavor
+
+            if options.flavor == 'browser' and options.immersiveMode:
                 self.testRoot = 'metro'
-            else:
-                self.testRoot = 'browser'
-        elif options.jetpackPackage:
-            self.testRoot = 'jetpack-package'
-        elif options.jetpackAddon:
-            self.testRoot = 'jetpack-addon'
-        elif options.a11y:
-            self.testRoot = 'a11y'
-        elif options.chrome:
-            self.testRoot = 'chrome'
         else:
             self.testRoot = self.TEST_PATH
         self.testRootAbs = os.path.join(SCRIPT_DIR, self.testRoot)
@@ -760,9 +750,9 @@ def buildTestURL(self, options):
             else:
                 testURL = "/".join([testURL, options.test_paths[0]])
 
-        if options.chrome or options.a11y:
+        if options.flavor in ('a11y', 'chrome'):
             testURL = "/".join([testHost, self.CHROME_PATH])
-        elif options.browserChrome or options.jetpackPackage or options.jetpackAddon:
+        elif options.flavor in ('browser', 'jetpack-addon', 'jetpack-package'):
             testURL = "about:blank"
         if options.nested_oop:
             testURL = "/".join([testHost, self.NESTED_OOP_TEST_PATH])
@@ -1066,8 +1056,7 @@ def remove_imptest_failure_expectations(tests, values):
                         self.log.warning("runtime file %s not found; defaulting to chunk-by-dir" %
                                          runtime_file)
                         options.chunkByRuntime = None
-                        flavor = self.getTestFlavor(options)
-                        if flavor in ('browser-chrome', 'devtools-chrome'):
+                        if options.flavor == 'browser':
                             # these values match current mozharness configs
                             options.chunkbyDir = 5
                         else:
@@ -1154,7 +1143,7 @@ def getTestManifest(self, options):
             assert manifestFileAbs.startswith(SCRIPT_DIR)
             manifest = TestManifest([manifestFileAbs], strict=False)
         else:
-            masterName = self.getTestFlavor(options) + '.ini'
+            masterName = self.normflavor(options.flavor) + '.ini'
             masterPath = os.path.join(SCRIPT_DIR, self.testRoot, masterName)
 
             if os.path.exists(masterPath):
@@ -1210,7 +1199,7 @@ def buildBrowserEnv(self, options, debugger=False, env=None):
         # inheriting all other inheritable handles as well.
         # We need to inherit them for plain mochitests for test logging purposes, so
         # we do so on the basis of a specific environment variable.
-        if self.getTestFlavor(options) == "mochitest":
+        if options.flavor == 'plain':
             browserEnv["MOZ_WIN_INHERIT_STD_HANDLES_PRE_VISTA"] = "1"
 
         # interpolate environment passed with options
@@ -1634,7 +1623,7 @@ def fillCertificateDB(self, options):
 
     def buildProfile(self, options):
         """ create the profile and add optional chrome bits and files if requested """
-        if options.browserChrome and options.timeout:
+        if options.flavor == 'browser' and options.timeout:
             options.extraPrefs.append(
                 "testing.browserTestHarness.timeout=%d" %
                 options.timeout)
@@ -2111,21 +2100,28 @@ def initializeLooping(self, options):
         self.urlOpts = []
 
     def resolve_runtime_file(self, options):
+        """
+        Return a path to the runtimes file for a given flavor and
+        subsuite.
+        """
+        template = "mochitest{e10s}-{suite_slug}.runtimes.json"
         data_dir = os.path.join(SCRIPT_DIR, 'runtimes')
 
-        flavor = self.getTestFlavor(options)
-        if flavor == 'browser-chrome' and options.subsuite == 'devtools':
-            flavor = 'devtools-chrome'
-        elif flavor == 'mochitest':
-            flavor = 'plain'
+        # Determine the suite slug in the runtimes file name
+        slug = self.normflavor(options.flavor)
+        if slug == 'browser-chrome' and options.subsuite == 'devtools':
+            slug = 'devtools-chrome'
+        elif slug == 'mochitest':
+            slug = 'plain'
             if options.subsuite:
-                flavor = options.subsuite
+                slug = options.subsuite
 
-        base = 'mochitest'
+        e10s = ''
         if options.e10s:
-            base = '{}-e10s'.format(base)
-        return os.path.join(data_dir, '{}-{}.runtimes.json'.format(
-            base, flavor))
+            e10s = '-e10s'
+
+        return os.path.join(data_dir, template.format(
+            e10s=e10s, suite_slug=slug))
 
     def normalize_paths(self, paths):
         # Normalize test paths so they are relative to test root
@@ -2195,7 +2191,7 @@ def runTests(self, options):
 
         # a11y and chrome tests don't run with e10s enabled in CI. Need to set
         # this here since |mach mochitest| sets the flavor after argument parsing.
-        if options.a11y or options.chrome:
+        if options.flavor in ('a11y', 'chrome'):
             options.e10s = False
         mozinfo.update({"e10s": options.e10s})  # for test manifest parsing.
 
@@ -2241,7 +2237,7 @@ def runTests(self, options):
         e10s_mode = "e10s" if options.e10s else "non-e10s"
 
         # printing total number of tests
-        if options.browserChrome:
+        if options.flavor == 'browser':
             print "TEST-INFO | checking window state"
             print "Browser Chrome Test Summary"
             print "\tPassed: %s" % self.countpass
@@ -2386,9 +2382,9 @@ def doTests(self, options, testsToFilter=None):
 
             # detect shutdown leaks for m-bc runs
             detectShutdownLeaks = mozinfo.info[
-                "debug"] and options.browserChrome
+                "debug"] and options.flavor == 'browser'
 
-            self.start_script_args.append(self.getTestFlavor(options))
+            self.start_script_args.append(self.normflavor(options.flavor))
             marionette_args = {
                 'symbols_path': options.symbolsPath,
                 'socket_timeout': options.marionette_socket_timeout,
@@ -2677,13 +2673,7 @@ def run_test_harness(parser, options):
 
     options.runByDir = False
 
-    if runner.getTestFlavor(options) == 'mochitest':
-        options.runByDir = True
-
-    if runner.getTestFlavor(options) == 'browser-chrome':
-        options.runByDir = True
-
-    if runner.getTestFlavor(options) == 'chrome':
+    if options.flavor in ('plain', 'browser', 'chrome'):
         options.runByDir = True
 
     if mozinfo.info.get('buildapp') == 'mulet':