Skip to content
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

Stream output of typecheck, rather than dumping at the end #10656

Merged
merged 2 commits into from
Aug 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/python/pants/backend/python/typecheck/mypy/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@
)
from pants.engine.process import FallibleProcessResult
from pants.engine.rules import Get, MultiGet, collect_rules, rule
from pants.engine.target import FieldSetWithOrigin, TransitiveTargets
from pants.engine.target import FieldSet, TransitiveTargets
from pants.engine.unions import UnionRule
from pants.util.logging import LogLevel
from pants.util.strutil import pluralize


@dataclass(frozen=True)
class MyPyFieldSet(FieldSetWithOrigin):
class MyPyFieldSet(FieldSet):
required_fields = (PythonSources,)

sources: PythonSources
Expand All @@ -60,7 +60,7 @@ def generate_args(mypy: MyPy, *, file_list_path: str) -> Tuple[str, ...]:
@rule(desc="Typecheck using MyPy", level=LogLevel.DEBUG)
async def mypy_typecheck(request: MyPyRequest, mypy: MyPy) -> TypecheckResults:
if mypy.skip:
return TypecheckResults()
return TypecheckResults([], typechecker_name="MyPy")

transitive_targets = await Get(
TransitiveTargets, Addresses(fs.address for fs in request.field_sets)
Expand Down Expand Up @@ -115,10 +115,11 @@ async def mypy_typecheck(request: MyPyRequest, mypy: MyPy) -> TypecheckResults:
input_digest=merged_input_files,
extra_env={"PEX_EXTRA_SYS_PATH": ":".join(prepared_sources.source_roots)},
description=f"Run MyPy on {pluralize(len(srcs_snapshot.files), 'file')}.",
level=LogLevel.DEBUG,
),
)
return TypecheckResults(
[TypecheckResult.from_fallible_process_result(result, typechecker_name="MyPy")]
[TypecheckResult.from_fallible_process_result(result)], typechecker_name="MyPy"
)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,17 @@

from pathlib import PurePath
from textwrap import dedent
from typing import List, Optional
from typing import List, Optional, Sequence

from pants.backend.python.dependency_inference import rules as dependency_inference_rules
from pants.backend.python.target_types import PythonLibrary
from pants.backend.python.typecheck.mypy.rules import MyPyFieldSet, MyPyRequest
from pants.backend.python.typecheck.mypy.rules import rules as mypy_rules
from pants.base.specs import AddressLiteralSpec
from pants.core.goals.typecheck import TypecheckResults
from pants.core.goals.typecheck import TypecheckResult, TypecheckResults
from pants.engine.addresses import Address
from pants.engine.fs import FileContent
from pants.engine.rules import RootRule
from pants.engine.target import TargetWithOrigin, WrappedTarget
from pants.engine.target import Target, WrappedTarget
from pants.testutil.engine.util import Params
from pants.testutil.external_tool_test_base import ExternalToolTestBase
from pants.testutil.option.util import create_options_bootstrapper
Expand Down Expand Up @@ -77,13 +76,13 @@ def rules(cls):
def target_types(cls):
return [PythonLibrary]

def make_target_with_origin(
def make_target(
self,
source_files: List[FileContent],
*,
package: Optional[str] = None,
name: str = "target",
) -> TargetWithOrigin:
) -> Target:
if not package:
package = self.package
for source_file in source_files:
Expand All @@ -100,25 +99,23 @@ def make_target_with_origin(
"""
),
)
target = self.request_single_product(
return self.request_single_product(
WrappedTarget,
Params(
Address(package, target_name=name),
create_options_bootstrapper(args=self.global_args),
),
).target
origin = AddressLiteralSpec(package, name)
return TargetWithOrigin(target, origin)

def run_mypy(
self,
targets: List[TargetWithOrigin],
targets: List[Target],
*,
config: Optional[str] = None,
passthrough_args: Optional[str] = None,
skip: bool = False,
additional_args: Optional[List[str]] = None,
) -> TypecheckResults:
) -> Sequence[TypecheckResult]:
args = list(self.global_args)
if config:
self.create_file(relpath="mypy.ini", contents=config)
Expand All @@ -129,30 +126,31 @@ def run_mypy(
args.append("--mypy-skip")
if additional_args:
args.extend(additional_args)
return self.request_single_product(
result = self.request_single_product(
TypecheckResults,
Params(
MyPyRequest(MyPyFieldSet.create(tgt) for tgt in targets),
create_options_bootstrapper(args=args),
),
)
return result.results

def test_passing_source(self) -> None:
target = self.make_target_with_origin([self.good_source])
target = self.make_target([self.good_source])
result = self.run_mypy([target])
assert len(result) == 1
assert result[0].exit_code == 0
assert "Success: no issues found" in result[0].stdout.strip()

def test_failing_source(self) -> None:
target = self.make_target_with_origin([self.bad_source])
target = self.make_target([self.bad_source])
result = self.run_mypy([target])
assert len(result) == 1
assert result[0].exit_code == 1
assert f"{self.package}/bad.py:4" in result[0].stdout

def test_mixed_sources(self) -> None:
target = self.make_target_with_origin([self.good_source, self.bad_source])
target = self.make_target([self.good_source, self.bad_source])
result = self.run_mypy([target])
assert len(result) == 1
assert result[0].exit_code == 1
Expand All @@ -162,8 +160,8 @@ def test_mixed_sources(self) -> None:

def test_multiple_targets(self) -> None:
targets = [
self.make_target_with_origin([self.good_source], name="t1"),
self.make_target_with_origin([self.bad_source], name="t2"),
self.make_target([self.good_source], name="t1"),
self.make_target([self.bad_source], name="t2"),
]
result = self.run_mypy(targets)
assert len(result) == 1
Expand All @@ -173,21 +171,21 @@ def test_multiple_targets(self) -> None:
assert "checked 2 source files" in result[0].stdout

def test_respects_config_file(self) -> None:
target = self.make_target_with_origin([self.needs_config_source])
target = self.make_target([self.needs_config_source])
result = self.run_mypy([target], config="[mypy]\ndisallow_any_expr = True\n")
assert len(result) == 1
assert result[0].exit_code == 1
assert f"{self.package}/needs_config.py:4" in result[0].stdout

def test_respects_passthrough_args(self) -> None:
target = self.make_target_with_origin([self.needs_config_source])
target = self.make_target([self.needs_config_source])
result = self.run_mypy([target], passthrough_args="--disallow-any-expr")
assert len(result) == 1
assert result[0].exit_code == 1
assert f"{self.package}/needs_config.py:4" in result[0].stdout

def test_skip(self) -> None:
target = self.make_target_with_origin([self.bad_source])
target = self.make_target([self.bad_source])
result = self.run_mypy([target], skip=True)
assert not result

Expand Down Expand Up @@ -234,7 +232,7 @@ def add(x: int, y: int) -> str:
),
FileContent(f"{self.package}/__init__.py", b""),
]
target = self.make_target_with_origin(sources_content)
target = self.make_target(sources_content)
result = self.run_mypy([target])
assert len(result) == 1
assert result[0].exit_code == 1
Expand Down
113 changes: 80 additions & 33 deletions src/python/pants/core/goals/typecheck.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,24 @@
# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

import itertools
from dataclasses import dataclass
from typing import Iterable
from typing import Iterable, Optional, Tuple

from pants.core.goals.style_request import StyleRequest
from pants.core.util_rules.filter_empty_sources import (
FieldSetsWithSources,
FieldSetsWithSourcesRequest,
)
from pants.engine.collection import Collection
from pants.engine.console import Console
from pants.engine.engine_aware import EngineAware
from pants.engine.goal import Goal, GoalSubsystem
from pants.engine.process import FallibleProcessResult
from pants.engine.rules import Get, MultiGet, collect_rules, goal_rule
from pants.engine.target import TargetsWithOrigins
from pants.engine.target import Targets
from pants.engine.unions import UnionMembership, union
from pants.util.logging import LogLevel
from pants.util.memo import memoized_property
from pants.util.meta import frozen_after_init
from pants.util.strutil import strip_v2_chroot_path


Expand All @@ -25,13 +27,13 @@ class TypecheckResult:
exit_code: int
stdout: str
stderr: str
typechecker_name: str
partition_description: Optional[str] = None

@staticmethod
def from_fallible_process_result(
process_result: FallibleProcessResult,
*,
typechecker_name: str,
partition_description: Optional[str] = None,
strip_chroot_path: bool = False,
) -> "TypecheckResult":
def prep_output(s: bytes) -> str:
Expand All @@ -41,18 +43,69 @@ def prep_output(s: bytes) -> str:
exit_code=process_result.exit_code,
stdout=prep_output(process_result.stdout),
stderr=prep_output(process_result.stderr),
typechecker_name=typechecker_name,
partition_description=partition_description,
)


class TypecheckResults(Collection[TypecheckResult]):
@frozen_after_init
@dataclass(unsafe_hash=True)
class TypecheckResults(EngineAware):
"""Zero or more TypecheckResult objects for a single type checker.

Typically, type checkers will return one result. If they no-oped, they will return zero results.
However, some type checkers may need to partition their input and thus may need to return
multiple results.
"""

results: Tuple[TypecheckResult, ...]
typechecker_name: str

def __init__(self, results: Iterable[TypecheckResult], *, typechecker_name: str) -> None:
self.results = tuple(results)
self.typechecker_name = typechecker_name

@property
def skipped(self) -> bool:
return bool(self.results) is False

@memoized_property
def exit_code(self) -> int:
return next((result.exit_code for result in self.results if result.exit_code != 0), 0)

def level(self) -> Optional[LogLevel]:
if self.skipped:
return LogLevel.DEBUG
return LogLevel.WARN if self.exit_code != 0 else LogLevel.INFO
Eric-Arellano marked this conversation as resolved.
Show resolved Hide resolved

def message(self) -> Optional[str]:
if self.skipped:
return "skipped."
message = "succeeded." if self.exit_code == 0 else f"failed (exit code {self.exit_code})."

def msg_for_result(result: TypecheckResult) -> str:
msg = ""
if result.stdout:
msg += f"\n{result.stdout}"
if result.stderr:
msg += f"\n{result.stderr}"
if msg:
msg = f"{msg.rstrip()}\n\n"
return msg

if len(self.results) == 1:
results_msg = msg_for_result(self.results[0])
else:
results_msg = "\n"
for i, result in enumerate(self.results):
msg = f"Partition #{i + 1}"
msg += (
f" - {result.partition_description}:" if result.partition_description else ":"
)
msg += msg_for_result(result) or "\n\n"
results_msg += msg
message += results_msg
return message


@union
class TypecheckRequest(StyleRequest):
Expand All @@ -76,14 +129,14 @@ class Typecheck(Goal):

@goal_rule
async def typecheck(
console: Console, targets_with_origins: TargetsWithOrigins, union_membership: UnionMembership
console: Console, targets: Targets, union_membership: UnionMembership
) -> Typecheck:
typecheck_request_types = union_membership[TypecheckRequest]
requests: Iterable[StyleRequest] = tuple(
lint_request_type(
lint_request_type.field_set_type.create(target_with_origin)
for target_with_origin in targets_with_origins
if lint_request_type.field_set_type.is_valid(target_with_origin.target)
lint_request_type.field_set_type.create(target)
for target in targets
if lint_request_type.field_set_type.is_valid(target)
)
for lint_request_type in typecheck_request_types
)
Expand All @@ -96,31 +149,25 @@ async def typecheck(
for request_cls, request in zip(typecheck_request_types, field_sets_with_sources)
if request
)
results = await MultiGet(
all_results = await MultiGet(
Get(TypecheckResults, TypecheckRequest, request) for request in valid_requests
)

sorted_results = sorted(
itertools.chain.from_iterable(results), key=lambda res: res.typechecker_name
)
if not sorted_results:
return Typecheck(exit_code=0)

exit_code = 0
for result in sorted_results:
console.print_stderr(
f"{console.green('✓')} {result.typechecker_name} succeeded."
if result.exit_code == 0
else f"{console.red('𐄂')} {result.typechecker_name} failed."
)
if result.stdout:
console.print_stderr(result.stdout)
if result.stderr:
console.print_stderr(result.stderr)
if result != sorted_results[-1]:
console.print_stderr("")
if result.exit_code != 0:
exit_code = result.exit_code
if all_results:
console.print_stderr("")
for results in sorted(all_results, key=lambda results: results.typechecker_name):
if results.skipped:
sigil = console.yellow("-")
status = "skipped"
elif results.exit_code == 0:
sigil = console.green("✓")
status = "succeeded"
else:
sigil = console.red("𐄂")
status = "failed"
exit_code = results.exit_code
console.print_stderr(f"{sigil} {results.typechecker_name} {status}.")

return Typecheck(exit_code)

Expand Down
Loading