From 8e33ec15fce0625c9aa69ccff2469c39954143b4 Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Mon, 30 Sep 2019 14:19:05 -0700 Subject: [PATCH 01/11] Pre-filter TestTarget @union members. Allows running tests over globbed targets without erroring on any non-test targets matched by the glob. Provides a mechanism that any console_rule can use for similar effects if needed. --- src/python/pants/build_graph/build_configuration.py | 6 ++++++ src/python/pants/rules/core/test.py | 12 +++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/python/pants/build_graph/build_configuration.py b/src/python/pants/build_graph/build_configuration.py index 5512d3a3910..d9a5784b1de 100644 --- a/src/python/pants/build_graph/build_configuration.py +++ b/src/python/pants/build_graph/build_configuration.py @@ -176,6 +176,12 @@ def union_rules(self): """ return self._union_rules + def union_members(self, union_type, targets): + if union_type not in self._union_rules: + raise TypeError(f'Not a registered union type: {union_type}') + test_target_adaptors = set(self._union_rules[union_type]) + return [tgt for tgt in targets if type(tgt.adaptor) in test_target_adaptors] + @memoized_method def _get_addressable_factory(self, target_type, alias): return TargetAddressable.factory(target_type=target_type, alias=alias) diff --git a/src/python/pants/rules/core/test.py b/src/python/pants/rules/core/test.py index f4e8d1dc50f..a6b4bbd4e51 100644 --- a/src/python/pants/rules/core/test.py +++ b/src/python/pants/rules/core/test.py @@ -4,16 +4,15 @@ import logging from pants.base.exiter import PANTS_FAILED_EXIT_CODE, PANTS_SUCCEEDED_EXIT_CODE -from pants.build_graph.address import Address +from pants.build_graph.build_configuration import BuildConfiguration 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.legacy.graph import HydratedTarget, HydratedTargets from pants.engine.rules import console_rule, rule from pants.engine.selectors import Get from pants.rules.core.core_test_model import Status, TestResult, TestTarget - # TODO(#6004): use proper Logging singleton, rather than static logger. logger = logging.getLogger(__name__) @@ -25,8 +24,11 @@ class Test(Goal): @console_rule -def fast_test(console: Console, addresses: BuildFileAddresses) -> Test: - test_results = yield [Get(TestResult, Address, address.to_address()) for address in addresses] +def fast_test(console: Console, addresses: BuildFileAddresses, + build_config: BuildConfiguration) -> Test: + all_targets = yield Get(HydratedTargets, BuildFileAddresses, addresses) + filtered_targets = build_config.union_members(TestTarget, all_targets) + test_results = yield [Get(TestResult, HydratedTarget, tgt) for tgt in filtered_targets] did_any_fail = False for address, test_result in zip(addresses, test_results): if test_result.status == Status.FAILURE: From 15c4c3a49a9b8195ad4048483dacf2f30544a210 Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Mon, 30 Sep 2019 15:45:15 -0700 Subject: [PATCH 02/11] Address code review comments. --- .../pants/build_graph/build_configuration.py | 10 ++++---- src/python/pants/rules/core/test.py | 23 ++++++++++--------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/python/pants/build_graph/build_configuration.py b/src/python/pants/build_graph/build_configuration.py index d9a5784b1de..dc7053af34c 100644 --- a/src/python/pants/build_graph/build_configuration.py +++ b/src/python/pants/build_graph/build_configuration.py @@ -170,17 +170,17 @@ 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 """ return self._union_rules - def union_members(self, union_type, targets): - if union_type not in self._union_rules: + def is_union_member(self, union_type, tgt): + try: + return type(tgt.adaptor) in self._union_rules[union_type] + except KeyError: raise TypeError(f'Not a registered union type: {union_type}') - test_target_adaptors = set(self._union_rules[union_type]) - return [tgt for tgt in targets if type(tgt.adaptor) in test_target_adaptors] @memoized_method def _get_addressable_factory(self, target_type, alias): diff --git a/src/python/pants/rules/core/test.py b/src/python/pants/rules/core/test.py index a6b4bbd4e51..a2909abbb0f 100644 --- a/src/python/pants/rules/core/test.py +++ b/src/python/pants/rules/core/test.py @@ -24,20 +24,19 @@ class Test(Goal): @console_rule -def fast_test(console: Console, addresses: BuildFileAddresses, - build_config: BuildConfiguration) -> Test: - all_targets = yield Get(HydratedTargets, BuildFileAddresses, addresses) - filtered_targets = build_config.union_members(TestTarget, all_targets) +def fast_test(console: Console, targets: HydratedTargets, build_config: BuildConfiguration) -> Test: + filtered_targets = [tgt for tgt in targets if build_config.is_union_member(TestTarget, tgt)] test_results = yield [Get(TestResult, HydratedTarget, tgt) for tgt in filtered_targets] did_any_fail = False - for address, test_result in zip(addresses, test_results): + for tgt, test_result in zip(filtered_targets, test_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 + tgt.address.reference(), + (console.red(test_result.stdout) if test_result.status == Status.FAILURE + else test_result.stdout) ) ) if test_result.stderr: @@ -45,15 +44,17 @@ def fast_test(console: Console, addresses: BuildFileAddresses, # two streams. console.write_stdout( "{} stderr:\n{}\n".format( - address.reference(), - console.red(test_result.stderr) if test_result.status == Status.FAILURE else test_result.stderr + tgt.address.reference(), + (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 tgt, test_result in zip(filtered_targets, test_results): + console.print_stdout('{0:80}.....{1:>10}'.format( + tgt.address.reference(), test_result.status.value)) if did_any_fail: console.print_stderr(console.red('Tests failed')) From 770a36cfd54cbc78487db58632779240fa00909e Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Tue, 1 Oct 2019 11:51:39 -0700 Subject: [PATCH 03/11] Don't use BuildConfiguration directly. Limits the exposed surface area. --- src/python/pants/engine/rules.py | 17 ++++++++++++++--- src/python/pants/init/engine_initializer.py | 7 ++++++- src/python/pants/rules/core/test.py | 10 +++++----- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/python/pants/engine/rules.py b/src/python/pants/engine/rules.py index 450eb7eeb51..518af03a8d4 100644 --- a/src/python/pants/engine/rules.py +++ b/src/python/pants/engine/rules.py @@ -6,12 +6,12 @@ import itertools import logging import sys -import typing 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, Iterable, Type, cast import asttokens from twitter.common.collections import OrderedSet @@ -211,7 +211,7 @@ def _get_starting_indent(source): def _make_rule( - return_type: type, parameter_types: typing.Iterable[type], cacheable: bool = True + return_type: type, parameter_types: Iterable[type], cacheable: bool = True ) -> Callable[[Callable], Callable]: """A @decorator that declares that a particular static function may be used as a TaskRule. @@ -427,6 +427,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, Iterable[type]] + + def is_union_member(self, union_type, tgt): + try: + return type(tgt.adaptor) in self.union_rules[union_type] + except KeyError: + raise TypeError(f'Not a registered union type: {union_type}') + + class Rule(ABC): """Rules declare how to produce products for the product graph. diff --git a/src/python/pants/init/engine_initializer.py b/src/python/pants/init/engine_initializer.py index 58f071a5c6e..d2628fcba82 100644 --- a/src/python/pants/init/engine_initializer.py +++ b/src/python/pants/init/engine_initializer.py @@ -40,7 +40,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, rule, UnionMembership from pants.engine.scheduler import Scheduler from pants.engine.selectors import Params from pants.init.options_initializer import BuildConfigInitializer, OptionsInitializer @@ -348,6 +348,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 = ( @@ -356,6 +360,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() + diff --git a/src/python/pants/rules/core/test.py b/src/python/pants/rules/core/test.py index e9a53d94c02..9a3e739d429 100644 --- a/src/python/pants/rules/core/test.py +++ b/src/python/pants/rules/core/test.py @@ -4,12 +4,10 @@ import logging from pants.base.exiter import PANTS_FAILED_EXIT_CODE, PANTS_SUCCEEDED_EXIT_CODE -from pants.build_graph.build_configuration import BuildConfiguration -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, HydratedTargets -from pants.engine.rules import console_rule, rule +from pants.engine.rules import console_rule, rule, UnionMembership from pants.engine.selectors import Get from pants.rules.core.core_test_model import Status, TestResult, TestTarget @@ -24,8 +22,10 @@ class Test(Goal): @console_rule -def fast_test(console: Console, targets: HydratedTargets, build_config: BuildConfiguration) -> Test: - filtered_targets = [tgt for tgt in targets if build_config.is_union_member(TestTarget, tgt)] +def fast_test(console: Console, + targets: HydratedTargets, + union_membership: UnionMembership) -> Test: + filtered_targets = [tgt for tgt in targets if union_membership.is_union_member(TestTarget, tgt)] test_results = yield [Get(TestResult, HydratedTarget, tgt) for tgt in filtered_targets] did_any_fail = False for tgt, test_result in zip(filtered_targets, test_results): From b194f323824fbcdb03c5786cfddc966a8ebbe368 Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Tue, 1 Oct 2019 13:28:14 -0700 Subject: [PATCH 04/11] Address code review comments. --- src/python/pants/build_graph/build_configuration.py | 6 ------ src/python/pants/engine/rules.py | 8 ++++---- src/python/pants/rules/core/test.py | 2 +- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/python/pants/build_graph/build_configuration.py b/src/python/pants/build_graph/build_configuration.py index dc7053af34c..d99070e9ddf 100644 --- a/src/python/pants/build_graph/build_configuration.py +++ b/src/python/pants/build_graph/build_configuration.py @@ -176,12 +176,6 @@ def union_rules(self): """ return self._union_rules - def is_union_member(self, union_type, tgt): - try: - return type(tgt.adaptor) in self._union_rules[union_type] - except KeyError: - raise TypeError(f'Not a registered union type: {union_type}') - @memoized_method def _get_addressable_factory(self, target_type, alias): return TargetAddressable.factory(target_type=target_type, alias=alias) diff --git a/src/python/pants/engine/rules.py b/src/python/pants/engine/rules.py index 518af03a8d4..03bab66ed63 100644 --- a/src/python/pants/engine/rules.py +++ b/src/python/pants/engine/rules.py @@ -431,11 +431,11 @@ def __new__(cls, union_base, union_member): class UnionMembership: union_rules: Dict[type, Iterable[type]] - def is_union_member(self, union_type, tgt): - try: - return type(tgt.adaptor) in self.union_rules[union_type] - except KeyError: + 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): diff --git a/src/python/pants/rules/core/test.py b/src/python/pants/rules/core/test.py index 9a3e739d429..03df12736de 100644 --- a/src/python/pants/rules/core/test.py +++ b/src/python/pants/rules/core/test.py @@ -25,7 +25,7 @@ class Test(Goal): def fast_test(console: Console, targets: HydratedTargets, union_membership: UnionMembership) -> Test: - filtered_targets = [tgt for tgt in targets if union_membership.is_union_member(TestTarget, tgt)] + filtered_targets = [tgt for tgt in targets if union_membership.is_member(TestTarget, tgt.adaptor)] test_results = yield [Get(TestResult, HydratedTarget, tgt) for tgt in filtered_targets] did_any_fail = False for tgt, test_result in zip(filtered_targets, test_results): From ca465ac9043b9b7e5e81bf4da7c76f57281d3c1a Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Fri, 4 Oct 2019 10:09:30 -0700 Subject: [PATCH 05/11] Lint --- src/python/pants/init/engine_initializer.py | 2 +- src/python/pants/rules/core/test.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/python/pants/init/engine_initializer.py b/src/python/pants/init/engine_initializer.py index d06bc406b0f..65bea4ed791 100644 --- a/src/python/pants/init/engine_initializer.py +++ b/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, UnionMembership +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 diff --git a/src/python/pants/rules/core/test.py b/src/python/pants/rules/core/test.py index 03df12736de..a9632cc3019 100644 --- a/src/python/pants/rules/core/test.py +++ b/src/python/pants/rules/core/test.py @@ -7,10 +7,11 @@ from pants.engine.console import Console from pants.engine.goal import Goal from pants.engine.legacy.graph import HydratedTarget, HydratedTargets -from pants.engine.rules import console_rule, rule, UnionMembership +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 + # TODO(#6004): use proper Logging singleton, rather than static logger. logger = logging.getLogger(__name__) From 7ec2092be2a852a75a465c0fbdd6c2e67a7ebe62 Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Fri, 4 Oct 2019 21:30:36 -0700 Subject: [PATCH 06/11] Fix bad import collision. --- src/python/pants/engine/rules.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/python/pants/engine/rules.py b/src/python/pants/engine/rules.py index c1d5a004632..d5b06e34acc 100644 --- a/src/python/pants/engine/rules.py +++ b/src/python/pants/engine/rules.py @@ -6,12 +6,13 @@ import itertools import logging import sys +import typing 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, Dict, Iterable, Type, cast +from typing import Any, Callable, Dict, Type, cast import asttokens from twitter.common.collections import OrderedSet @@ -211,7 +212,7 @@ def _get_starting_indent(source): def _make_rule( - return_type: type, parameter_types: Iterable[type], cacheable: bool = True + return_type: type, parameter_types: typing.Iterable[type], cacheable: bool = True ) -> Callable[[Callable], Callable]: """A @decorator that declares that a particular static function may be used as a TaskRule. @@ -430,7 +431,7 @@ def __new__(cls, union_base, union_member): @dataclass(frozen=True) class UnionMembership: - union_rules: Dict[type, Iterable[type]] + union_rules: Dict[type, typing.Iterable[type]] def is_member(self, union_type, putative_member): members = self.union_rules.get(union_type) From 85d54cca55fff4fe31879cbd7cbd15585b4f0c03 Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Fri, 4 Oct 2019 21:51:12 -0700 Subject: [PATCH 07/11] Address code review feedback. --- src/python/pants/rules/core/test.py | 58 ++++++++++++++++++----------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/src/python/pants/rules/core/test.py b/src/python/pants/rules/core/test.py index a9632cc3019..c77333ab65e 100644 --- a/src/python/pants/rules/core/test.py +++ b/src/python/pants/rules/core/test.py @@ -2,16 +2,20 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). import logging +from typing import Optional + +from dataclasses import dataclass from pants.base.exiter import PANTS_FAILED_EXIT_CODE, PANTS_SUCCEEDED_EXIT_CODE +from pants.build_graph.address import BuildFileAddress, Address +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, HydratedTargets +from pants.engine.legacy.graph import HydratedTarget 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 - # TODO(#6004): use proper Logging singleton, rather than static logger. logger = logging.getLogger(__name__) @@ -22,20 +26,25 @@ class Test(Goal): name = 'test' +@dataclass +class AddressAndTestResult: + address: BuildFileAddress + test_result: Optional[TestResult] # If None, target was not a test target. + + @console_rule -def fast_test(console: Console, - targets: HydratedTargets, - union_membership: UnionMembership) -> Test: - filtered_targets = [tgt for tgt in targets if union_membership.is_member(TestTarget, tgt.adaptor)] - test_results = yield [Get(TestResult, HydratedTarget, tgt) for tgt in filtered_targets] +def fast_test(console: Console, addresses: BuildFileAddresses) -> Test: + results = yield [Get(AddressAndTestResult, Address, addr.to_address()) for addr in addresses] did_any_fail = False - for tgt, test_result in zip(filtered_targets, 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( - tgt.address.reference(), + address.reference(), (console.red(test_result.stdout) if test_result.status == Status.FAILURE else test_result.stdout) ) @@ -45,7 +54,7 @@ def fast_test(console: Console, # two streams. console.write_stdout( "{} stderr:\n{}\n".format( - tgt.address.reference(), + address.reference(), (console.red(test_result.stderr) if test_result.status == Status.FAILURE else test_result.stderr) ) @@ -53,9 +62,9 @@ def fast_test(console: Console, console.write_stdout("\n") - for tgt, test_result in zip(filtered_targets, test_results): + for address, test_result in filtered_results: console.print_stdout('{0:80}.....{1:>10}'.format( - tgt.address.reference(), test_result.status.value)) + address.reference(), test_result.status.value)) if did_any_fail: console.print_stderr(console.red('Tests failed')) @@ -67,19 +76,24 @@ def fast_test(console: Console, @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(): From 8f133774659be30031f3118056210d2658a8dd57 Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Sat, 5 Oct 2019 10:38:32 -0700 Subject: [PATCH 08/11] Lint fix. --- build-support/githooks/pre-commit | 2 +- src/python/pants/rules/core/test.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/build-support/githooks/pre-commit b/build-support/githooks/pre-commit index 45b6d8108cb..27524b031cd 100755 --- a/build-support/githooks/pre-commit +++ b/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 diff --git a/src/python/pants/rules/core/test.py b/src/python/pants/rules/core/test.py index c77333ab65e..4d7ed7bebcd 100644 --- a/src/python/pants/rules/core/test.py +++ b/src/python/pants/rules/core/test.py @@ -2,12 +2,11 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). import logging -from typing import Optional - 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 BuildFileAddress, 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 @@ -16,6 +15,7 @@ from pants.engine.selectors import Get from pants.rules.core.core_test_model import Status, TestResult, TestTarget + # TODO(#6004): use proper Logging singleton, rather than static logger. logger = logging.getLogger(__name__) From a6507f1fcf2765b5f96a398272effc67451f5199 Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Sat, 5 Oct 2019 11:02:21 -0700 Subject: [PATCH 09/11] Fix tests --- tests/python/pants_test/rules/test_test.py | 39 +++++++++++++++------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/tests/python/pants_test/rules/test_test.py b/tests/python/pants_test/rules/test_test.py index 5ae5a309b93..a9034ca3293 100644 --- a/tests/python/pants_test/rules/test_test.py +++ b/tests/python/pants_test/rules/test_test.py @@ -7,7 +7,10 @@ from pants.build_graph.address import Address, BuildFileAddress from pants.engine.legacy.graph import HydratedTarget from pants.engine.legacy.structs import PythonTestsAdaptor -from pants.rules.core.test import Status, TestResult, coordinator_of_tests, fast_test +from pants.engine.rules import UnionMembership +from pants.rules.core.core_test_model import TestTarget +from pants.rules.core.test import Status, TestResult, coordinator_of_tests, fast_test, \ + AddressAndTestResult from pants_test.engine.util import MockConsole, run_rule from pants_test.test_base import TestBase @@ -16,14 +19,16 @@ class TestTest(TestBase): def single_target_test(self, result, expected_console_output, success=True): console = MockConsole(use_colors=False) - res = run_rule(fast_test, console, (self.make_build_target_address("some/target"),), { - (TestResult, Address): lambda _: result, + addr = self.make_build_target_address("some/target") + res = run_rule(fast_test, console, (addr,), { + (AddressAndTestResult, Address): lambda _: AddressAndTestResult(addr, result), }) self.assertEquals(console.stdout.getvalue(), expected_console_output) self.assertEquals(0 if success else 1, res.exit_code) - def make_build_target_address(self, spec): + @staticmethod + def make_build_target_address(spec): address = Address.parse(spec) return BuildFileAddress( build_file=None, @@ -61,14 +66,15 @@ def test_output_mixed(self): def make_result(target): if target == target1: - return TestResult(status=Status.SUCCESS, stdout='I passed\n', stderr='') + tr = TestResult(status=Status.SUCCESS, stdout='I passed\n', stderr='') elif target == target2: - return TestResult(status=Status.FAILURE, stdout='I failed\n', stderr='') + tr = TestResult(status=Status.FAILURE, stdout='I failed\n', stderr='') else: raise Exception("Unrecognised target") + return AddressAndTestResult(target, tr) res = run_rule(fast_test, console, (target1, target2), { - (TestResult, Address): make_result, + (AddressAndTestResult, Address): make_result, }) self.assertEqual(1, res.exit_code) @@ -97,10 +103,19 @@ def test_stderr(self): ) def test_coordinator_python_test(self): + addr = Address.parse("some/target") target_adaptor = PythonTestsAdaptor(type_alias='python_tests') with self.captured_logging(logging.INFO): - result = run_rule(coordinator_of_tests, HydratedTarget(Address.parse("some/target"), target_adaptor, ()), { - (TestResult, PythonTestsAdaptor): lambda _: TestResult(status=Status.FAILURE, stdout='foo', stderr=''), - }) - - self.assertEqual(result, TestResult(status=Status.FAILURE, stdout='foo', stderr='')) + result = run_rule( + coordinator_of_tests, + HydratedTarget(addr, target_adaptor, ()), + UnionMembership(union_rules={TestTarget: [PythonTestsAdaptor]}), + { + (TestResult, PythonTestsAdaptor): + lambda _: TestResult(status=Status.FAILURE, stdout='foo', stderr=''), + }) + + self.assertEqual( + result, + AddressAndTestResult(addr, TestResult(status=Status.FAILURE, stdout='foo', stderr='')) + ) From 8b181b8e32167467fe11af6ed3d81489e554af15 Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Sat, 5 Oct 2019 11:16:29 -0700 Subject: [PATCH 10/11] Dataclass frozen fix. --- src/python/pants/rules/core/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/rules/core/test.py b/src/python/pants/rules/core/test.py index 4d7ed7bebcd..7f4c950beae 100644 --- a/src/python/pants/rules/core/test.py +++ b/src/python/pants/rules/core/test.py @@ -26,7 +26,7 @@ class Test(Goal): name = 'test' -@dataclass +@dataclass(frozen=True) class AddressAndTestResult: address: BuildFileAddress test_result: Optional[TestResult] # If None, target was not a test target. From 07281c0f6bc6b0088c131d50a5a363f19ca724bc Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Sat, 5 Oct 2019 15:18:12 -0700 Subject: [PATCH 11/11] Lint. --- tests/python/pants_test/rules/test_test.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/python/pants_test/rules/test_test.py b/tests/python/pants_test/rules/test_test.py index a9034ca3293..c51635c26b3 100644 --- a/tests/python/pants_test/rules/test_test.py +++ b/tests/python/pants_test/rules/test_test.py @@ -9,8 +9,13 @@ from pants.engine.legacy.structs import PythonTestsAdaptor from pants.engine.rules import UnionMembership from pants.rules.core.core_test_model import TestTarget -from pants.rules.core.test import Status, TestResult, coordinator_of_tests, fast_test, \ - AddressAndTestResult +from pants.rules.core.test import ( + AddressAndTestResult, + Status, + TestResult, + coordinator_of_tests, + fast_test, +) from pants_test.engine.util import MockConsole, run_rule from pants_test.test_base import TestBase