No bug: [taskgraph] Make `verify_test_packaging` handle `--target-kind` gracefully; r=Callek
authorTom Prince <mozilla@hocat.ca>
Tue, 14 Jul 2020 07:17:26 +0000
changeset 540327 8c0e953f215b570c18e2deee1570c0e3224cf74d
parent 540326 ab433cee6227224ad8a66b55ca750d5e1778ba93
child 540328 4d835cc17a7ff0ad08c5c3da1df70b470d709acf
push id37599
push userapavel@mozilla.com
push dateTue, 14 Jul 2020 15:35:20 +0000
treeherdermozilla-central@bca48c382991 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersCallek
milestone80.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
No bug: [taskgraph] Make `verify_test_packaging` handle `--target-kind` gracefully; r=Callek If we are generating only a part of the graph, to given kind, don't fail if a build is packaging tests and there is no corresponding test task, as the tests may not have been generated. Differential Revision: https://phabricator.services.mozilla.com/D82097
taskcluster/docs/parameters.rst
taskcluster/mach_commands.py
taskcluster/taskgraph/generator.py
taskcluster/taskgraph/parameters.py
taskcluster/taskgraph/util/verify.py
taskcluster/test/conftest.py
taskcluster/test/test_mach_try_auto.py
--- a/taskcluster/docs/parameters.rst
+++ b/taskcluster/docs/parameters.rst
@@ -230,8 +230,15 @@ them are specified, they must all be spe
 ``comm_head_ref``
 
 Code Review
 -----------
 
 ``phabricator_diff``
    The code review process needs to know the Phabricator Differential diff that
    started the analysis. This parameter must start with `PHID-DIFF-`
+
+Local configuration
+-------------------
+
+``target-kind``
+  Generate only the given kind and its kind-dependencies. This is used for local inspection of the graph
+  and is not supported at run-time.
--- a/taskcluster/mach_commands.py
+++ b/taskcluster/mach_commands.py
@@ -332,22 +332,25 @@ class MachCommands(MachCommandBase):
         import taskgraph.parameters
         import taskgraph.generator
         import taskgraph
         if options['fast']:
             taskgraph.fast = True
 
         try:
             self.setup_logging(quiet=options['quiet'], verbose=options['verbose'])
-            parameters = taskgraph.parameters.parameters_loader(options['parameters'])
+            parameters = taskgraph.parameters.parameters_loader(
+                options['parameters'],
+                overrides={'target-kind': options.get('target_kind')},
+                strict=False,
+            )
 
             tgg = taskgraph.generator.TaskGraphGenerator(
                 root_dir=options.get('root'),
                 parameters=parameters,
-                target_kind=options.get('target_kind'),
             )
 
             tg = getattr(tgg, graph_attr)
 
             show_method = getattr(self, 'show_taskgraph_' + (options['format'] or 'labels'))
             tg = self.get_filtered_taskgraph(tg, options["tasks_regex"])
 
             fh = options['output_file']
--- a/taskcluster/taskgraph/generator.py
+++ b/taskcluster/taskgraph/generator.py
@@ -109,29 +109,27 @@ class TaskGraphGenerator(object):
     # circuit generation of the entire graph by never completing the generator.
 
     def __init__(
         self,
         root_dir,
         parameters,
         decision_task_id="DECISION-TASK",
         write_artifacts=False,
-        target_kind=None,
     ):
         """
         @param root_dir: root directory, with subdirectories for each kind
         @param paramaters: parameters for this task-graph generation, or callable
             taking a `GraphConfig` and returning parameters
         @type parameters: Union[Parameters, Callable[[GraphConfig], Parameters]]
         """
         if root_dir is None:
             root_dir = 'taskcluster/ci'
         self.root_dir = ensure_text(root_dir)
         self._parameters = parameters
-        self._target_kind = target_kind
         self._decision_task_id = decision_task_id
         self._write_artifacts = write_artifacts
 
         # start the generator
         self._run = self._run()
         self._run_results = {}
 
     @property
@@ -257,21 +255,22 @@ class TaskGraphGenerator(object):
         self.verify_kinds(kinds)
 
         edges = set()
         for kind in six.itervalues(kinds):
             for dep in kind.config.get('kind-dependencies', []):
                 edges.add((kind.name, dep, 'kind-dependency'))
         kind_graph = Graph(set(kinds), edges)
 
-        if self._target_kind:
+        if parameters.get('target-kind'):
+            target_kind = parameters['target-kind']
             logger.info(
                 "Limiting kinds to {target_kind} and dependencies".format(
-                    target_kind=self._target_kind))
-            kind_graph = kind_graph.transitive_closure({self._target_kind, 'docker-image'})
+                    target_kind=target_kind))
+            kind_graph = kind_graph.transitive_closure({target_kind, 'docker-image'})
 
         logger.info("Generating full task set")
         all_tasks = {}
         for kind_name in kind_graph.visit_postorder():
             logger.debug("Loading tasks for kind {}".format(kind_name))
             kind = kinds[kind_name]
             try:
                 new_tasks = kind.load_tasks(
@@ -414,14 +413,17 @@ class TaskGraphGenerator(object):
 
 
 def load_tasks_for_kind(parameters, kind, root_dir=None):
     """
     Get all the tasks of a given kind.
 
     This function is designed to be called from outside of taskgraph.
     """
-    tgg = TaskGraphGenerator(root_dir=root_dir, parameters=parameters, target_kind=kind)
+    # make parameters read-write
+    parameters = dict(parameters)
+    parameters['target-kind'] = kind
+    tgg = TaskGraphGenerator(root_dir=root_dir, parameters=parameters)
     return {
         task.task['metadata']['name']: task
         for task in tgg.full_task_set
         if task.kind == kind
     }
--- a/taskcluster/taskgraph/parameters.py
+++ b/taskcluster/taskgraph/parameters.py
@@ -91,16 +91,18 @@ base_schema = Schema({
     Required('release_history'): {text_type: dict},
     Required('release_partners'): Any(None, [text_type]),
     Required('release_partner_config'): Any(None, dict),
     Required('release_partner_build_number'): int,
     Required('release_type'): text_type,
     Required('release_product'): Any(None, text_type),
     Required('required_signoffs'): [text_type],
     Required('signoff_urls'): dict,
+    # target-kind is not included, since it should never be
+    # used at run-time
     Required('target_tasks_method'): text_type,
     Required('tasks_for'): text_type,
     Required('test_manifest_loader'): text_type,
     Required('try_mode'): Any(None, text_type),
     Required('try_options'): Any(None, dict),
     Required('try_task_config'): dict,
     Required('version'): text_type,
 })
--- a/taskcluster/taskgraph/util/verify.py
+++ b/taskcluster/taskgraph/util/verify.py
@@ -40,19 +40,28 @@ class VerificationSequence(object):
     _verifications = attr.ib(factory=dict)
 
     def __call__(self, graph_name, graph, graph_config, parameters):
         for verification in self._verifications.get(graph_name, []):
             if not match_run_on_projects(parameters["project"], verification.run_on_projects):
                 continue
             scratch_pad = {}
             graph.for_each_task(
-                verification.verify, scratch_pad=scratch_pad, graph_config=graph_config
+                verification.verify,
+                scratch_pad=scratch_pad,
+                graph_config=graph_config,
+                parameters=parameters,
             )
-            verification.verify(None, graph, scratch_pad=scratch_pad, graph_config=graph_config)
+            verification.verify(
+                None,
+                graph,
+                scratch_pad=scratch_pad,
+                graph_config=graph_config,
+                parameters=parameters,
+            )
         return graph_name, graph
 
     def add(self, graph_name, run_on_projects={"all"}):
         def wrap(func):
             self._verifications.setdefault(graph_name, []).append(
                 Verification(func, run_on_projects)
             )
             return func
@@ -123,17 +132,17 @@ def verify_docs(filename, identifiers, a
         if not match_group:
             raise Exception(
                 "{}: `{}` missing from doc file: `{}`"
                 .format(appearing_as, identifier, filename)
             )
 
 
 @verifications.add('full_task_graph')
-def verify_task_graph_symbol(task, taskgraph, scratch_pad, graph_config):
+def verify_task_graph_symbol(task, taskgraph, scratch_pad, graph_config, parameters):
     """
         This function verifies that tuple
         (collection.keys(), machine.platform, groupSymbol, symbol) is unique
         for a target task graph.
     """
     if task is None:
         return
     task_dict = task.task
@@ -164,17 +173,17 @@ def verify_task_graph_symbol(task, taskg
                         join_symbol(group_symbol, symbol),
                     )
                 )
             else:
                 scratch_pad[key] = task.label
 
 
 @verifications.add('full_task_graph')
-def verify_trust_domain_v2_routes(task, taskgraph, scratch_pad, graph_config):
+def verify_trust_domain_v2_routes(task, taskgraph, scratch_pad, graph_config, parameters):
     """
     This function ensures that any two tasks have distinct ``index.{trust-domain}.v2`` routes.
     """
     if task is None:
         return
     route_prefix = "index.{}.v2".format(graph_config['trust-domain'])
     task_dict = task.task
     routes = task_dict.get('routes', [])
@@ -186,17 +195,17 @@ def verify_trust_domain_v2_routes(task, 
                     "conflict between {}:{} for route: {}"
                     .format(task.label, scratch_pad[route], route)
                 )
             else:
                 scratch_pad[route] = task.label
 
 
 @verifications.add('full_task_graph')
-def verify_routes_notification_filters(task, taskgraph, scratch_pad, graph_config):
+def verify_routes_notification_filters(task, taskgraph, scratch_pad, graph_config, parameters):
     """
         This function ensures that only understood filters for notifications are
         specified.
 
         See: https://firefox-ci-tc.services.mozilla.com/docs/manual/using/task-notifications
     """
     if task is None:
         return
@@ -212,17 +221,17 @@ def verify_routes_notification_filters(t
             if route_filter not in valid_filters:
                 raise Exception(
                     '{} has invalid notification filter ({})'
                     .format(task.label, route_filter)
                 )
 
 
 @verifications.add('full_task_graph')
-def verify_dependency_tiers(task, taskgraph, scratch_pad, graph_config):
+def verify_dependency_tiers(task, taskgraph, scratch_pad, graph_config, parameters):
     tiers = scratch_pad
     if task is not None:
         tiers[task.label] = task.task.get('extra', {}) \
                                      .get('treeherder', {}) \
                                      .get('tier', six.MAXSIZE)
     else:
         def printable_tier(tier):
             if tier == six.MAXSIZE:
@@ -239,17 +248,17 @@ def verify_dependency_tiers(task, taskgr
                 if tier < tiers[d]:
                     raise Exception(
                         '{} (tier {}) cannot depend on {} (tier {})'
                         .format(task.label, printable_tier(tier),
                                 d, printable_tier(tiers[d])))
 
 
 @verifications.add('full_task_graph')
-def verify_required_signoffs(task, taskgraph, scratch_pad, graph_config):
+def verify_required_signoffs(task, taskgraph, scratch_pad, graph_config, parameters):
     """
     Task with required signoffs can't be dependencies of tasks with less
     required signoffs.
     """
     all_required_signoffs = scratch_pad
     if task is not None:
         all_required_signoffs[task.label] = set(task.attributes.get('required_signoffs', []))
     else:
@@ -266,17 +275,17 @@ def verify_required_signoffs(task, taskg
                 if required_signoffs < all_required_signoffs[d]:
                     raise Exception(
                         '{} ({}) cannot depend on {} ({})'
                         .format(task.label, printable_signoff(required_signoffs),
                                 d, printable_signoff(all_required_signoffs[d])))
 
 
 @verifications.add('full_task_graph')
-def verify_toolchain_alias(task, taskgraph, scratch_pad, graph_config):
+def verify_toolchain_alias(task, taskgraph, scratch_pad, graph_config, parameters):
     """
         This function verifies that toolchain aliases are not reused.
     """
     if task is None:
         return
     attributes = task.attributes
     if "toolchain-alias" in attributes:
         key = attributes['toolchain-alias']
@@ -289,37 +298,38 @@ def verify_toolchain_alias(task, taskgra
                     key,
                 )
             )
         else:
             scratch_pad[key] = task.label
 
 
 @verifications.add('optimized_task_graph')
-def verify_always_optimized(task, taskgraph, scratch_pad, graph_config):
+def verify_always_optimized(task, taskgraph, scratch_pad, graph_config, parameters):
     """
         This function ensures that always-optimized tasks have been optimized.
     """
     if task is None:
         return
     if task.task.get('workerType') == 'always-optimized':
         raise Exception('Could not optimize the task {!r}'.format(task.label))
 
 
 @verifications.add('full_task_graph', run_on_projects=RELEASE_PROJECTS)
-def verify_shippable_no_sccache(task, taskgraph, scratch_pad, graph_config):
+def verify_shippable_no_sccache(task, taskgraph, scratch_pad, graph_config, parameters):
     if task and task.attributes.get('shippable'):
         if task.task.get('payload', {}).get('env', {}).get('USE_SCCACHE'):
             raise Exception(
                 'Shippable job {} cannot use sccache'.format(task.label))
 
 
 @verifications.add('full_task_graph')
-def verify_test_packaging(task, taskgraph, scratch_pad, graph_config):
+def verify_test_packaging(task, taskgraph, scratch_pad, graph_config, parameters):
     if task is None:
+        has_target_kind = parameters.get('target-kind') is None
         exceptions = []
         for task in six.itervalues(taskgraph.tasks):
             if task.kind == 'build' and not task.attributes.get('skip-verify-test-packaging'):
                 build_env = task.task.get('payload', {}).get('env', {})
                 package_tests = build_env.get('MOZ_AUTOMATION_PACKAGE_TESTS')
                 shippable = task.attributes.get('shippable', False)
                 build_has_tests = scratch_pad.get(task.label)
 
@@ -337,31 +347,34 @@ def verify_test_packaging(task, taskgrap
                             'Build job {} has tests dependent on it and does not specify '
                             'MOZ_AUTOMATION_PACKAGE_TESTS=1 in the environment'.format(task.label))
                 else:
                     # Build tasks that aren't in the scratch pad have no
                     # dependent tests, so we shouldn't package tests.
                     # With the caveat that we expect shippable jobs to always
                     # produce tests.
                     if not build_has_tests and not shippable:
-                        exceptions.append(
-                            'Build job {} has no tests, but specifies '
-                            'MOZ_AUTOMATION_PACKAGE_TESTS={} in the environment. '
-                            'Unset MOZ_AUTOMATION_PACKAGE_TESTS in the task definition '
-                            'to fix.'.format(task.label, package_tests))
+                        # If we have not generated all task kinds, we can't verify that
+                        # there are no dependent tests.
+                        if has_target_kind:
+                            exceptions.append(
+                                'Build job {} has no tests, but specifies '
+                                'MOZ_AUTOMATION_PACKAGE_TESTS={} in the environment. '
+                                'Unset MOZ_AUTOMATION_PACKAGE_TESTS in the task definition '
+                                'to fix.'.format(task.label, package_tests))
         if exceptions:
             raise Exception("\n".join(exceptions))
         return
     if task.kind == 'test':
         build_task = taskgraph[task.dependencies['build']]
         scratch_pad[build_task.label] = 1
 
 
 @verifications.add('full_task_graph')
-def verify_run_known_projects(task, taskgraph, scratch_pad, graph_config):
+def verify_run_known_projects(task, taskgraph, scratch_pad, graph_config, parameters):
     """ Validates the inputs in run-on-projects.
 
     We should never let 'try' (or 'try-comm-central') be in run-on-projects even though it
     is valid because it is not considered for try pushes.  While here we also validate for
     other unknown projects or typos.
     """
     if task and task.attributes.get('run_on_projects'):
         projects = set(task.attributes['run_on_projects'])
@@ -376,17 +389,17 @@ def verify_run_known_projects(task, task
         if invalid_projects:
             raise Exception(
                 "Task '{}' has an invalid run-on-projects value: "
                 "{}".format(task.label, invalid_projects)
             )
 
 
 @verifications.add('full_task_graph')
-def verify_local_toolchains(task, taskgraph, scratch_pad, graph_config):
+def verify_local_toolchains(task, taskgraph, scratch_pad, graph_config, parameters):
     """
     Toolchains that are used for local development need to be built on a
     level-3 branch to installable via `mach bootstrap`. We ensure here that all
     such tasks run on at least trunk projects, even if they aren't pulled in as
     a dependency of other tasks in the graph.
 
     There is code in `mach artifact toolchain` that verifies that anything
     installed via `mach bootstrap` has the attribute set.
--- a/taskcluster/test/conftest.py
+++ b/taskcluster/test/conftest.py
@@ -30,19 +30,19 @@ def datadir():
 
 @pytest.fixture(scope="session")
 def create_tgg(responses, datadir):
 
     # Setup logging.
     lm = LoggingManager()
     lm.add_terminal_logging()
 
-    def inner(parameters=None, overrides=None, target_kind=None):
+    def inner(parameters=None, overrides=None):
         params = parameters_loader(parameters, strict=False, overrides=overrides)
-        tgg = TaskGraphGenerator(None, params, target_kind=target_kind)
+        tgg = TaskGraphGenerator(None, params)
 
         # Mock out certain requests as they may depend on a revision that does
         # not exist on hg.mozilla.org.
         mock_requests = {}
 
         # bugbug /push/schedules
         url = BUGBUG_BASE_URL + "/push/{project}/{head_rev}/schedules".format(**tgg.parameters)
         mock_requests[url] = "bugbug-push-schedules.json"
--- a/taskcluster/test/test_mach_try_auto.py
+++ b/taskcluster/test/test_mach_try_auto.py
@@ -14,19 +14,19 @@ from taskgraph.util.chunking import Bugb
 
 pytestmark = pytest.mark.slow
 
 
 @pytest.fixture(scope="module")
 def tgg(create_tgg):
     params = TRY_AUTO_PARAMETERS.copy()
     params.update(
-        {"head_repository": "https://hg.mozilla.org/try", "project": "try"}
+        {"head_repository": "https://hg.mozilla.org/try", "project": "try", "target_kind": "test"}
     )
-    tgg = create_tgg(overrides=params, target_kind="test")
+    tgg = create_tgg(overrides=params)
     return tgg
 
 
 @pytest.fixture(scope="module")
 def params(tgg):
     return tgg.parameters