Bug 1333167: Add extra try options to taskcluster. r=dustin a=jmaher
authorWander Lairson Costa <wcosta@mozilla.com>
Thu, 02 Feb 2017 09:34:43 -0200
changeset 469876 ffd3ef23726e0027143ac948047ac63d41ae4700
parent 469875 bb1ad8ae88c31076be3cb06d673c258f1de52e32
child 469877 c34193caeedabd2834bd337a8b5551b8cafa1011
push id43881
push userbmo:gps@mozilla.com
push dateThu, 02 Feb 2017 23:49:03 +0000
reviewersdustin, jmaher
bugs1333167
milestone54.0a1
Bug 1333167: Add extra try options to taskcluster. r=dustin a=jmaher We add the following command line options to Taskcluster try syntax: --spsProfile - enable profile mode. --rebuild-talos <N> - retrigger talos tests N times. --setenv <VAR>=<val> - add extra environments variables. --tag <TAG> - run tests only the tag TAG. --no-retry - doesn't retry failed jobs. We have a chicken-egg problem, as we first generate the full task graph and then parse the try message. But the graph generation step needs to know the try message to process the aforementioned options. The solution is to parse the message before graph generation and then pass the command line options to the transforms. Then, each transform can look at the option that interests it and process it accordingly. The message parse function is configured in kind.yml, which gives some flexibility for future implementations of alternative syntaxes. MozReview-Commit-ID: GPFdi0FD6Vn
taskcluster/ci/build/kind.yml
taskcluster/ci/test/kind.yml
taskcluster/docs/loading.rst
taskcluster/taskgraph/generator.py
taskcluster/taskgraph/target_tasks.py
taskcluster/taskgraph/test/test_target_tasks.py
taskcluster/taskgraph/test/test_try_option_syntax.py
taskcluster/taskgraph/transforms/build.py
taskcluster/taskgraph/transforms/tests.py
taskcluster/taskgraph/try_option_syntax.py
--- a/taskcluster/ci/build/kind.yml
+++ b/taskcluster/ci/build/kind.yml
@@ -10,8 +10,10 @@ transforms:
    - taskgraph.transforms.job:transforms
    - taskgraph.transforms.task:transforms
 
 jobs-from:
     - android.yml
     - linux.yml
     - macosx.yml
     - windows.yml
+
+parse-commit: taskgraph.try_option_syntax:parse_message
--- a/taskcluster/ci/test/kind.yml
+++ b/taskcluster/ci/test/kind.yml
@@ -1,8 +1,10 @@
 implementation: taskgraph.task.test:TestTask
 
 kind-dependencies:
     - build
 
 transforms:
    - taskgraph.transforms.tests:transforms
    - taskgraph.transforms.task:transforms
+
+parse-commit: taskgraph.try_option_syntax:parse_message
--- a/taskcluster/docs/loading.rst
+++ b/taskcluster/docs/loading.rst
@@ -24,8 +24,17 @@ This is handled by the ``TransformTask``
 For kinds producing tasks that depend on other tasks -- for example, signing
 tasks depend on build tasks -- ``TransformTask`` has a ``get_inputs`` method
 that can be overridden in subclasses and written to return a set of items based
 on tasks that already exist.  You can see a nice example of this behavior in
 ``taskcluster/taskgraph/task/post_build.py``.
 
 For more information on how all of this works, consult the docstrings and
 comments in the source code itself.
+
+Try option syntax
+-----------------
+
+The ``parse-commit`` optional field specified in ``kind.yml`` links to a
+function to parse the command line options in the ``--message`` mach parameter.
+Currently, the only valid value is ``taskgraph.try_option_syntax:parse_message``.
+The parsed arguments are stored in ``config.config['args']``, it corresponds
+to the same object returned by ``parse_args`` from ``argparse`` Python module.
--- a/taskcluster/taskgraph/generator.py
+++ b/taskcluster/taskgraph/generator.py
@@ -1,16 +1,17 @@
 # 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 logging
 import os
 import yaml
+import copy
 
 from . import filter_tasks
 from .graph import Graph
 from .taskgraph import TaskGraph
 from .optimize import optimize_task_graph
 from .util.python_path import find_object
 from .util.verify import (
     verify_docs,
@@ -33,17 +34,25 @@ class Kind(object):
         try:
             impl = self.config['implementation']
         except KeyError:
             raise KeyError("{!r} does not define implementation".format(self.path))
         return find_object(impl)
 
     def load_tasks(self, parameters, loaded_tasks):
         impl_class = self._get_impl_class()
-        return impl_class.load_tasks(self.name, self.path, self.config,
+        config = copy.deepcopy(self.config)
+
+        if 'parse-commit' in self.config:
+            parse_commit = find_object(config['parse-commit'])
+            config['args'] = parse_commit(parameters['message'])
+        else:
+            config['args'] = None
+
+        return impl_class.load_tasks(self.name, self.path, config,
                                      parameters, loaded_tasks)
 
 
 class TaskGraphGenerator(object):
     """
     The central controller for taskgraph.  This handles all phases of graph
     generation.  The task is generated from all of the kinds defined in
     subdirectories of the generator's root directory.
--- a/taskcluster/taskgraph/target_tasks.py
+++ b/taskcluster/taskgraph/target_tasks.py
@@ -37,22 +37,42 @@ def get_method(method):
 @_target_task('try_option_syntax')
 def target_tasks_try_option_syntax(full_task_graph, parameters):
     """Generate a list of target tasks based on try syntax in
     parameters['message'] and, for context, the full task graph."""
     options = try_option_syntax.TryOptionSyntax(parameters['message'], full_task_graph)
     target_tasks_labels = [t.label for t in full_task_graph.tasks.itervalues()
                            if options.task_matches(t.attributes)]
 
-    # If the developer wants test jobs to be rebuilt N times we add that value here
-    if int(options.trigger_tests) > 1:
-        for l in target_tasks_labels:
-            task = full_task_graph[l]
-            if 'unittest_suite' in task.attributes:
-                task.attributes['task_duplicates'] = options.trigger_tests
+    attributes = {
+        k: getattr(options, k) for k in [
+            'env',
+            'no_retry',
+            'tag',
+        ]
+    }
+
+    for l in target_tasks_labels:
+        task = full_task_graph[l]
+        if 'unittest_suite' in task.attributes:
+            task.attributes['task_duplicates'] = options.trigger_tests
+
+    for l in target_tasks_labels:
+        task = full_task_graph[l]
+        # If the developer wants test jobs to be rebuilt N times we add that value here
+        if options.trigger_tests > 1 and 'unittest_suite' in task.attributes:
+            task.attributes['task_duplicates'] = options.trigger_tests
+            task.attributes['profile'] = False
+
+        # If the developer wants test talos jobs to be rebuilt N times we add that value here
+        if options.talos_trigger_tests > 1 and 'talos_suite' in task.attributes:
+            task.attributes['task_duplicates'] = options.talos_trigger_tests
+            task.attributes['profile'] = options.profile
+
+        task.attributes.update(attributes)
 
     # Add notifications here as well
     if options.notifications:
         for task in full_task_graph:
             owner = parameters.get('owner')
             routes = task.task.setdefault('routes', [])
             if options.notifications == 'all':
                 routes.append("notify.email.{}.on-any".format(owner))
--- a/taskcluster/taskgraph/test/test_target_tasks.py
+++ b/taskcluster/taskgraph/test/test_target_tasks.py
@@ -13,17 +13,22 @@ from ..taskgraph import TaskGraph
 from .util import TestTask
 from mozunit import main
 
 
 class FakeTryOptionSyntax(object):
 
     def __init__(self, message, task_graph):
         self.trigger_tests = 0
+        self.talos_trigger_tests = 0
         self.notifications = None
+        self.env = []
+        self.profile = False
+        self.tag = None
+        self.no_retry = False
 
     def task_matches(self, attributes):
         return 'at-at' in attributes
 
 
 class TestTargetTasks(unittest.TestCase):
 
     def default_matches(self, run_on_projects, project):
--- a/taskcluster/taskgraph/test/test_try_option_syntax.py
+++ b/taskcluster/taskgraph/test/test_try_option_syntax.py
@@ -51,25 +51,37 @@ class TestTryOptionSyntax(unittest.TestC
     def test_empty_message(self):
         "Given an empty message, it should return an empty value"
         tos = TryOptionSyntax('', empty_graph)
         self.assertEqual(tos.build_types, [])
         self.assertEqual(tos.jobs, [])
         self.assertEqual(tos.unittests, [])
         self.assertEqual(tos.talos, [])
         self.assertEqual(tos.platforms, [])
+        self.assertEqual(tos.trigger_tests, 0)
+        self.assertEqual(tos.talos_trigger_tests, 0)
+        self.assertEqual(tos.env, [])
+        self.assertFalse(tos.profile)
+        self.assertIsNone(tos.tag)
+        self.assertFalse(tos.no_retry)
 
     def test_message_without_try(self):
         "Given a non-try message, it should return an empty value"
         tos = TryOptionSyntax('Bug 1234: frobnicte the foo', empty_graph)
         self.assertEqual(tos.build_types, [])
         self.assertEqual(tos.jobs, [])
         self.assertEqual(tos.unittests, [])
         self.assertEqual(tos.talos, [])
         self.assertEqual(tos.platforms, [])
+        self.assertEqual(tos.trigger_tests, 0)
+        self.assertEqual(tos.talos_trigger_tests, 0)
+        self.assertEqual(tos.env, [])
+        self.assertFalse(tos.profile)
+        self.assertIsNone(tos.tag)
+        self.assertFalse(tos.no_retry)
 
     def test_unknown_args(self):
         "unknown arguments are ignored"
         tos = TryOptionSyntax('try: --doubledash -z extra', empty_graph)
         # equilvant to "try:"..
         self.assertEqual(tos.build_types, [])
         self.assertEqual(tos.jobs, None)
 
@@ -241,16 +253,21 @@ class TestTryOptionSyntax(unittest.TestC
 
     # -t shares an implementation with -u, so it's not tested heavily
 
     def test_trigger_tests(self):
         "--rebuild 10 sets trigger_tests"
         tos = TryOptionSyntax('try: --rebuild 10', empty_graph)
         self.assertEqual(tos.trigger_tests, 10)
 
+    def test_talos_trigger_tests(self):
+        "--rebuild-talos 10 sets talos_trigger_tests"
+        tos = TryOptionSyntax('try: --rebuild-talos 10', empty_graph)
+        self.assertEqual(tos.talos_trigger_tests, 10)
+
     def test_interactive(self):
         "--interactive sets interactive"
         tos = TryOptionSyntax('try: --interactive', empty_graph)
         self.assertEqual(tos.interactive, True)
 
     def test_all_email(self):
         "--all-emails sets notifications"
         tos = TryOptionSyntax('try: --all-emails', empty_graph)
@@ -261,10 +278,30 @@ class TestTryOptionSyntax(unittest.TestC
         tos = TryOptionSyntax('try: --failure-emails', empty_graph)
         self.assertEqual(tos.notifications, 'failure')
 
     def test_no_email(self):
         "no email settings don't set notifications"
         tos = TryOptionSyntax('try:', empty_graph)
         self.assertEqual(tos.notifications, None)
 
+    def test_setenv(self):
+        "--setenv VAR=value adds a environment variables setting to env"
+        tos = TryOptionSyntax('try: --setenv VAR1=value1 --setenv VAR2=value2', empty_graph)
+        self.assertEqual(tos.env, ['VAR1=value1', 'VAR2=value2'])
+
+    def test_profile(self):
+        "--spsProfile sets profile to true"
+        tos = TryOptionSyntax('try: --spsProfile', empty_graph)
+        self.assertTrue(tos.profile)
+
+    def test_tag(self):
+        "--tag TAG sets tag to TAG value"
+        tos = TryOptionSyntax('try: --tag tagName', empty_graph)
+        self.assertEqual(tos.tag, 'tagName')
+
+    def test_no_retry(self):
+        "--no-retry sets no_retry to true"
+        tos = TryOptionSyntax('try: --no-retry', empty_graph)
+        self.assertTrue(tos.no_retry)
+
 if __name__ == '__main__':
     main()
--- a/taskcluster/taskgraph/transforms/build.py
+++ b/taskcluster/taskgraph/transforms/build.py
@@ -24,9 +24,21 @@ def set_defaults(config, jobs):
             job['worker'].setdefault('docker-image', {'in-tree': 'desktop-build'})
             job['worker']['chain-of-trust'] = True
             job.setdefault('extra', {})
             job['extra'].setdefault('chainOfTrust', {})
             job['extra']['chainOfTrust'].setdefault('inputs', {})
             job['extra']['chainOfTrust']['inputs']['docker-image'] = {
                 "task-reference": "<docker-image>"
             }
+        job['worker'].setdefault('env', {})
         yield job
+
+
+@transforms.add
+def set_env(config, jobs):
+    """Set extra environment variables from try command line."""
+    for job in jobs:
+        env = config.config['args'].env
+        if env:
+            job_env = job['worker']['env']
+            job_env.update(dict(x.split('=') for x in env))
+        yield job
--- a/taskcluster/taskgraph/transforms/tests.py
+++ b/taskcluster/taskgraph/transforms/tests.py
@@ -563,16 +563,35 @@ def set_retry_exit_status(config, tests)
     """Set the retry exit status to TBPL_RETRY, the value returned by mozharness
        scripts to indicate a transient failure that should be retried."""
     for test in tests:
         test['retry-exit-status'] = 4
         yield test
 
 
 @transforms.add
+def set_profile(config, tests):
+    """Set profiling mode for tests."""
+    for test in tests:
+        if config.config['args'].profile and test['suite'] == 'talos':
+            test['mozharness']['extra-options'].append('--spsProfile')
+        yield test
+
+
+@transforms.add
+def set_tag(config, tests):
+    """Set test for a specific tag."""
+    for test in tests:
+        tag = config.config['args'].tag
+        if tag:
+            test['mozharness']['extra-options'].extend(['--tag', tag])
+        yield test
+
+
+@transforms.add
 def remove_linux_pgo_try_talos(config, tests):
     """linux64-pgo talos tests don't run on try."""
     def predicate(test):
         return not(
             test['test-platform'] == 'linux64-pgo/opt'
             and test['suite'] == 'talos'
             and config.params['project'] == 'try'
         )
--- a/taskcluster/taskgraph/try_option_syntax.py
+++ b/taskcluster/taskgraph/try_option_syntax.py
@@ -167,16 +167,83 @@ RIDEALONG_BUILDS = {
         'sm-mozjs-sys',
         'sm-msan',
     ],
 }
 
 TEST_CHUNK_SUFFIX = re.compile('(.*)-([0-9]+)$')
 
 
+def escape_whitespace_in_brackets(input_str):
+    '''
+    In tests you may restrict them by platform [] inside of the brackets
+    whitespace may occur this is typically invalid shell syntax so we escape it
+    with backslash sequences    .
+    '''
+    result = ""
+    in_brackets = False
+    for char in input_str:
+        if char == '[':
+            in_brackets = True
+            result += char
+            continue
+
+        if char == ']':
+            in_brackets = False
+            result += char
+            continue
+
+        if char == ' ' and in_brackets:
+            result += '\ '
+            continue
+
+        result += char
+
+    return result
+
+
+def parse_message(message):
+    # shlex used to ensure we split correctly when giving values to argparse.
+    parts = shlex.split(escape_whitespace_in_brackets(message))
+    try_idx = None
+    for idx, part in enumerate(parts):
+        if part == TRY_DELIMITER:
+            try_idx = idx
+            break
+
+    if try_idx is None:
+        return None
+
+    # Argument parser based on try flag flags
+    parser = argparse.ArgumentParser()
+    parser.add_argument('-b', '--build', dest='build_types')
+    parser.add_argument('-p', '--platform', nargs='?',
+                        dest='platforms', const='all', default='all')
+    parser.add_argument('-u', '--unittests', nargs='?',
+                        dest='unittests', const='all', default='all')
+    parser.add_argument('-t', '--talos', nargs='?', dest='talos', const='all', default='none')
+    parser.add_argument('-i', '--interactive',
+                        dest='interactive', action='store_true', default=False)
+    parser.add_argument('-e', '--all-emails',
+                        dest='notifications', action='store_const', const='all')
+    parser.add_argument('-f', '--failure-emails',
+                        dest='notifications', action='store_const', const='failure')
+    parser.add_argument('-j', '--job', dest='jobs', action='append')
+    parser.add_argument('--rebuild-talos', dest='talos_trigger_tests', action='store',
+                        type=int, default=1)
+    parser.add_argument('--setenv', dest='env', action='append')
+    parser.add_argument('--spsProfile', dest='profile', action='store_true')
+    parser.add_argument('--tag', dest='tag', action='store', default=None)
+    parser.add_argument('--no-retry', dest='no_retry', action='store_true')
+    # In order to run test jobs multiple times
+    parser.add_argument('--rebuild', dest='trigger_tests', type=int, default=1)
+    args, _ = parser.parse_known_args(parts[try_idx:])
+    return args
+
+
 class TryOptionSyntax(object):
 
     def __init__(self, message, full_task_graph):
         """
         Parse a "try syntax" formatted commit message.  This is the old "-b do -p
         win32 -u all" format.  Aliases are applied to map short names to full
         names.
 
@@ -184,18 +251,21 @@ class TryOptionSyntax(object):
 
         - build_types: a list containing zero or more of 'opt' and 'debug'
         - platforms: a list of selected platform names, or None for all
         - unittests: a list of tests, of the form given below, or None for all
         - jobs: a list of requested job names, or None for all
         - trigger_tests: the number of times tests should be triggered (--rebuild)
         - interactive: true if --interactive
         - notifications: either None if no notifications or one of 'all' or 'failure'
-
-        Note that -t is currently completely ignored.
+        - talos_trigger_tests: the number of time talos tests should be triggered (--rebuild-talos)
+        - env: additional environment variables (ENV=value)
+        - profile: run talos in profile mode
+        - tag: restrict tests to the specified tag
+        - no_retry: do not retry failed jobs
 
         The unittests and talos lists contain dictionaries of the form:
 
         {
             'test': '<suite name>',
             'platforms': [..platform names..], # to limit to only certain platforms
             'only_chunks': set([..chunk numbers..]), # to limit only to certain chunks
         }
@@ -203,56 +273,40 @@ class TryOptionSyntax(object):
         self.jobs = []
         self.build_types = []
         self.platforms = []
         self.unittests = []
         self.talos = []
         self.trigger_tests = 0
         self.interactive = False
         self.notifications = None
-
-        # shlex used to ensure we split correctly when giving values to argparse.
-        parts = shlex.split(self.escape_whitespace_in_brackets(message))
-        try_idx = None
-        for idx, part in enumerate(parts):
-            if part == TRY_DELIMITER:
-                try_idx = idx
-                break
-
-        if try_idx is None:
-            return
+        self.talos_trigger_tests = 0
+        self.env = []
+        self.profile = False
+        self.tag = None
+        self.no_retry = False
 
-        # Argument parser based on try flag flags
-        parser = argparse.ArgumentParser()
-        parser.add_argument('-b', '--build', dest='build_types')
-        parser.add_argument('-p', '--platform', nargs='?',
-                            dest='platforms', const='all', default='all')
-        parser.add_argument('-u', '--unittests', nargs='?',
-                            dest='unittests', const='all', default='all')
-        parser.add_argument('-t', '--talos', nargs='?', dest='talos', default='none')
-        parser.add_argument('-i', '--interactive',
-                            dest='interactive', action='store_true', default=False)
-        parser.add_argument('-e', '--all-emails',
-                            dest='notifications', action='store_const', const='all')
-        parser.add_argument('-f', '--failure-emails',
-                            dest='notifications', action='store_const', const='failure')
-        parser.add_argument('-j', '--job', dest='jobs', action='append')
-        # In order to run test jobs multiple times
-        parser.add_argument('--rebuild', dest='trigger_tests', type=int, default=1)
-        args, _ = parser.parse_known_args(parts[try_idx:])
+        args = parse_message(message)
+        if args is None:
+            return
 
         self.jobs = self.parse_jobs(args.jobs)
         self.build_types = self.parse_build_types(args.build_types)
         self.platforms = self.parse_platforms(args.platforms)
         self.unittests = self.parse_test_option(
             "unittest_try_name", args.unittests, full_task_graph)
         self.talos = self.parse_test_option("talos_try_name", args.talos, full_task_graph)
         self.trigger_tests = args.trigger_tests
         self.interactive = args.interactive
         self.notifications = args.notifications
+        self.talos_trigger_tests = args.talos_trigger_tests
+        self.env = args.env
+        self.profile = args.profile
+        self.tag = args.tag
+        self.no_retry = args.no_retry
 
     def parse_jobs(self, jobs_arg):
         if not jobs_arg or jobs_arg == ['all']:
             return None
         expanded = []
         for job in jobs_arg:
             expanded.extend(j.strip() for j in job.split(','))
         return expanded
@@ -462,43 +516,16 @@ class TryOptionSyntax(object):
     def find_all_attribute_suffixes(self, graph, prefix):
         rv = set()
         for t in graph.tasks.itervalues():
             for a in t.attributes:
                 if a.startswith(prefix):
                     rv.add(a[len(prefix):])
         return sorted(rv)
 
-    def escape_whitespace_in_brackets(self, input_str):
-        '''
-        In tests you may restrict them by platform [] inside of the brackets
-        whitespace may occur this is typically invalid shell syntax so we escape it
-        with backslash sequences    .
-        '''
-        result = ""
-        in_brackets = False
-        for char in input_str:
-            if char == '[':
-                in_brackets = True
-                result += char
-                continue
-
-            if char == ']':
-                in_brackets = False
-                result += char
-                continue
-
-            if char == ' ' and in_brackets:
-                result += '\ '
-                continue
-
-            result += char
-
-        return result
-
     def task_matches(self, attributes):
         attr = attributes.get
 
         def check_run_on_projects():
             return set(['try', 'all']) & set(attr('run_on_projects', []))
 
         def match_test(try_spec, attr_name):
             if attr('build_type') not in self.build_types:
@@ -555,9 +582,14 @@ class TryOptionSyntax(object):
             "build_types: " + ", ".join(self.build_types),
             "platforms: " + none_for_all(self.platforms),
             "unittests: " + none_for_all(self.unittests),
             "talos: " + none_for_all(self.talos),
             "jobs: " + none_for_all(self.jobs),
             "trigger_tests: " + str(self.trigger_tests),
             "interactive: " + str(self.interactive),
             "notifications: " + str(self.notifications),
+            "talos_trigger_tests: " + str(self.talos_trigger_tests),
+            "env: " + str(self.env),
+            "profile: " + str(self.profile),
+            "tag: " + str(self.tag),
+            "no_retry: " + str(self.no_retry),
         ])