Bug 1391675 - [tryselect] Move --no-push into common arguments, r=armenzg
authorAndrew Halberstadt <ahalberstadt@mozilla.com>
Mon, 21 Aug 2017 13:14:31 -0400
changeset 376420 a2a371e7e6f861a1711c7bae5bf94037bdff6f56
parent 376419 6f29f12cd26d1ba7a568615aaea973bcd9b9ec07
child 376421 c4fc5865ac38643b28e83e1155e8de048e6b4c1c
push id94095
push userkwierso@gmail.com
push dateThu, 24 Aug 2017 01:09:53 +0000
treeherdermozilla-inbound@d1c70c20e7b5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarmenzg
bugs1391675
milestone57.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 1391675 - [tryselect] Move --no-push into common arguments, r=armenzg The main motivation behind this change is to make testing easier, so e.g: ./mach try fuzzy --no-push and ./mach try syntax --no-push both work the same way. MozReview-Commit-ID: LmjA3Kq7xKN
tools/tryselect/cli.py
tools/tryselect/selectors/fuzzy.py
tools/tryselect/selectors/syntax.py
tools/tryselect/vcs.py
--- a/tools/tryselect/cli.py
+++ b/tools/tryselect/cli.py
@@ -6,17 +6,25 @@ from __future__ import absolute_import, 
 
 from argparse import ArgumentParser
 
 from .templates import all_templates
 
 
 class BaseTryParser(ArgumentParser):
     name = 'try'
-    common_arguments = []
+    common_arguments = [
+        [['--no-push'],
+         {'dest': 'push',
+          'action': 'store_false',
+          'help': 'Do not push to try as a result of running this command (if '
+                  'specified this command will only print calculated try '
+                  'syntax and selection info).',
+          }],
+    ]
     arguments = []
     templates = []
 
     def __init__(self, *args, **kwargs):
         ArgumentParser.__init__(self, *args, **kwargs)
 
         group = self.add_argument_group("{} arguments".format(self.name))
         for cli, kwargs in self.arguments:
--- a/tools/tryselect/selectors/fuzzy.py
+++ b/tools/tryselect/selectors/fuzzy.py
@@ -191,25 +191,26 @@ def fzf_bootstrap(update=False):
 def format_header():
     shortcuts = []
     for action, key in sorted(fzf_header_shortcuts.iteritems()):
         shortcuts.append('{t.white}{action}{t.normal}: {t.yellow}<{key}>{t.normal}'.format(
                          t=terminal, action=action, key=key))
     return FZF_HEADER.format(shortcuts=', '.join(shortcuts), t=terminal)
 
 
-def run_fuzzy_try(update=False, query=None, templates=None, full=False, parameters=None, **kwargs):
+def run_fuzzy_try(update=False, query=None, templates=None, full=False, parameters=None,
+                  push=True, **kwargs):
     fzf = fzf_bootstrap(update)
 
     if not fzf:
         print(FZF_NOT_FOUND)
         return
 
     vcs = VCSHelper.create()
-    vcs.check_working_directory()
+    vcs.check_working_directory(push)
 
     all_tasks = generate_tasks(parameters, full)
 
     key_shortcuts = [k + ':' + v for k, v in fzf_shortcuts.iteritems()]
     cmd = [
         fzf, '-m',
         '--bind', ','.join(key_shortcuts),
         '--header', format_header(),
@@ -225,9 +226,9 @@ def run_fuzzy_try(update=False, query=No
     proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stdin=subprocess.PIPE)
     selected = proc.communicate('\n'.join(all_tasks))[0].splitlines()
 
     if not selected:
         print("no tasks selected")
         return
 
     msg = "Pushed via 'mach try fuzzy', see diff for scheduled tasks"
-    return vcs.push_to_try(msg, selected, templates)
+    return vcs.push_to_try(msg, selected, templates, push=push)
--- a/tools/tryselect/selectors/syntax.py
+++ b/tools/tryselect/selectors/syntax.py
@@ -52,23 +52,16 @@ class SyntaxParser(BaseTryParser):
           'help': 'Restrict tests to the given tag (may be specified multiple times).',
           }],
         [['--and'],
          {'action': 'store_true',
           'dest': 'intersection',
           'help': 'When -u and paths are supplied run only the intersection of the '
                   'tests specified by the two arguments.',
           }],
-        [['--no-push'],
-         {'dest': 'push',
-          'action': 'store_false',
-          'help': 'Do not push to try as a result of running this command (if '
-                  'specified this command will only print calculated try '
-                  'syntax and selection info).',
-          }],
         [['--save'],
          {'dest': 'save',
           'action': 'store',
           'help': 'Save the command line arguments for future use with --preset.',
           }],
         [['--preset'],
          {'dest': 'load',
           'action': 'store',
@@ -701,16 +694,15 @@ class AutoTry(object):
                       '--artifact flag in try syntax (use --no-artifact '
                       'to override)')
 
         if kwargs["verbose"] and paths_by_flavor:
             print('The following tests will be selected: ')
             for flavor, paths in paths_by_flavor.iteritems():
                 print("%s: %s" % (flavor, ",".join(paths)))
 
-        if kwargs["verbose"] or not kwargs["push"]:
+        if kwargs["verbose"]:
             print('The following try syntax was calculated:\n%s' % msg)
 
-        if kwargs["push"]:
-            self.vcs.push_to_try(msg)
+        self.vcs.push_to_try(msg, push=kwargs['push'])
 
         if kwargs["save"] is not None:
             self.save_config(kwargs["save"], msg)
--- a/tools/tryselect/vcs.py
+++ b/tools/tryselect/vcs.py
@@ -81,83 +81,97 @@ class VCSHelper(object):
         with open(config, 'w') as fh:
             try_task_config = {'tasks': sorted(labels)}
             if templates:
                 try_task_config['templates'] = templates
 
             json.dump(try_task_config, fh, indent=2)
         return config
 
-    def check_working_directory(self):
+    def check_working_directory(self, push=True):
+        if not push:
+            return
+
         if self.has_uncommitted_changes:
             print(UNCOMMITTED_CHANGES)
             sys.exit(1)
 
+    def push_to_try(self, msg, labels=None, templates=None, push=True):
+        self.check_working_directory(push)
+
+        config = None
+        if labels:
+            config = self.write_task_config(labels, templates)
+
+        try:
+            if not push:
+                print("Calculated try selector:")
+                if config:
+                    with open(config) as fh:
+                        print(fh.read())
+                else:
+                    print(msg)
+                return
+
+            self._push_to_try(msg, config)
+        finally:
+            if config and os.path.isfile(config):
+                os.remove(config)
+
     @abstractmethod
-    def push_to_try(self, msg, labels=None):
+    def _push_to_try(self, msg, config):
         pass
 
     @abstractproperty
     def files_changed(self):
         pass
 
     @abstractproperty
     def has_uncommitted_changes(self):
         pass
 
 
 class HgHelper(VCSHelper):
 
-    def push_to_try(self, msg, labels=None, templates=None):
-        self.check_working_directory()
-
-        if labels:
-            config = self.write_task_config(labels, templates)
-            self.run(['hg', 'add', config])
-
+    def _push_to_try(self, msg, config):
         try:
+            if config:
+                self.run(['hg', 'add', config])
             return subprocess.check_call(['hg', 'push-to-try', '-m', msg])
         except subprocess.CalledProcessError:
             try:
                 self.run(['hg', 'showconfig', 'extensions.push-to-try'])
             except subprocess.CalledProcessError:
                 print(HG_PUSH_TO_TRY_NOT_FOUND)
             return 1
         finally:
             self.run(['hg', 'revert', '-a'])
 
-            if labels and os.path.isfile(config):
-                os.remove(config)
-
     @property
     def files_changed(self):
         return self.run(['hg', 'log', '-r', '::. and not public()',
                          '--template', '{join(files, "\n")}\n'])
 
     @property
     def has_uncommitted_changes(self):
         stat = [s for s in self.run(['hg', 'status', '-amrn']).split() if s]
         return len(stat) > 0
 
 
 class GitHelper(VCSHelper):
 
-    def push_to_try(self, msg, labels=None, templates=None):
-        self.check_working_directory()
-
+    def _push_to_try(self, msg, config):
         try:
             subprocess.check_output(['git', 'cinnabar', '--version'], stderr=subprocess.STDOUT)
         except subprocess.CalledProcessError:
             print(GIT_CINNABAR_NOT_FOUND)
             return 1
 
-        if labels:
-            config = self.write_task_config(labels, templates)
+        if config:
             self.run(['git', 'add', config])
-
         subprocess.check_call(['git', 'commit', '--allow-empty', '-m', msg])
         try:
             return subprocess.call(['git', 'push', 'hg::ssh://hg.mozilla.org/try',
                                     '+HEAD:refs/heads/branches/default/tip'])
         finally:
             self.run(['git', 'reset', 'HEAD~'])
 
     @property