Bug 1236382 - Add commonly used arguments to mach try, remove the extra arguments functionality. r=jgraham draft
authorChris Manchester <cmanchester@mozilla.com>
Mon, 04 Jan 2016 14:07:11 -0800
changeset 329602 147cbd57e595c679649c0d752b456e57bec08319
parent 329145 1dbe350b57b17ec1ce2887441b79c6f51b429378
child 513994 f320077788362c83b8cce2f065f8e11d51d41917
push id10563
push usercmanchester@mozilla.com
push dateMon, 08 Feb 2016 19:12:26 +0000
reviewersjgraham
bugs1236382
milestone47.0a1
Bug 1236382 - Add commonly used arguments to mach try, remove the extra arguments functionality. r=jgraham Using nargs='*' in conjunction with nargs=REMAINDER creates an ambiguity when the argument using nargs='*' is optional, it is not specified, and the user intends their arguments to be interpreted as "extra" arguments. This commit removes the nargs=REMAINDER argument for mach_try, and implements its most common users from trychooser as a set of regular arguments to echo to generated try syntax.
testing/mach_commands.py
testing/tools/autotry/autotry.py
--- a/testing/mach_commands.py
+++ b/testing/mach_commands.py
@@ -497,16 +497,17 @@ class PushToTry(MachCommandBase):
         if not allow_subitems:
             if not all(item == [] for item in rv.itervalues()):
                 raise ValueError("Unexpected subitems in argument")
             return rv.keys()
         else:
             return rv
 
     def validate_args(self, **kwargs):
+        from autotry import AutoTry
         if not kwargs["paths"] and not kwargs["tests"] and not kwargs["tags"]:
             print("Paths, tags, or tests must be specified as an argument to autotry.")
             sys.exit(1)
 
         if kwargs["platforms"] is None:
             if 'AUTOTRY_PLATFORM_HINT' in os.environ:
                 kwargs["platforms"] = [os.environ['AUTOTRY_PLATFORM_HINT']]
             else:
@@ -547,17 +548,21 @@ class PushToTry(MachCommandBase):
             paths.append(os.path.relpath(p, self.topsrcdir))
 
         try:
             tags = self.normalise_list(kwargs["tags"]) if kwargs["tags"] else []
         except ValueError as e:
             print("Error parsing --tags argument:\n%s" % e.message)
             sys.exit(1)
 
-        return kwargs["builds"], platforms, tests, talos, paths, tags, kwargs["extra_args"]
+        extra_values = {k['dest'] for k in AutoTry.pass_through_arguments.values()}
+        extra_args = {k: v for k, v in kwargs.items()
+                      if k in extra_values and v}
+
+        return kwargs["builds"], platforms, tests, talos, paths, tags, extra_args
 
 
     @Command('try',
              category='testing',
              description='Push selected tests to the try server',
              parser=autotry_parser)
 
     def autotry(self, **kwargs):
@@ -627,17 +632,17 @@ class PushToTry(MachCommandBase):
 
         if kwargs["push"] and at.find_uncommited_changes():
             print('ERROR please commit changes before continuing')
             sys.exit(1)
 
         if not any(kwargs[item] for item in ("paths", "tests", "tags")):
             kwargs["paths"], kwargs["tags"] = at.find_paths_and_tags(kwargs["verbose"])
 
-        builds, platforms, tests, talos, paths, tags, extra_args = self.validate_args(**kwargs)
+        builds, platforms, tests, talos, paths, tags, extra = self.validate_args(**kwargs)
 
         if paths or tags:
             driver = self._spawn(BuildDriver)
             driver.install_tests(remove=False)
 
             paths = [os.path.relpath(os.path.normpath(os.path.abspath(item)), self.topsrcdir)
                      for item in paths]
             paths_by_flavor = at.paths_by_flavor(paths=paths, tags=tags)
@@ -649,17 +654,17 @@ class PushToTry(MachCommandBase):
 
             if not kwargs["intersection"]:
                 paths_by_flavor = at.remove_duplicates(paths_by_flavor, tests)
         else:
             paths_by_flavor = {}
 
         try:
             msg = at.calc_try_syntax(platforms, tests, talos, builds, paths_by_flavor, tags,
-                                     extra_args, kwargs["intersection"])
+                                     extra, kwargs["intersection"])
         except ValueError as e:
             print(e.message)
             sys.exit(1)
 
         if kwargs["verbose"] and paths_by_flavor:
             print('The following tests will be selected: ')
             for flavor, paths in paths_by_flavor.iteritems():
                 print("%s: %s" % (flavor, ",".join(paths)))
--- a/testing/tools/autotry/autotry.py
+++ b/testing/tools/autotry/autotry.py
@@ -35,21 +35,21 @@ def arg_parser():
                         'specified this command will only print calculated try '
                         'syntax and selection info).')
     parser.add_argument('--save', dest="save", action='store',
                         help="Save the command line arguments for future use with --preset.")
     parser.add_argument('--preset', dest="load", action='store',
                         help="Load a saved set of arguments. Additional arguments will override saved ones.")
     parser.add_argument('--list', action='store_true',
                         help="List all saved try strings")
-    parser.add_argument('extra_args', nargs=argparse.REMAINDER,
-                        help='Extra arguments to put in the try push.')
     parser.add_argument('-v', "--verbose", dest='verbose', action='store_true', default=False,
                         help='Print detailed information about the resulting test selection '
                         'and commands performed.')
+    for arg, opts in AutoTry.pass_through_arguments.items():
+        parser.add_argument(arg, **opts)
     return parser
 
 class TryArgumentTokenizer(object):
     symbols = [("seperator", ","),
                ("list_start", "\["),
                ("list_end", "\]"),
                ("item", "([^,\[\]\s][^,\[\]]+)"),
                ("space", "\s+")]
@@ -166,16 +166,48 @@ class AutoTry(object):
         "chrome": "mochitest-o",
         "browser-chrome": "mochitest-bc",
         "devtools-chrome": "mochitest-dt",
         "crashtest": "crashtest",
         "reftest": "reftest",
         "web-platform-tests": "web-platform-tests",
     }
 
+    # Arguments we will accept on the command line and pass through to try
+    # syntax with no further intervention. The set is taken from
+    # http://trychooser.pub.build.mozilla.org
+    pass_through_arguments = {
+        '--rebuild': {
+            'action': 'store',
+            'dest': 'rebuild',
+            'help': 'Re-trigger all test jobs (up to 20 times)',
+        },
+        '--rebuild-talos': {
+            'action': 'store',
+            'dest': 'rebuild_talos',
+            'help': 'Re-trigger all talos jobs',
+        },
+        '--interactive': {
+            'action': 'store_true',
+            'dest': 'interactive',
+            'help': 'Allow ssh-like access to running test containers',
+        },
+        '--no-retry': {
+            'action': 'store_true',
+            'dest': 'no_retry',
+            'help': 'Do not retrigger failed tests',
+        },
+        '--setenv': {
+            'action': 'append',
+            'dest': 'setenv',
+            'help': 'Set the corresponding variable in the test environment for'
+                    'applicable harnesses.',
+        }
+    }
+
     def __init__(self, topsrcdir, resolver_func, mach_context):
         self.topsrcdir = topsrcdir
         self._resolver_func = resolver_func
         self._resolver = None
         self.mach_context = mach_context
 
         if os.path.exists(os.path.join(self.topsrcdir, '.hg')):
             self._use_git = False
@@ -294,17 +326,17 @@ class AutoTry(object):
     def remove_duplicates(self, paths_by_flavor, tests):
         rv = {}
         for item in paths_by_flavor:
             if self.flavor_suites[item] not in tests:
                 rv[item] = paths_by_flavor[item].copy()
         return rv
 
     def calc_try_syntax(self, platforms, tests, talos, builds, paths_by_flavor, tags,
-                        extra_args, intersection):
+                        extras, intersection):
         parts = ["try:", "-b", builds, "-p", ",".join(platforms)]
 
         suites = tests if not intersection else {}
         paths = set()
         for flavor, flavor_tests in paths_by_flavor.iteritems():
             suite = self.flavor_suites[flavor]
             if suite not in suites and (not intersection or suite in tests):
                 for job_name in self.flavor_jobs[flavor]:
@@ -321,22 +353,34 @@ class AutoTry(object):
 
         parts.append("-t")
         parts.append(",".join("%s%s" % (k, "[%s]" % ",".join(v) if v else "")
                               for k,v in sorted(talos.items())) if talos else "none")
 
         if tags:
             parts.append(' '.join('--tag %s' % t for t in tags))
 
-        if extra_args is not None:
-            parts.extend(extra_args)
-
         if paths:
             parts.append("--try-test-paths %s" % " ".join(sorted(paths)))
 
+        args_by_dest = {v['dest']: k for k, v in AutoTry.pass_through_arguments.items()}
+        for dest, value in extras.iteritems():
+            assert dest in args_by_dest
+            arg = args_by_dest[dest]
+            action = AutoTry.pass_through_arguments[arg]['action']
+            if action == 'store':
+                parts.append(arg)
+                parts.append(value)
+            if action == 'append':
+                for e in value:
+                    parts.append(arg)
+                    parts.append(e)
+            if action == 'store_true':
+                parts.append(arg)
+
         return " ".join(parts)
 
     def _run_git(self, *args):
         args = ['git'] + list(args)
         ret = subprocess.call(args)
         if ret:
             print('ERROR git command %s returned %s' %
                   (args, ret))