Skip to content

Commit

Permalink
Add tests to lint-v2 and fmt-v2 (pantsbuild#8668)
Browse files Browse the repository at this point in the history
### Problem

We weren't testing `lint`, which has an edge case when one of the targets results in a lint failure. `fmt` had an integration test which can be rewritten to a less costly unit test.

### Result

We test more edge cases.

It should be easier to write future tests for these files and to write `run_rule()`-based tests for other rules, as the more examples we have of how to test rules, the easier it will become.
  • Loading branch information
Eric-Arellano authored Nov 21, 2019
1 parent 2424580 commit 8521c1b
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 33 deletions.
2 changes: 1 addition & 1 deletion src/python/pants/rules/core/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ python_tests(
'src/python/pants/backend/jvm/targets:all',
'src/python/pants/backend/python/targets',
'src/python/pants/build_graph',
'src/python/pants/engine:fs',
'src/python/pants/engine/legacy:graph',
'src/python/pants/engine/legacy:structs',
'src/python/pants/testutil:test_base',
Expand All @@ -43,7 +44,6 @@ python_tests(
'examples/src/java/org/pantsbuild/example:hello_directory',
'examples/src/scala/org/pantsbuild/example:hello_directory',
'examples/src/resources/org/pantsbuild/example:hello_directory',
'testprojects/tests/java/org/pantsbuild/testproject:dummies_directory',
],
tags = {'integration'},
)
16 changes: 8 additions & 8 deletions src/python/pants/rules/core/fmt.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ class Fmt(Goal):
@console_rule
def fmt(console: Console, targets: HydratedTargets, union_membership: UnionMembership) -> Fmt:
results = yield [
Get(FmtResult, TargetWithSources, target.adaptor)
for target in targets
# TODO: make TargetAdaptor return a 'sources' field with an empty snapshot instead of
# raising to remove the hasattr() checks here!
if union_membership.is_member(TargetWithSources, target.adaptor) and hasattr(target.adaptor, "sources")
]
Get(FmtResult, TargetWithSources, target.adaptor)
for target in targets
# TODO: make TargetAdaptor return a 'sources' field with an empty snapshot instead of
# raising to remove the hasattr() checks here!
if union_membership.is_member(TargetWithSources, target.adaptor) and hasattr(target.adaptor, "sources")
]

for result in results:
files_content = yield Get(FilesContent, Digest, result.digest)
Expand All @@ -64,5 +64,5 @@ def fmt(console: Console, targets: HydratedTargets, union_membership: UnionMembe

def rules():
return [
fmt,
]
fmt,
]
16 changes: 0 additions & 16 deletions src/python/pants/rules/core/fmt_integration_test.py

This file was deleted.

53 changes: 53 additions & 0 deletions src/python/pants/rules/core/fmt_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from typing import Callable, List, Tuple, Type

from pants.engine.fs import EMPTY_DIRECTORY_DIGEST, Digest, FilesContent
from pants.engine.legacy.graph import HydratedTarget, HydratedTargets
from pants.engine.legacy.structs import JvmAppAdaptor, PythonTargetAdaptor, TargetAdaptor
from pants.engine.rules import UnionMembership
from pants.rules.core.fmt import Fmt, FmtResult, TargetWithSources, fmt
from pants.testutil.engine.util import MockConsole, MockGet, run_rule


def make_target(
*, name: str = "target", adaptor_type: Type[TargetAdaptor] = PythonTargetAdaptor
) -> HydratedTarget:
return HydratedTarget(
address=f"src/{name}",
adaptor=adaptor_type(sources=(), name=name),
dependencies=()
)


def run_fmt_rule(
*,
targets: List[HydratedTarget],
mock_formatter: Callable[[PythonTargetAdaptor], FmtResult],
) -> Tuple[Fmt, MockConsole]:
console = MockConsole(use_colors=False)
result: Fmt = run_rule(
fmt,
rule_args=[
console,
HydratedTargets(targets),
UnionMembership(union_rules={TargetWithSources: [PythonTargetAdaptor]})
],
mock_gets=[
MockGet(product_type=FmtResult, subject_type=PythonTargetAdaptor, mock=mock_formatter),
MockGet(product_type=FilesContent, subject_type=Digest, mock=lambda _: FilesContent([]))
],
)
return result, console


def test_non_union_member_noops() -> None:
result, console = run_fmt_rule(
targets=[make_target(adaptor_type=JvmAppAdaptor)],
mock_formatter=lambda adaptor: FmtResult(
digest=EMPTY_DIRECTORY_DIGEST, stdout=f"Formatted target `{adaptor.name}`", stderr=""
),
)
assert result.exit_code == 0
assert console.stdout.getvalue().strip() == ""
16 changes: 8 additions & 8 deletions src/python/pants/rules/core/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ class Lint(Goal):
@console_rule
def lint(console: Console, targets: HydratedTargets, union_membership: UnionMembership) -> Lint:
results = yield [
Get(LintResult, TargetWithSources, target.adaptor)
for target in targets
# TODO: make TargetAdaptor return a 'sources' field with an empty snapshot instead of
# raising to remove the hasattr() checks here!
if union_membership.is_member(TargetWithSources, target.adaptor) and hasattr(target.adaptor, "sources")
]
Get(LintResult, TargetWithSources, target.adaptor)
for target in targets
# TODO: make TargetAdaptor return a 'sources' field with an empty snapshot instead of
# raising to remove the hasattr() checks here!
if union_membership.is_member(TargetWithSources, target.adaptor) and hasattr(target.adaptor, "sources")
]

exit_code = 0
for result in results:
Expand All @@ -50,5 +50,5 @@ def lint(console: Console, targets: HydratedTargets, union_membership: UnionMemb

def rules():
return [
lint,
]
lint,
]
63 changes: 63 additions & 0 deletions src/python/pants/rules/core/lint_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from typing import Callable, List, Tuple, Type

from pants.engine.legacy.graph import HydratedTarget, HydratedTargets
from pants.engine.legacy.structs import JvmAppAdaptor, PythonTargetAdaptor, TargetAdaptor
from pants.engine.rules import UnionMembership
from pants.rules.core.lint import Lint, LintResult, TargetWithSources, lint
from pants.testutil.engine.util import MockConsole, MockGet, run_rule


def make_target(
*, name: str = "target", adaptor_type: Type[TargetAdaptor] = PythonTargetAdaptor
) -> HydratedTarget:
return HydratedTarget(
address=f"src/{name}",
adaptor=adaptor_type(sources=(), name=name),
dependencies=()
)


def run_lint_rule(
*,
targets: List[HydratedTarget],
mock_linter: Callable[[PythonTargetAdaptor], LintResult],
) -> Tuple[Lint, MockConsole]:
console = MockConsole(use_colors=False)
result: Lint = run_rule(
lint,
rule_args=[
console,
HydratedTargets(targets),
UnionMembership(union_rules={TargetWithSources: [PythonTargetAdaptor]})
],
mock_gets=[
MockGet(product_type=LintResult, subject_type=PythonTargetAdaptor, mock=mock_linter),
],
)
return result, console


def test_non_union_member_noops() -> None:
result, console = run_lint_rule(
targets=[make_target(adaptor_type=JvmAppAdaptor)],
mock_linter=lambda target: LintResult(exit_code=1, stdout="", stderr=""),
)
assert result.exit_code == 0
assert console.stdout.getvalue().strip() == ""


def test_failure_for_a_single_target_propagates():
def mock_linter(adaptor: PythonTargetAdaptor) -> LintResult:
if adaptor.name == "bad":
return LintResult(exit_code=127, stdout="failure", stderr="failure")
return LintResult(exit_code=0, stdout=f"Linted the target `{adaptor.name}`", stderr="..")

result, console = run_lint_rule(
targets=[make_target(name="good"), make_target(name="bad")], mock_linter=mock_linter,
)
assert result.exit_code == 127
assert console.stdout.getvalue().splitlines() == ["Linted the target `good`", "failure"]
assert console.stderr.getvalue().splitlines() == ["..", "failure"]

0 comments on commit 8521c1b

Please sign in to comment.