Bug 1562287: Factor out generation of try_task_config; r=ahal

Factor out the logic for calculating `try_task_config` from `push_to_try`,
so it can be called only for those selectors that need it.

Differential Revision: https://phabricator.services.mozilla.com/D36363

--HG--
extra : moz-landing-system : lando

tools/tryselect/push.py

@@ -79,7 +79,21 @@ def check_working_directory(push=True):
         sys.exit(1)
 
 
-def push_to_try(method, msg, labels=None, templates=None, try_task_config=None,
+def generate_try_task_config(method, labels, templates=None):
+    if templates is not None:
+        templates.setdefault('env', {}).update({'TRY_SELECTOR': method})
+
+    try_task_config = {
+        '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)
 
@@ -88,22 +102,11 @@ def push_to_try(method, msg, labels=None, templates=None, try_task_config=None,
     commit_message = ('%s%s\n\nPushed via `mach try %s`' %
                       (msg, closed_tree_string, method))
 
-    if templates is not None:
-        templates.setdefault('env', {}).update({'TRY_SELECTOR': method})
-
-    if labels or labels == []:
-        try_task_config = {
-            'version': 1,
-            'tasks': sorted(labels),
-        }
-        if templates:
-            try_task_config['templates'] = templates
-        if push:
-            write_task_config_history(msg, try_task_config)
-
     config_path = None
     changed_files = []
     if try_task_config:
+        if push and method != 'again':
+            write_task_config_history(msg, try_task_config)
         config_path = write_task_config(try_task_config)
         changed_files.append(config_path)
 

tools/tryselect/selectors/chooser/__init__.py

@@ -10,7 +10,7 @@ from threading import Timer
 
 from tryselect.cli import BaseTryParser
 from tryselect.tasks import generate_tasks
-from tryselect.push import check_working_directory, push_to_try
+from tryselect.push import check_working_directory, push_to_try, generate_try_task_config
 
 here = os.path.abspath(os.path.dirname(__file__))
 
@@ -48,5 +48,6 @@ def run(update=False, query=None, templates=None, full=False, parameters=None,
         return
 
     msg = "Try Chooser Enhanced ({} tasks selected)".format(len(selected))
-    return push_to_try('chooser', message.format(msg=msg), selected, templates, push=push,
-                       closed_tree=closed_tree)
+    return push_to_try('chooser', message.format(msg=msg),
+                       try_task_config=generate_try_task_config('chooser', selected, templates),
+                       push=push, closed_tree=closed_tree)

tools/tryselect/selectors/coverage.py

@@ -23,7 +23,7 @@ from mozversioncontrol import get_repository_object
 
 from ..cli import BaseTryParser
 from ..tasks import generate_tasks, filter_tasks_by_paths, resolve_tests_by_suite
-from ..push import push_to_try
+from ..push import push_to_try, generate_try_task_config
 
 here = os.path.abspath(os.path.dirname(__file__))
 build = MozbuildObject.from_environment(cwd=here)
@@ -378,5 +378,6 @@ def run(templates={}, full=False, parameters=None, push=True, message='{msg}', c
 
     # Build commit message.
     msg = 'try coverage - ' + test_count_message
-    return push_to_try('coverage', message.format(msg=msg), tasks, templates, push=push,
-                       closed_tree=closed_tree)
+    return push_to_try('coverage', message.format(msg=msg),
+                       try_task_config=generate_try_task_config('coverage', tasks, templates),
+                       push=push, closed_tree=closed_tree)

tools/tryselect/selectors/empty.py

@@ -5,7 +5,7 @@
 from __future__ import absolute_import, print_function, unicode_literals
 
 from ..cli import BaseTryParser
-from ..push import push_to_try
+from ..push import push_to_try, generate_try_task_config
 
 
 class EmptyParser(BaseTryParser):
@@ -15,5 +15,6 @@ class EmptyParser(BaseTryParser):
 
 def run(message='{msg}', push=True, closed_tree=False):
     msg = 'No try selector specified, use "Add New Jobs" to select tasks.'
-    return push_to_try('empty', message.format(msg=msg), [], push=push,
-                       closed_tree=closed_tree)
+    return push_to_try('empty', message.format(msg=msg),
+                       try_task_config=generate_try_task_config('empty', []),
+                       push=push, closed_tree=closed_tree)

tools/tryselect/selectors/fuzzy.py

@@ -16,7 +16,7 @@ from mozterm import Terminal
 
 from ..cli import BaseTryParser
 from ..tasks import generate_tasks, filter_tasks_by_paths
-from ..push import check_working_directory, push_to_try
+from ..push import check_working_directory, push_to_try, generate_try_task_config
 
 terminal = Terminal()
 
@@ -295,5 +295,6 @@ def run(update=False, query=None, intersect_query=None, templates=None, full=Fal
         args.append("paths={}".format(':'.join(test_paths)))
     if args:
         msg = "{} {}".format(msg, '&'.join(args))
-    return push_to_try('fuzzy', message.format(msg=msg), selected, templates, push=push,
-                       closed_tree=closed_tree)
+    return push_to_try('fuzzy', message.format(msg=msg),
+                       try_task_config=generate_try_task_config('fuzzy', selected, templates),
+                       push=push, closed_tree=closed_tree)

tools/tryselect/test/test_again.py

@@ -20,7 +20,13 @@ def patch_history_path(tmpdir, monkeypatch):
 
 
 def test_try_again(monkeypatch):
-    push.push_to_try('fuzzy', 'Fuzzy message', ['foo', 'bar'], {'artifact': True})
+    push.push_to_try(
+        "fuzzy",
+        "Fuzzy message",
+        try_task_config=push.generate_try_task_config(
+            "fuzzy", ["foo", "bar"], {"artifact": True},
+        ),
+    )
 
     assert os.path.isfile(push.history_path)
     with open(push.history_path, 'r') as fh:
@@ -51,7 +57,14 @@ def test_try_again(monkeypatch):
 def test_no_push_does_not_generate_history(tmpdir):
     assert not os.path.isfile(push.history_path)
 
-    push.push_to_try('fuzzy', 'Fuzzy', ['foo', 'bar'], {'artifact': True}, push=False)
+    push.push_to_try(
+        "fuzzy",
+        "Fuzzy",
+        try_task_config=push.generate_try_task_config(
+            "fuzzy", ["foo", "bar"], {"artifact": True},
+        ),
+        push=False,
+    )
     assert not os.path.isfile(push.history_path)
     assert again.run() == 1