move target dict to TestBase and clean up DeclarativeTaskTestMixin

tests/python/pants_test/backend/python/tasks/test_build_local_python_distributions.py

@@ -17,7 +17,7 @@
   BuildLocalPythonDistributionsTestBase
 
 
-class TestBuildLocalDistsNativeSources(BuildLocalPythonDistributionsTestBase):
+class TestBuildLocalPythonDistributions(BuildLocalPythonDistributionsTestBase):
 
   dist_specs = OrderedDict([
 

tests/python/pants_test/backend/python/tasks/util/build_local_dists_test_base.py

@@ -39,9 +39,15 @@ def run_after_task_types(cls):
   def rules(cls):
     return super(BuildLocalPythonDistributionsTestBase, cls).rules() + native_backend_rules()
 
+  @classproperty
+  def dist_specs(cls):
+    """Fed into `self.populate_target_dict()`."""
+    raise NotImplementedError('dist_specs must be implemented!')
+
   def setUp(self):
     super(BuildLocalPythonDistributionsTestBase, self).setUp()
-    self.populate_target_dict()
+    # Share the target mapping across all test cases.
+    self.target_dict = self.populate_target_dict(self.dist_specs)
 
   def _get_dist_snapshot_version(self, task, python_dist_target):
     """Get the target's fingerprint, and guess the resulting version string of the built dist.
@@ -65,7 +71,11 @@ def _get_dist_snapshot_version(self, task, python_dist_target):
     return re.sub(r'[^a-zA-Z0-9]', '.', versioned_target_fingerprint.lower())
 
   def _create_distribution_synthetic_target(self, python_dist_target, extra_targets=[]):
-    context, _, python_create_distributions_task_instance, _ = self.invoke_tasks(
+    all_specified_targets = list(self.target_dict.values())
+    result = self.invoke_tasks(
+      # We set `target_closure` to check that all the targets in the build graph are exactly the
+      # ones we've just created before building python_dist()s (which creates further targets).
+      target_closure=all_specified_targets,
       target_roots=[python_dist_target] + extra_targets,
       for_subsystems=[PythonRepos, LibcDev],
       # TODO(#6848): we should be testing all of these with both of our toolchains.
@@ -74,8 +84,10 @@ def _create_distribution_synthetic_target(self, python_dist_target, extra_target
           'toolchain_variant': 'llvm',
         },
       })
+    context = result.context
+    python_create_distributions_task_instance = result.this_task
 
-    synthetic_tgts = set(context.build_graph.targets()) - set(self._all_specified_targets())
+    synthetic_tgts = set(context.build_graph.targets()) - set(all_specified_targets)
     self.assertEqual(1, len(synthetic_tgts))
     synthetic_target = next(iter(synthetic_tgts))
 

tests/python/pants_test/task_test_base.py

@@ -12,13 +12,14 @@
 
 from future.utils import PY2
 
-from pants.build_graph.address import Address
 from pants.goal.goal import Goal
 from pants.ivy.bootstrapper import Bootstrapper
 from pants.task.console_task import ConsoleTask
+from pants.task.task import Task
 from pants.util.contextutil import temporary_dir
 from pants.util.memo import memoized_method
 from pants.util.meta import classproperty
+from pants.util.objects import SubclassesOf, TypedCollection, datatype
 from pants.util.process_handler import subprocess
 from pants_test.test_base import TestBase
 
@@ -306,87 +307,52 @@ def assert_console_raises(self, exception, **kwargs):
 
 
 class DeclarativeTaskTestMixin(object):
-  """Experimental mixin combining target descriptions with a file path dict.
+  """Experimental mixin for task tests allows specifying tasks to be run before or after the task.
 
-  This class should be mixed in to subclasses of `TaskTestBase`!
-
-  NB: `self.populate_target_dict()` should be called in the `setUp()` method to use the target specs
-  specified in `dist_specs`!
-
-  This mixin also allows specifying tasks to be run before or after the task_type() is executed when
-  calling `self.invoke_tasks()`.
+  Calling `self.invoke_tasks()` will create instances of and execute task types in
+  `self.run_before_task_types()`, then `task_type()`, then `self.run_after_task_types()`.
   """
 
-  @classproperty
-  def dist_specs(cls):
-    """This is an informally-specified nested dict.
-
-    Special keys are 'key' (used to index into `self.target_dict`) and 'filemap' (creates files at
-    the specified relative paths). The rest of the keys are fed into `self.make_target()`. An
-    `OrderedDict` of 2-tuples may be used if targets need to be created in a specific order (e.g. if
-    they have dependencies on each other).
-    """
-    raise NotImplementedError('dist_specs must be implemented!')
-
   @classproperty
   def run_before_task_types(cls):
-    """
-    By default, we just use a `BuildLocalPythonDistributions` task. When testing with C/C++ targets,
-    we want to compile and link them as well to get the resulting dist to build, so we add those
-    task types here and execute them beforehand.
-    """
     return []
 
   @classproperty
   def run_after_task_types(cls):
-    """Tasks to run after local dists are built, similar to `run_before_task_types`."""
     return []
 
-  def populate_target_dict(self):
-    self.target_dict = {}
-
-    # Create a target from each specification and insert it into `self.target_dict`.
-    for target_spec, target_kwargs in self.dist_specs.items():
-      unprocessed_kwargs = target_kwargs.copy()
-
-      target_base = Address.parse(target_spec).spec_path
-
-      # Populate the target's owned files from the specification.
-      filemap = unprocessed_kwargs.pop('filemap', {})
-      for rel_path, content in filemap.items():
-        buildroot_path = os.path.join(target_base, rel_path)
-        self.create_file(buildroot_path, content)
-
-      # Ensure any dependencies exist in the target dict (`dist_specs` must then be an
-      # OrderedDict).
-      # The 'key' is used to access the target in `self.target_dict`.
-      key = unprocessed_kwargs.pop('key')
-      dep_targets = []
-      for dep_spec in unprocessed_kwargs.pop('dependencies', []):
-        existing_tgt_key = self.dist_specs[dep_spec]['key']
-        dep_targets.append(self.target_dict[existing_tgt_key])
-
-      # Register the generated target.
-      generated_target = self.make_target(
-        spec=target_spec, dependencies=dep_targets, **unprocessed_kwargs)
-      self.target_dict[key] = generated_target
-
   @memoized_method
   def _synthesize_task_types(self, task_types=()):
     return [
       self.synthesize_task_subtype(tsk, '__tmp_{}'.format(tsk.__name__))
-      # TODO: make @memoized_method convert lists to tuples for hashing!
+      # TODO(#7127): make @memoized_method convert lists to tuples for hashing!
       for tsk in task_types
     ]
 
-  def _all_specified_targets(self):
-    return list(self.target_dict.values())
-
   def _create_task(self, task_type, context):
     """Helper method to instantiate tasks besides self._testing_task_type in the test workdir."""
     return task_type(context, self.test_workdir)
 
-  def invoke_tasks(self, **context_kwargs):
+  class TaskInvocationResult(datatype([
+      'context',
+      ('before_tasks', TypedCollection(SubclassesOf(Task))),
+      ('this_task', SubclassesOf(Task)),
+      ('after_tasks', TypedCollection(SubclassesOf(Task))),
+  ])): pass
+
+  def invoke_tasks(self, target_closure=None, **context_kwargs):
+    """Create and execute the declaratively specified tasks in order.
+
+    Create instances of and execute task types in `self.run_before_task_types()`, then
+    `task_type()`, then `self.run_after_task_types()`.
+
+    :param Iterable target_closure: If not None, check that the build graph contains exactly these
+                                    targets before executing the tasks.
+    :param **context_kwargs: kwargs passed to `self.context()`. Note that this method already sets
+                                    `for_task_types`.
+    :return: A datatype containing the created context and the task instances which were executed.
+    :rtype: :class:`DeclarativeTaskTestMixin.TaskInvocationResult`
+    """
     run_before_synthesized_task_types = self._synthesize_task_types(tuple(self.run_before_task_types))
     run_after_synthesized_task_types = self._synthesize_task_types(tuple(self.run_after_task_types))
     all_synthesized_task_types = run_before_synthesized_task_types + [
@@ -396,7 +362,8 @@ def invoke_tasks(self, **context_kwargs):
     context = self.context(
       for_task_types=all_synthesized_task_types,
       **context_kwargs)
-    self.assertEqual(set(self._all_specified_targets()), set(context.build_graph.targets()))
+    if target_closure is not None:
+      self.assertEqual(set(target_closure), set(context.build_graph.targets()))
 
     run_before_task_instances = [
       self._create_task(task_type, context)
@@ -415,4 +382,8 @@ def invoke_tasks(self, **context_kwargs):
     for tsk in all_task_instances:
       tsk.execute()
 
-    return (context, run_before_task_instances, current_task_instance, run_after_task_instances)
+    return self.TaskInvocationResult(
+      context=context,
+      before_tasks=run_before_task_instances,
+      this_task=current_task_instance,
+      after_tasks=run_after_task_instances)

tests/python/pants_test/test_base.py

@@ -681,3 +681,51 @@ def retrieve_single_product_at_target_base(self, product_mapping, target):
     single_base_dir = assert_single_element(list(mapping_for_target.keys()))
     single_product = assert_single_element(mapping_for_target[single_base_dir])
     return single_product
+
+  def populate_target_dict(self, target_map):
+    """Return a dict containing targets with files generated according to `target_map`.
+
+    The values of `target_map` should themselves be a flat dict, which contains keyword arguments
+    fed into `self.make_target()`, along with a few special keys. Special keys are:
+    - 'key' (required -- used to index into the returned dict).
+    - 'filemap' (creates files at the specified relative paths to the target).
+
+    An `OrderedDict` of 2-tuples must be used with the targets topologically ordered, if
+    they have dependencies on each other. Note that dependency cycles are not currently supported
+    with this method.
+
+    :param target_map: Dict mapping each target address to generate -> kwargs for
+                       `self.make_target()`, along with a 'key' and optionally a 'filemap' argument.
+    :return: Dict mapping the required 'key' argument -> target instance for each element of
+             `target_map`.
+    :rtype: dict
+    """
+    target_dict = {}
+
+    # Create a target from each specification and insert it into `target_dict`.
+    for target_spec, target_kwargs in target_map.items():
+      unprocessed_kwargs = target_kwargs.copy()
+
+      target_base = Address.parse(target_spec).spec_path
+
+      # Populate the target's owned files from the specification.
+      filemap = unprocessed_kwargs.pop('filemap', {})
+      for rel_path, content in filemap.items():
+        buildroot_path = os.path.join(target_base, rel_path)
+        self.create_file(buildroot_path, content)
+
+      # Ensure any dependencies exist in the target dict (`target_map` must then be an
+      # OrderedDict).
+      # The 'key' is used to access the target in `target_dict`.
+      key = unprocessed_kwargs.pop('key')
+      dep_targets = []
+      for dep_spec in unprocessed_kwargs.pop('dependencies', []):
+        existing_tgt_key = target_map[dep_spec]['key']
+        dep_targets.append(target_dict[existing_tgt_key])
+
+      # Register the generated target.
+      generated_target = self.make_target(
+        spec=target_spec, dependencies=dep_targets, **unprocessed_kwargs)
+      target_dict[key] = generated_target
+
+    return target_dict