From 225b100443cdbf736cdc7a8f40cbf1da8d60208e Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Thu, 4 Jul 2019 06:25:11 +0000 Subject: [PATCH] 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 | 31 ++++++++++--------- tools/tryselect/selectors/chooser/__init__.py | 7 +++-- tools/tryselect/selectors/coverage.py | 7 +++-- tools/tryselect/selectors/empty.py | 7 +++-- tools/tryselect/selectors/fuzzy.py | 7 +++-- tools/tryselect/test/test_again.py | 17 ++++++++-- 6 files changed, 48 insertions(+), 28 deletions(-) diff --git a/tools/tryselect/push.py b/tools/tryselect/push.py index 31c17ec0a34a..01de821d6843 100644 --- a/tools/tryselect/push.py +++ b/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) diff --git a/tools/tryselect/selectors/chooser/__init__.py b/tools/tryselect/selectors/chooser/__init__.py index 46290159a336..74846264f07b 100644 --- a/tools/tryselect/selectors/chooser/__init__.py +++ b/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) diff --git a/tools/tryselect/selectors/coverage.py b/tools/tryselect/selectors/coverage.py index 70a82069fcb3..bf4652c90a44 100644 --- a/tools/tryselect/selectors/coverage.py +++ b/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) diff --git a/tools/tryselect/selectors/empty.py b/tools/tryselect/selectors/empty.py index 6628bd29c683..c434db851b89 100644 --- a/tools/tryselect/selectors/empty.py +++ b/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) diff --git a/tools/tryselect/selectors/fuzzy.py b/tools/tryselect/selectors/fuzzy.py index bfc65511d013..db315c44b9c1 100644 --- a/tools/tryselect/selectors/fuzzy.py +++ b/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) diff --git a/tools/tryselect/test/test_again.py b/tools/tryselect/test/test_again.py index 7cba830d277f..f256c40cbab1 100644 --- a/tools/tryselect/test/test_again.py +++ b/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