Bug 1562287: Allow setting other try_task_config settings than `templates`; r=ahal
authorTom Prince <mozilla@hocat.ca>
Thu, 04 Jul 2019 06:25:18 +0000
changeset 540973 e5c77a0eaa15b376361ca5837a3d75b228584471
parent 540972 08361960eaf4014b5d7c25ccf4a4baaeaeea8107
child 540974 9e1f67f66b607808fa3c4a1a879e1d36abe491f3
push id11533
push userarchaeopteryx@coole-files.de
push dateMon, 08 Jul 2019 18:18:03 +0000
treeherdermozilla-beta@f4452e031aed [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersahal
bugs1562287
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1562287: Allow setting other try_task_config settings than `templates`; r=ahal Templates invoke the `morph` logic, which is somewhat confusing and inflexible. Update the machinery to support setting other `try_task_config` values. Differential Revision: https://phabricator.services.mozilla.com/D36364
tools/tryselect/mach_commands.py
tools/tryselect/push.py
tools/tryselect/selectors/chooser/__init__.py
tools/tryselect/selectors/coverage.py
tools/tryselect/selectors/fuzzy.py
tools/tryselect/templates.py
tools/tryselect/test/test_again.py
--- a/tools/tryselect/mach_commands.py
+++ b/tools/tryselect/mach_commands.py
@@ -13,16 +13,17 @@ from mach.decorators import (
     CommandProvider,
     Command,
     SettingsProvider,
     SubCommand,
 )
 from mozboot.util import get_state_dir
 from mozbuild.base import BuildEnvironmentNotFoundException, MachCommandBase
 
+
 CONFIG_ENVIRONMENT_NOT_FOUND = '''
 No config environment detected. This means we are unable to properly
 detect test files in the specified paths or tags. Please run:
 
     $ mach configure
 
 and try again.
 '''.lstrip()
@@ -152,34 +153,35 @@ class TrySelect(MachCommandBase):
                     defaults[k] = v
                 else:
                     nondefaults[k] = v
 
             kwargs = merge(defaults, preset, nondefaults)
 
         return kwargs
 
-    def handle_templates(self, **kwargs):
-        kwargs.setdefault('templates', {})
+    def handle_try_config(self, **kwargs):
+        from tryselect.util.dicttools import merge
+        kwargs.setdefault('try_config', {})
         for cls in self.parser.templates.itervalues():
-            context = cls.context(**kwargs)
-            if context is not None:
-                kwargs['templates'].update(context)
+            try_config = cls.try_config(**kwargs)
+            if try_config is not None:
+                kwargs['try_config'] = merge(kwargs['try_config'], try_config)
 
             for name in cls.dests:
                 del kwargs[name]
 
         return kwargs
 
     def run(self, **kwargs):
         if 'preset' in self.parser.common_groups:
             kwargs = self.handle_presets(**kwargs)
 
         if self.parser.templates:
-            kwargs = self.handle_templates(**kwargs)
+            kwargs = self.handle_try_config(**kwargs)
 
         mod = importlib.import_module('tryselect.selectors.{}'.format(self.subcommand))
         return mod.run(**kwargs)
 
     @Command('try',
              category='ci',
              description='Push selected tasks to the try server',
              parser=generic_parser)
--- a/tools/tryselect/push.py
+++ b/tools/tryselect/push.py
@@ -74,26 +74,26 @@ def check_working_directory(push=True):
     if not push:
         return
 
     if not vcs.working_directory_clean():
         print(UNCOMMITTED_CHANGES)
         sys.exit(1)
 
 
-def generate_try_task_config(method, labels, templates=None):
-    if templates is not None:
-        templates.setdefault('env', {}).update({'TRY_SELECTOR': method})
+def generate_try_task_config(method, labels, try_config=None):
+    try_task_config = try_config or {}
 
-    try_task_config = {
+    templates = try_task_config.setdefault('templates', {})
+    templates.setdefault('env', {}).update({'TRY_SELECTOR': method})
+
+    try_task_config.update({
         'version': 1,
         'tasks': sorted(labels),
-    }
-    if templates:
-        try_task_config['templates'] = templates
+    })
 
     return try_task_config
 
 
 def push_to_try(method, msg, try_task_config=None,
                 push=True, closed_tree=False, files_to_change=None):
     check_working_directory(push)
 
--- a/tools/tryselect/selectors/chooser/__init__.py
+++ b/tools/tryselect/selectors/chooser/__init__.py
@@ -17,17 +17,17 @@ here = os.path.abspath(os.path.dirname(_
 
 class ChooserParser(BaseTryParser):
     name = 'chooser'
     arguments = []
     common_groups = ['push', 'task']
     templates = ['artifact', 'env', 'rebuild', 'chemspill-prio', 'gecko-profile']
 
 
-def run(update=False, query=None, templates=None, full=False, parameters=None,
+def run(update=False, query=None, try_config=None, full=False, parameters=None,
         save=False, preset=None, mod_presets=False, push=True, message='{msg}',
         closed_tree=False):
     from .app import create_application
     check_working_directory(push)
 
     tg = generate_tasks(parameters, full)
     app = create_application(tg)
 
@@ -44,10 +44,10 @@ def run(update=False, query=None, templa
 
     selected = app.tasks
     if not selected:
         print("no tasks selected")
         return
 
     msg = "Try Chooser Enhanced ({} tasks selected)".format(len(selected))
     return push_to_try('chooser', message.format(msg=msg),
-                       try_task_config=generate_try_task_config('chooser', selected, templates),
+                       try_task_config=generate_try_task_config('chooser', selected, try_config),
                        push=push, closed_tree=closed_tree)
--- a/tools/tryselect/selectors/coverage.py
+++ b/tools/tryselect/selectors/coverage.py
@@ -338,17 +338,17 @@ def filter_tasks_by_chunks(tasks, chunks
 
 def is_opt_task(task):
     '''True if the task runs on a supported platform and build type combination.
     This is used to remove -ccov/asan/pgo tasks, along with all /debug tasks.
     '''
     return any(platform in task for platform in OPT_TASK_PATTERNS)
 
 
-def run(templates={}, full=False, parameters=None, push=True, message='{msg}', closed_tree=False):
+def run(try_config={}, full=False, parameters=None, push=True, message='{msg}', closed_tree=False):
     download_coverage_mapping(vcs.base_ref)
 
     changed_sources = vcs.get_outgoing_files()
     test_files, test_chunks = find_tests(changed_sources)
     if not test_files and not test_chunks:
         print('ERROR Could not find any tests or chunks to run.')
         return 1
 
@@ -369,15 +369,15 @@ def run(templates={}, full=False, parame
         test_plural='' if len(test_files) == 1 else 's',
         test_singular='s' if len(test_files) == 1 else '',
         task_count=len(tasks),
         task_plural='' if len(tasks) == 1 else 's')
     print('Found ' + test_count_message)
 
     # Set the test paths to be run by setting MOZHARNESS_TEST_PATHS.
     path_env = {'MOZHARNESS_TEST_PATHS': json.dumps(resolve_tests_by_suite(test_files))}
-    templates.setdefault('env', {}).update(path_env)
+    try_config.setdefault('templates', {}).setdefault('env', {}).update(path_env)
 
     # Build commit message.
     msg = 'try coverage - ' + test_count_message
     return push_to_try('coverage', message.format(msg=msg),
-                       try_task_config=generate_try_task_config('coverage', tasks, templates),
+                       try_task_config=generate_try_task_config('coverage', tasks, try_config),
                        push=push, closed_tree=closed_tree)
--- a/tools/tryselect/selectors/fuzzy.py
+++ b/tools/tryselect/selectors/fuzzy.py
@@ -214,17 +214,17 @@ def run_fzf(cmd, tasks):
         selected = out[1:]
     return query, selected
 
 
 def filter_target_task(task):
     return not any(re.search(pattern, task) for pattern in TARGET_TASK_FILTERS)
 
 
-def run(update=False, query=None, intersect_query=None, templates=None, full=False,
+def run(update=False, query=None, intersect_query=None, try_config=None, full=False,
         parameters=None, save_query=False, push=True, message='{msg}',
         test_paths=None, exact=False, closed_tree=False):
     fzf = fzf_bootstrap(update)
 
     if not fzf:
         print(FZF_NOT_FOUND)
         return 1
 
@@ -291,10 +291,10 @@ def run(update=False, query=None, inters
     # build commit message
     msg = "Fuzzy"
     args = ["query={}".format(q) for q in queries]
     if test_paths:
         args.append("paths={}".format(':'.join(test_paths)))
     if args:
         msg = "{} {}".format(msg, '&'.join(args))
     return push_to_try('fuzzy', message.format(msg=msg),
-                       try_task_config=generate_try_task_config('fuzzy', selected, templates),
+                       try_task_config=generate_try_task_config('fuzzy', selected, try_config),
                        push=push, closed_tree=closed_tree)
--- a/tools/tryselect/templates.py
+++ b/tools/tryselect/templates.py
@@ -18,32 +18,43 @@ from argparse import Action, SUPPRESS
 import mozpack.path as mozpath
 from mozbuild.base import BuildEnvironmentNotFoundException, MozbuildObject
 from .tasks import resolve_tests_by_suite
 
 here = os.path.abspath(os.path.dirname(__file__))
 build = MozbuildObject.from_environment(cwd=here)
 
 
-class Template(object):
+class TryConfig(object):
     __metaclass__ = ABCMeta
 
     def __init__(self):
         self.dests = set()
 
     def add_arguments(self, parser):
         for cli, kwargs in self.arguments:
             action = parser.add_argument(*cli, **kwargs)
             self.dests.add(action.dest)
 
     @abstractproperty
     def arguments(self):
         pass
 
     @abstractmethod
+    def try_config(self, **kwargs):
+        pass
+
+
+class Template(TryConfig):
+    def try_config(self, **kwargs):
+        context = self.context(**kwargs)
+        if context:
+            return {'templates': context}
+
+    @abstractmethod
     def context(self, **kwargs):
         pass
 
 
 class Artifact(Template):
 
     arguments = [
         [['--artifact'],
--- a/tools/tryselect/test/test_again.py
+++ b/tools/tryselect/test/test_again.py
@@ -10,64 +10,64 @@ import mozunit
 import pytest
 
 from tryselect import push
 from tryselect.selectors import again
 
 
 @pytest.fixture(autouse=True)
 def patch_history_path(tmpdir, monkeypatch):
-    monkeypatch.setattr(push, 'history_path', tmpdir.join('history.json').strpath)
+    monkeypatch.setattr(push, "history_path", tmpdir.join("history.json").strpath)
     reload(again)
 
 
 def test_try_again(monkeypatch):
     push.push_to_try(
         "fuzzy",
         "Fuzzy message",
         try_task_config=push.generate_try_task_config(
-            "fuzzy", ["foo", "bar"], {"artifact": True},
+            "fuzzy", ["foo", "bar"], {"templates": {"artifact": True}},
         ),
     )
 
     assert os.path.isfile(push.history_path)
-    with open(push.history_path, 'r') as fh:
+    with open(push.history_path, "r") as fh:
         assert len(fh.readlines()) == 1
 
     def fake_push_to_try(*args, **kwargs):
         return args, kwargs
 
-    monkeypatch.setattr(push, 'push_to_try', fake_push_to_try)
+    monkeypatch.setattr(push, "push_to_try", fake_push_to_try)
     reload(again)
 
     args, kwargs = again.run()
 
-    assert args[0] == 'again'
-    assert args[1] == 'Fuzzy message'
+    assert args[0] == "again"
+    assert args[1] == "Fuzzy message"
 
-    try_task_config = kwargs.pop('try_task_config')
-    assert sorted(try_task_config.get('tasks')) == sorted(['foo', 'bar'])
-    assert try_task_config.get('templates') == {
-        'artifact': True,
-        'env': {'TRY_SELECTOR': 'fuzzy'},
+    try_task_config = kwargs.pop("try_task_config")
+    assert sorted(try_task_config.get("tasks")) == sorted(["foo", "bar"])
+    assert try_task_config.get("templates") == {
+        "artifact": True,
+        "env": {"TRY_SELECTOR": "fuzzy"},
     }
 
-    with open(push.history_path, 'r') as fh:
+    with open(push.history_path, "r") as fh:
         assert len(fh.readlines()) == 1
 
 
 def test_no_push_does_not_generate_history(tmpdir):
     assert not os.path.isfile(push.history_path)
 
     push.push_to_try(
         "fuzzy",
         "Fuzzy",
         try_task_config=push.generate_try_task_config(
-            "fuzzy", ["foo", "bar"], {"artifact": True},
+            "fuzzy", ["foo", "bar"], {"templates": {"artifact": True}},
         ),
         push=False,
     )
     assert not os.path.isfile(push.history_path)
     assert again.run() == 1
 
 
-if __name__ == '__main__':
+if __name__ == "__main__":
     mozunit.main()