Merge branch 'master' of github.com:pantsbuild/pants into move-testinfra

build-support/githooks/pre-commit

@@ -53,7 +53,7 @@ echo "* Checking shell scripts via our custom linter"
 # fails in pants changed.
 if git rev-parse --verify "${MERGE_BASE}" &>/dev/null; then
   echo "* Checking imports"
-  ./build-support/bin/isort.sh || die "To fix import sort order, run \`\"$(pwd)/build-support/bin/isort.sh\" -f\`"
+  ./build-support/bin/isort.sh || die "To fix import sort order, run \`build-support/bin/isort.sh -f\`"
 
   # TODO(CMLivingston) Make lint use `-q` option again after addressing proper workunit labeling:
   # https://github.com/pantsbuild/pants/issues/6633

src/python/pants/build_graph/build_configuration.py

@@ -170,7 +170,7 @@ def rules(self):
     return list(self._rules)
 
   def union_rules(self):
-    """Returns a mapping of registered union base types -> [a list of union member types].
+    """Returns a mapping of registered union base types -> [OrderedSet of union member types].
 
     :rtype: OrderedDict
     """

src/python/pants/engine/rules.py

@@ -10,8 +10,9 @@
 from abc import ABC, abstractmethod
 from collections import OrderedDict
 from collections.abc import Iterable
+from dataclasses import dataclass
 from textwrap import dedent
-from typing import Any, Callable, Type, cast
+from typing import Any, Callable, Dict, Type, cast
 
 import asttokens
 from twitter.common.collections import OrderedSet
@@ -428,6 +429,17 @@ def __new__(cls, union_base, union_member):
     return super().__new__(cls, union_base, union_member)
 
 
+@dataclass(frozen=True)
+class UnionMembership:
+  union_rules: Dict[type, typing.Iterable[type]]
+
+  def is_member(self, union_type, putative_member):
+    members = self.union_rules.get(union_type)
+    if members is None:
+      raise TypeError(f'Not a registered union type: {union_type}')
+    return type(putative_member) in members
+
+
 class Rule(ABC):
   """Rules declare how to produce products for the product graph.
 

src/python/pants/init/engine_initializer.py

@@ -42,7 +42,7 @@
 from pants.engine.mapper import AddressMapper
 from pants.engine.parser import SymbolTable
 from pants.engine.platform import create_platform_rules
-from pants.engine.rules import RootRule, rule
+from pants.engine.rules import RootRule, UnionMembership, rule
 from pants.engine.scheduler import Scheduler
 from pants.engine.selectors import Params
 from pants.init.options_initializer import BuildConfigInitializer, OptionsInitializer
@@ -357,6 +357,10 @@ def build_configuration_singleton() -> BuildConfiguration:
     def symbol_table_singleton() -> SymbolTable:
       return symbol_table
 
+    @rule
+    def union_membership_singleton() -> UnionMembership:
+      return UnionMembership(build_configuration.union_rules())
+
     # Create a Scheduler containing graph and filesystem rules, with no installed goals. The
     # LegacyBuildGraph will explicitly request the products it needs.
     rules = (
@@ -365,6 +369,7 @@ def symbol_table_singleton() -> SymbolTable:
         glob_match_error_behavior_singleton,
         build_configuration_singleton,
         symbol_table_singleton,
+        union_membership_singleton,
       ] +
       create_legacy_graph_tasks() +
       create_fs_rules() +

src/python/pants/rules/core/test.py

@@ -2,14 +2,16 @@
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
 import logging
+from dataclasses import dataclass
+from typing import Optional
 
 from pants.base.exiter import PANTS_FAILED_EXIT_CODE, PANTS_SUCCEEDED_EXIT_CODE
-from pants.build_graph.address import Address
+from pants.build_graph.address import Address, BuildFileAddress
 from pants.engine.addressable import BuildFileAddresses
 from pants.engine.console import Console
 from pants.engine.goal import Goal
 from pants.engine.legacy.graph import HydratedTarget
-from pants.engine.rules import console_rule, rule
+from pants.engine.rules import UnionMembership, console_rule, rule
 from pants.engine.selectors import Get
 from pants.rules.core.core_test_model import Status, TestResult, TestTarget
 
@@ -24,18 +26,27 @@ class Test(Goal):
   name = 'test'
 
 
+@dataclass(frozen=True)
+class AddressAndTestResult:
+  address: BuildFileAddress
+  test_result: Optional[TestResult]  # If None, target was not a test target.
+
+
 @console_rule
 def fast_test(console: Console, addresses: BuildFileAddresses) -> Test:
-  test_results = yield [Get(TestResult, Address, address.to_address()) for address in addresses]
+  results = yield [Get(AddressAndTestResult, Address, addr.to_address()) for addr in addresses]
   did_any_fail = False
-  for address, test_result in zip(addresses, test_results):
+  filtered_results = [(x.address, x.test_result) for x in results if x.test_result is not None]
+
+  for address, test_result in filtered_results:
     if test_result.status == Status.FAILURE:
       did_any_fail = True
     if test_result.stdout:
       console.write_stdout(
         "{} stdout:\n{}\n".format(
           address.reference(),
-          console.red(test_result.stdout) if test_result.status == Status.FAILURE else test_result.stdout
+          (console.red(test_result.stdout) if test_result.status == Status.FAILURE
+           else test_result.stdout)
         )
       )
     if test_result.stderr:
@@ -44,14 +55,16 @@ def fast_test(console: Console, addresses: BuildFileAddresses) -> Test:
       console.write_stdout(
         "{} stderr:\n{}\n".format(
           address.reference(),
-          console.red(test_result.stderr) if test_result.status == Status.FAILURE else test_result.stderr
+          (console.red(test_result.stderr) if test_result.status == Status.FAILURE
+           else test_result.stderr)
         )
       )
 
   console.write_stdout("\n")
 
-  for address, test_result in zip(addresses, test_results):
-    console.print_stdout('{0:80}.....{1:>10}'.format(address.reference(), test_result.status.value))
+  for address, test_result in filtered_results:
+    console.print_stdout('{0:80}.....{1:>10}'.format(
+      address.reference(), test_result.status.value))
 
   if did_any_fail:
     console.print_stderr(console.red('Tests failed'))
@@ -63,19 +76,24 @@ def fast_test(console: Console, addresses: BuildFileAddresses) -> Test:
 
 
 @rule
-def coordinator_of_tests(target: HydratedTarget) -> TestResult:
+def coordinator_of_tests(target: HydratedTarget,
+                         union_membership: UnionMembership) -> AddressAndTestResult:
   # TODO(#6004): when streaming to live TTY, rely on V2 UI for this information. When not a
   # live TTY, periodically dump heavy hitters to stderr. See
   # https://github.com/pantsbuild/pants/issues/6004#issuecomment-492699898.
-  logger.info("Starting tests: {}".format(target.address.reference()))
-  # NB: This has the effect of "casting" a TargetAdaptor to a member of the TestTarget union. If the
-  # TargetAdaptor is not a member of the union, it will fail at runtime with a useful error message.
-  result = yield Get(TestResult, TestTarget, target.adaptor)
-  logger.info("Tests {}: {}".format(
-    "succeeded" if result.status == Status.SUCCESS else "failed",
-    target.address.reference(),
-  ))
-  yield result
+  if union_membership.is_member(TestTarget, target.adaptor):
+    logger.info("Starting tests: {}".format(target.address.reference()))
+    # NB: This has the effect of "casting" a TargetAdaptor to a member of the TestTarget union.
+    # The adaptor will always be a member because of the union membership check above, but if
+    # it were not it would fail at runtime with a useful error message.
+    result = yield Get(TestResult, TestTarget, target.adaptor)
+    logger.info("Tests {}: {}".format(
+      "succeeded" if result.status == Status.SUCCESS else "failed",
+      target.address.reference(),
+    ))
+  else:
+    result = None  # Not a test target.
+  yield AddressAndTestResult(target.address, result)
 
 
 def rules():