-
-
Notifications
You must be signed in to change notification settings - Fork 636
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pre-filter TestTarget @union members #8368
Changes from 9 commits
8e33ec1
15c4c3a
6720729
770a36c
b194f32
8e100cd
5b5f6de
ca465ac
c9fe6e9
7ec2092
85d54cc
8f13377
a6507f1
8b181b8
07281c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,12 +4,10 @@ | |
import logging | ||
|
||
from pants.base.exiter import PANTS_FAILED_EXIT_CODE, PANTS_SUCCEEDED_EXIT_CODE | ||
from pants.build_graph.address import 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 | ||
from pants.engine.rules import console_rule, rule | ||
from pants.engine.legacy.graph import HydratedTarget, HydratedTargets | ||
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 | ||
|
||
|
@@ -25,33 +23,39 @@ 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, | ||
targets: HydratedTargets, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, there is a minor optimization here which might not matter, but which might be worth a TODO. The effect of moving The previous formulation would only parse I'd also feel a little bit better about that because of the TODO/NB in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting! I'll take a swipe at this tomorrow. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, did it now. I like it better this way! PTAL. |
||
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] | ||
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: | ||
# NB: we write to stdout, rather than to stderr, to avoid potential issues interleaving the | ||
# 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')) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bad change with
Iterable
. That's now overriding line 11.