Bug 1513951 - [tryselect] Implement in-tree try_presets.yml file r=gbrown
authorAndrew Halberstadt <ahalberstadt@mozilla.com>
Thu, 28 Feb 2019 21:38:26 +0000
changeset 519878 74de0c0022dc572d9a6aab43e723942836eb8afd
parent 519877 3ea24e73acf56ab31adf31b779df854e468a4906
child 519879 c66965245aaa69ae80df2a398c62d15ffd1f76eb
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgbrown
bugs1513951
milestone67.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 1513951 - [tryselect] Implement in-tree try_presets.yml file r=gbrown This creates a global preset file at: tools/tryselect/try_presets.yml Any presets defined here will be available for everyone to use. Differential Revision: https://phabricator.services.mozilla.com/D21435
tools/tryselect/cli.py
tools/tryselect/mach_commands.py
tools/tryselect/preset.py
tools/tryselect/test/conftest.py
tools/tryselect/test/python.ini
tools/tryselect/test/setup.sh
tools/tryselect/test/test_preset.t
tools/tryselect/test/test_preset_migration.t
tools/tryselect/test/test_presets.py
tools/tryselect/try_presets.yml
--- a/tools/tryselect/cli.py
+++ b/tools/tryselect/cli.py
@@ -1,21 +1,19 @@
 # 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
 
 import os
 import subprocess
-import sys
 import tempfile
 from argparse import ArgumentParser
 
-from .preset import presets
 from .templates import all_templates
 
 
 COMMON_ARGUMENT_GROUPS = {
     'push': [
         [['-m', '--message'],
          {'const': 'editor',
           'default': '{msg}',
@@ -41,25 +39,27 @@ COMMON_ARGUMENT_GROUPS = {
          {'default': None,
           'help': 'Save selection for future use with --preset.',
           }],
         [['--preset'],
          {'default': None,
           'help': 'Load a saved selection.',
           }],
         [['--list-presets'],
-         {'action': 'store_true',
-          'dest': 'list_presets',
-          'default': False,
+         {'action': 'store_const',
+          'dest': 'preset_action',
+          'const': 'list',
+          'default': None,
           'help': 'List available preset selections.',
           }],
         [['--edit-presets'],
-         {'action': 'store_true',
-          'dest': 'edit_presets',
-          'default': False,
+         {'action': 'store_const',
+          'dest': 'preset_action',
+          'const': 'edit',
+          'default': None,
           'help': 'Edit the preset file.',
           }],
     ],
     'task': [
         [['--full'],
          {'action': 'store_true',
           'default': False,
           'help': "Use the full set of tasks as input to fzf (instead of "
@@ -112,25 +112,16 @@ class BaseTryParser(ArgumentParser):
 
                 with tempfile.NamedTemporaryFile(mode='r') as fh:
                     subprocess.call([os.environ['EDITOR'], fh.name])
                     args.message = fh.read().strip()
 
             if '{msg}' not in args.message:
                 args.message = '{}\n\n{}'.format(args.message, '{msg}')
 
-        if 'preset' in self.common_groups:
-            if args.list_presets:
-                presets.list()
-                sys.exit()
-
-            if args.edit_presets:
-                presets.edit()
-                sys.exit()
-
     def parse_known_args(self, *args, **kwargs):
         args, remainder = ArgumentParser.parse_known_args(self, *args, **kwargs)
         self.validate(args)
 
         if self.templates:
             for cls in self.templates.itervalues():
                 context = cls.context(**vars(args))
                 if context is not None:
--- a/tools/tryselect/mach_commands.py
+++ b/tools/tryselect/mach_commands.py
@@ -10,17 +10,17 @@ import os
 import sys
 
 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
 
@@ -65,48 +65,70 @@ class TrySelect(MachCommandBase):
 
     def __init__(self, context):
         super(TrySelect, self).__init__(context)
         from tryselect import push
         push.MAX_HISTORY = self._mach_context.settings['try']['maxhistory']
         self.subcommand = self._mach_context.handler.subcommand
         self.parser = self._mach_context.handler.parser
 
-    def handle_presets(self, save, preset, **kwargs):
+    def handle_presets(self, preset_action, save, preset, **kwargs):
         """Handle preset related arguments.
 
         This logic lives here so that the underlying selectors don't need
         special preset handling. They can all save and load presets the same
         way.
         """
-        from tryselect.preset import presets, migrate_old_presets
+        from tryselect.preset import MergedHandler, migrate_old_presets
         from tryselect.util.dicttools import merge
 
+        # Create our handler using both local and in-tree presets. The first
+        # path in this list will be treated as the 'user' file for the purposes
+        # of saving and editing. All subsequent paths are 'read-only'. We check
+        # an environment variable first for testing purposes.
+        if os.environ.get('MACH_TRY_PRESET_PATHS'):
+            preset_paths = os.environ['MACH_TRY_PRESET_PATHS'].split(os.pathsep)
+        else:
+            preset_paths = [
+                os.path.join(get_state_dir(), 'try_presets.yml'),
+                os.path.join(self.topsrcdir, 'tools', 'tryselect', 'try_presets.yml'),
+            ]
+
+        presets = MergedHandler(*preset_paths)
+        user_presets = presets.handlers[0]
+
         # TODO: Remove after Jan 1, 2020.
-        migrate_old_presets()
+        migrate_old_presets(user_presets)
+
+        if preset_action == 'list':
+            presets.list()
+            sys.exit()
+
+        if preset_action == 'edit':
+            user_presets.edit()
+            sys.exit()
 
         default = self.parser.get_default
         if save:
             selector = self.subcommand or self._mach_context.settings['try']['default']
 
             # Only save non-default values for simplicity.
             kwargs = {k: v for k, v in kwargs.items() if v != default(k)}
-            presets.save(save, selector=selector, **kwargs)
+            user_presets.save(save, selector=selector, **kwargs)
             print('preset saved, run with: --preset={}'.format(save))
             sys.exit()
 
         if preset:
             if preset not in presets:
-                # TODO: This should live in the parser's validation method, but
-                # for now we want this check to run *after* preset migration.
                 self.parser.error("preset '{}' does not exist".format(preset))
 
             name = preset
             preset = presets[name]
             selector = preset['selector']
+            preset.pop('description', None)  # description isn't used by any selectors
 
             if not self.subcommand:
                 self.subcommand = selector
             elif self.subcommand != selector:
                 print("error: preset '{}' exists for a different selector "
                       "(did you mean to run 'mach try {}' instead?)".format(
                         name, selector))
                 sys.exit(1)
@@ -124,17 +146,17 @@ class TrySelect(MachCommandBase):
             kwargs = merge(defaults, preset, nondefaults)
 
         return kwargs
 
     @Command('try',
              category='ci',
              description='Push selected tasks to the try server',
              parser=generic_parser)
-    def try_default(self, argv, **kwargs):
+    def try_default(self, argv=None, **kwargs):
         """Push selected tests to the try server.
 
         The |mach try| command is a frontend for scheduling tasks to
         run on try server using selectors. A selector is a subcommand
         that provides its own set of command line arguments and are
         listed below.
 
         If no subcommand is specified, the `syntax` selector is run by
--- a/tools/tryselect/preset.py
+++ b/tools/tryselect/preset.py
@@ -28,17 +28,22 @@ class PresetHandler(object):
         return self._presets
 
     def __contains__(self, name):
         return name in self.presets
 
     def __getitem__(self, name):
         return self.presets[name]
 
+    def __len__(self):
+        return len(self.presets)
+
     def __str__(self):
+        if not self.presets:
+            return ''
         return yaml.safe_dump(self.presets, default_flow_style=False)
 
     def list(self):
         if not self.presets:
             print("no presets found")
         else:
             print(self)
 
@@ -51,22 +56,60 @@ class PresetHandler(object):
 
     def save(self, name, **data):
         self.presets[name] = data
 
         with open(self.path, "w") as fh:
             fh.write(str(self))
 
 
-presets = PresetHandler(os.path.join(get_state_dir(), "try_presets.yml"))
+class MergedHandler(object):
+    def __init__(self, *paths):
+        """Helper class for dealing with multiple preset files."""
+        self.handlers = [PresetHandler(p) for p in paths]
+
+    def __contains__(self, name):
+        return any(name in handler for handler in self.handlers)
+
+    def __getitem__(self, name):
+        for handler in self.handlers:
+            if name in handler:
+                return handler[name]
+        raise KeyError(name)
+
+    def __len__(self):
+        return sum(len(h) for h in self.handlers)
+
+    def __str__(self):
+        all_presets = {
+            k: v
+            for handler in self.handlers
+            for k, v in handler.presets.items()
+        }
+        return yaml.safe_dump(all_presets, default_flow_style=False)
+
+    def list(self):
+        if len(self) == 0:
+            print("no presets found")
+            return
+
+        for handler in self.handlers:
+            val = str(handler)
+            if val:
+                val = '\n  '.join([''] + val.splitlines() + [''])  # indent all lines by 2 spaces
+                print("Presets from {}:".format(handler.path))
+                print(val)
 
 
-def migrate_old_presets():
+def migrate_old_presets(presets):
     """Move presets from the old `autotry.ini` format to the new
     `try_presets.yml` one.
+
+    Args:
+        presets (PresetHandler): Handler to migrate old presets into.
     """
     from .selectors.syntax import AutoTry, SyntaxParser
     old_preset_path = os.path.join(get_state_dir(), 'autotry.ini')
     if os.path.isfile(presets.path) or not os.path.isfile(old_preset_path):
         return
 
     print("migrating saved presets from '{}' to '{}'".format(old_preset_path, presets.path))
     config = ConfigParser.ConfigParser()
--- a/tools/tryselect/test/conftest.py
+++ b/tools/tryselect/test/conftest.py
@@ -1,15 +1,18 @@
 # 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
 
+import os
+
 import pytest
+import yaml
 from mock import MagicMock
 from moztest.resolve import TestResolver
 
 from tryselect import push
 
 
 @pytest.fixture
 def patch_resolver(monkeypatch):
@@ -35,8 +38,23 @@ def pytest_generate_tests(metafunc):
         def load_tests():
             for template, tests in metafunc.module.TEMPLATE_TESTS.items():
                 for args, expected in tests:
                     yield (template, args, expected)
 
         tests = list(load_tests())
         ids = ['{} {}'.format(t[0], ' '.join(t[1])).strip() for t in tests]
         metafunc.parametrize('template,args,expected', tests, ids=ids)
+
+    elif all(fixture in metafunc.fixturenames for fixture in ('shared_name', 'shared_preset')):
+        preset_path = os.path.join(push.build.topsrcdir, 'tools', 'tryselect', 'try_presets.yml')
+        with open(preset_path, 'r') as fh:
+            presets = yaml.safe_load(fh).items()
+
+        ids = [p[0] for p in presets]
+
+        # Mark fuzzy presets on Windows xfail due to fzf not being installed.
+        if os.name == 'nt':
+            for i, preset in enumerate(presets):
+                if preset[1]['selector'] == 'fuzzy':
+                    presets[i] = pytest.param(*preset, marks=pytest.mark.xfail)
+
+        metafunc.parametrize('shared_name,shared_preset', presets, ids=ids)
--- a/tools/tryselect/test/python.ini
+++ b/tools/tryselect/test/python.ini
@@ -1,7 +1,8 @@
 [DEFAULT]
 subsuite=try
 skip-if = python == 3
 
 [test_again.py]
+[test_presets.py]
 [test_tasks.py]
 [test_templates.py]
--- a/tools/tryselect/test/setup.sh
+++ b/tools/tryselect/test/setup.sh
@@ -1,10 +1,11 @@
 export topsrcdir=$TESTDIR/../../../
 export MOZBUILD_STATE_PATH=$TMP/mozbuild
+export MACH_TRY_PRESET_PATHS=$MOZBUILD_STATE_PATH/try_presets.yml
 
 export MACHRC=$TMP/machrc
 cat > $MACHRC << EOF
 [try]
 default=syntax
 EOF
 
 calculate_hash='import hashlib, os, sys
--- a/tools/tryselect/test/test_preset.t
+++ b/tools/tryselect/test/test_preset.t
@@ -14,28 +14,30 @@ Test preset with no subcommand
 
   $ ./mach try syntax $testargs --preset foo
   Commit message:
   try: -b do -p linux -u mochitests -t none --tag foo
   
   Pushed via `mach try syntax`
 
   $ ./mach try $testargs --list-presets
-  foo:
-    no_artifact: true
-    platforms:
-    - linux
-    selector: syntax
-    tags:
-    - foo
-    talos:
-    - none
-    tests:
-    - mochitests
+  Presets from */mozbuild/try_presets.yml: (glob)
   
+    foo:
+      no_artifact: true
+      platforms:
+      - linux
+      selector: syntax
+      tags:
+      - foo
+      talos:
+      - none
+      tests:
+      - mochitests
+    
   $ unset EDITOR
   $ ./mach try $testargs --edit-presets
   error: must set the $EDITOR environment variable to use --edit-presets
   $ export EDITOR=cat
   $ ./mach try $testargs --edit-presets
   foo:
     no_artifact: true
     platforms:
@@ -61,40 +63,42 @@ Test preset with syntax subcommand
 
   $ ./mach try $testargs --preset bar
   Commit message:
   try: -b do -p win32 -u none -t all --tag bar
   
   Pushed via `mach try syntax`
 
   $ ./mach try syntax $testargs --list-presets
-  bar:
-    no_artifact: true
-    platforms:
-    - win32
-    push: false
-    selector: syntax
-    tags:
-    - bar
-    talos:
-    - all
-    tests:
-    - none
-  foo:
-    no_artifact: true
-    platforms:
-    - linux
-    selector: syntax
-    tags:
-    - foo
-    talos:
-    - none
-    tests:
-    - mochitests
+  Presets from */mozbuild/try_presets.yml: (glob)
   
+    bar:
+      no_artifact: true
+      platforms:
+      - win32
+      push: false
+      selector: syntax
+      tags:
+      - bar
+      talos:
+      - all
+      tests:
+      - none
+    foo:
+      no_artifact: true
+      platforms:
+      - linux
+      selector: syntax
+      tags:
+      - foo
+      talos:
+      - none
+      tests:
+      - mochitests
+    
   $ ./mach try syntax $testargs --edit-presets
   bar:
     no_artifact: true
     platforms:
     - win32
     push: false
     selector: syntax
     tags:
@@ -172,46 +176,48 @@ Test preset with fuzzy subcommand
           "env": {
               "TRY_SELECTOR": "fuzzy"
           }
       },
       "version": 1
   }
   
   $ ./mach try fuzzy $testargs --list-presets
-  bar:
-    no_artifact: true
-    platforms:
-    - win32
-    push: false
-    selector: syntax
-    tags:
-    - bar
-    talos:
-    - all
-    tests:
-    - none
-  baz:
-    no_artifact: true
-    push: false
-    query:
-    - '''baz'
-    selector: fuzzy
-  foo:
-    no_artifact: true
-    platforms:
-    - linux
-    selector: syntax
-    tags:
-    - foo
-    talos:
-    - none
-    tests:
-    - mochitests
+  Presets from */mozbuild/try_presets.yml: (glob)
   
+    bar:
+      no_artifact: true
+      platforms:
+      - win32
+      push: false
+      selector: syntax
+      tags:
+      - bar
+      talos:
+      - all
+      tests:
+      - none
+    baz:
+      no_artifact: true
+      push: false
+      query:
+      - '''baz'
+      selector: fuzzy
+    foo:
+      no_artifact: true
+      platforms:
+      - linux
+      selector: syntax
+      tags:
+      - foo
+      talos:
+      - none
+      tests:
+      - mochitests
+    
   $ ./mach try fuzzy $testargs --edit-presets
   bar:
     no_artifact: true
     platforms:
     - win32
     push: false
     selector: syntax
     tags:
--- a/tools/tryselect/test/test_preset_migration.t
+++ b/tools/tryselect/test/test_preset_migration.t
@@ -6,51 +6,49 @@ Test migration of syntax preset
   $ rm $MOZBUILD_STATE_PATH/try_presets.yml
   $ cat > $MOZBUILD_STATE_PATH/autotry.ini << EOF
   > [try]
   > foo = -b o -p all -u mochitest -t none --tag bar
   > EOF
   $ ls $MOZBUILD_STATE_PATH/autotry.ini
   */mozbuild/autotry.ini (glob)
   $ ./mach try syntax $testargs --list-presets
-  no presets found
+  migrating saved presets from '*/mozbuild/autotry.ini' to '*/mozbuild/try_presets.yml' (glob)
+  Presets from */mozbuild/try_presets.yml: (glob)
+  
+    foo:
+      builds: o
+      platforms:
+      - all
+      selector: syntax
+      tags:
+      - bar
+      talos:
+      - none
+      tests:
+      - mochitest
+    
   $ ./mach try syntax $testargs --preset foo
-  migrating saved presets from '*/mozbuild/autotry.ini' to '*/mozbuild/try_presets.yml' (glob)
   Commit message:
   try: -b o -p all -u mochitest -t none --tag bar
   
   Pushed via `mach try syntax`
   $ ls $MOZBUILD_STATE_PATH/autotry.ini
   */mozbuild/autotry.ini': No such file or directory (glob)
   [2]
-  $ ./mach try syntax $testargs --list-presets
-  foo:
-    builds: o
-    platforms:
-    - all
-    selector: syntax
-    tags:
-    - bar
-    talos:
-    - none
-    tests:
-    - mochitest
-  
- 
  Test migration of fuzzy preset
 
   $ rm $MOZBUILD_STATE_PATH/try_presets.yml
   $ cat > $MOZBUILD_STATE_PATH/autotry.ini << EOF
   > [fuzzy]
   > foo = 'foo | 'bar
   > EOF
   $ ls $MOZBUILD_STATE_PATH/autotry.ini
   */mozbuild/autotry.ini (glob)
-  $ ./mach try fuzzy $testargs --list-presets
-  no presets found
+
   $ ./mach try fuzzy $testargs --preset foo
   migrating saved presets from '*/mozbuild/autotry.ini' to '*/mozbuild/try_presets.yml' (glob)
   Commit message:
   Fuzzy query='foo | 'bar
   
   Pushed via `mach try fuzzy`
   Calculated try_task_config.json:
   {
@@ -65,21 +63,23 @@ Test migration of syntax preset
       },
       "version": 1
   }
   
   $ ls $MOZBUILD_STATE_PATH/autotry.ini
   */mozbuild/autotry.ini': No such file or directory (glob)
   [2]
   $ ./mach try fuzzy $testargs --list-presets
-  foo:
-    query:
-    - '''foo | ''bar'
-    selector: fuzzy
+  Presets from */mozbuild/try_presets.yml: (glob)
   
+    foo:
+      query:
+      - '''foo | ''bar'
+      selector: fuzzy
+    
  
  Test unknown section prints message
 
   $ rm $MOZBUILD_STATE_PATH/try_presets.yml
   $ cat > $MOZBUILD_STATE_PATH/autotry.ini << EOF
   > [unknown]
   > foo = bar
   > baz = foo
new file mode 100644
--- /dev/null
+++ b/tools/tryselect/test/test_presets.py
@@ -0,0 +1,43 @@
+# 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
+
+import mozunit
+import pytest
+
+
+@pytest.fixture(scope='session')
+def run_mach():
+    import mach_bootstrap
+    from mach.config import ConfigSettings
+    from tryselect.tasks import build
+
+    mach = mach_bootstrap.bootstrap(build.topsrcdir)
+
+    def inner(args):
+        mach.settings = ConfigSettings()
+        return mach.run(args)
+
+    return inner
+
+
+def test_shared_presets(run_mach, shared_name, shared_preset):
+    """This test makes sure that we don't break any of the in-tree presets when
+    renaming/removing variables in any of the selectors.
+    """
+    assert 'description' in shared_preset
+    assert 'selector' in shared_preset
+
+    selector = shared_preset['selector']
+    if selector == 'fuzzy':
+        assert 'query' in shared_preset
+        assert isinstance(shared_preset['query'], list)
+
+    # Run the preset and assert there were no exceptions.
+    assert run_mach(['try', '--no-push', '--preset', shared_name]) == 0
+
+
+if __name__ == '__main__':
+    mozunit.main()
new file mode 100644
--- /dev/null
+++ b/tools/tryselect/try_presets.yml
@@ -0,0 +1,18 @@
+---
+# Presets defined here will be available to all users. Run them with:
+#   $ mach try --preset <name>
+#
+# If editing this file, make sure to run:
+#   $ mach python-test tools/tryselect/test/test_presets.py
+#
+# Descriptions are required. Please keep this in alphabetical order.
+
+sample-suites:
+    selector: fuzzy
+    description: >-
+        Runs one chunk of every test suite plus all suites that aren't chunked.
+        It is useful for testing infrastructure changes that can affect the
+        harnesses themselves but are unlikely to break specific tests.
+    query:
+        - ^test- -1$
+        - ^test- !1$ !2$ !3$ !4$ !5$ !6$ !7$ !8$ !9$ !0$