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.

MozReview-Commit-ID: FOuDZxUfHu9
This commit is contained in:
Chris Manchester 2016-01-04 14:07:11 -08:00
parent 1ba302d1f5
commit a2f2f2792b
2 changed files with 63 additions and 9 deletions

View File

@ -502,6 +502,7 @@ class PushToTry(MachCommandBase):
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)
@ -552,7 +553,11 @@ class PushToTry(MachCommandBase):
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',
@ -632,7 +637,7 @@ class PushToTry(MachCommandBase):
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)
@ -654,7 +659,7 @@ class PushToTry(MachCommandBase):
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)

View File

@ -40,11 +40,11 @@ def arg_parser():
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):
@ -171,6 +171,43 @@ class AutoTry(object):
"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 with a few additions.
#
# Note that the meaning of store_false and store_true arguments is
# not preserved here, as we're only using these to echo the literal
# arguments to another consumer. Specifying either store_false or
# store_true here will have an equivalent effect.
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
@ -299,7 +336,7 @@ class AutoTry(object):
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 {}
@ -326,12 +363,24 @@ class AutoTry(object):
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 in ('store_true', 'store_false'):
parts.append(arg)
return " ".join(parts)
def _run_git(self, *args):