Bug 1562287: Factor out generation of try_task_config; r=ahal
authorTom Prince <mozilla@hocat.ca>
Thu, 04 Jul 2019 06:25:11 +0000
changeset 544161 08361960eaf4014b5d7c25ccf4a4baaeaeea8107
parent 544160 e4d3721c6fad4d49ab1dcf1be6266a88445ca5f6
child 544162 e5c77a0eaa15b376361ca5837a3d75b228584471
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [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: 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
tools/tryselect/push.py
tools/tryselect/selectors/chooser/__init__.py
tools/tryselect/selectors/coverage.py
tools/tryselect/selectors/empty.py
tools/tryselect/selectors/fuzzy.py
tools/tryselect/test/test_again.py
--- a/tools/tryselect/push.py
+++ b/tools/tryselect/push.py
@@ -74,41 +74,44 @@ def check_working_directory(push=True):
     if not push:
         return
 
     if not vcs.working_directory_clean():
         print(UNCOMMITTED_CHANGES)
         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)
 
     # Format the commit message
     closed_tree_string = " ON A CLOSED TREE" if closed_tree else ""
     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)
 
     if files_to_change:
         for path, content in files_to_change.items():
             path = os.path.join(vcs.path, path)
             with open(path, 'w') as fh:
                 fh.write(content)
--- a/tools/tryselect/selectors/chooser/__init__.py
+++ b/tools/tryselect/selectors/chooser/__init__.py
@@ -5,17 +5,17 @@
 from __future__ import absolute_import, print_function, unicode_literals
 
 import os
 import webbrowser
 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__))
 
 
 class ChooserParser(BaseTryParser):
     name = 'chooser'
     arguments = []
     common_groups = ['push', 'task']
@@ -43,10 +43,11 @@ def run(update=False, query=None, templa
     app.run()
 
     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), 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)
--- a/tools/tryselect/selectors/coverage.py
+++ b/tools/tryselect/selectors/coverage.py
@@ -18,17 +18,17 @@ import datetime
 from mozboot.util import get_state_dir
 from mozbuild.base import MozbuildObject
 from mozpack.files import FileFinder
 from moztest.resolve import TestResolver
 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)
 vcs = get_repository_object(build.topsrcdir)
 
 root_hash = hashlib.sha256(os.path.abspath(build.topsrcdir)).hexdigest()
 cache_dir = os.path.join(get_state_dir(), 'cache', root_hash, 'chunk_mapping')
 if not os.path.isdir(cache_dir):
@@ -373,10 +373,11 @@ def run(templates={}, full=False, parame
     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)
 
     # 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)
--- a/tools/tryselect/selectors/empty.py
+++ b/tools/tryselect/selectors/empty.py
@@ -1,19 +1,20 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 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):
     name = 'empty'
     common_groups = ['push']
 
 
 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)
--- a/tools/tryselect/selectors/fuzzy.py
+++ b/tools/tryselect/selectors/fuzzy.py
@@ -11,17 +11,17 @@ import subprocess
 import sys
 from distutils.spawn import find_executable
 
 from mozboot.util import get_state_dir
 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()
 
 here = os.path.abspath(os.path.dirname(__file__))
 
 # Some tasks show up in the target task set, but are either special cases
 # or uncommon enough that they should only be selectable with --full.
 TARGET_TASK_FILTERS = (
@@ -290,10 +290,11 @@ 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), 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)
--- a/tools/tryselect/test/test_again.py
+++ b/tools/tryselect/test/test_again.py
@@ -15,17 +15,23 @@ 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)
     reload(again)
 
 
 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:
         assert len(fh.readlines()) == 1
 
     def fake_push_to_try(*args, **kwargs):
         return args, kwargs
 
@@ -46,15 +52,22 @@ def test_try_again(monkeypatch):
 
     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', ['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
 
 
 if __name__ == '__main__':
     mozunit.main()