Skip to content

Commit

Permalink
Restore debug hints for test execution (pantsbuild#17406)
Browse files Browse the repository at this point in the history
Closes pantsbuild#17386

91d68f1 broke streaming output for `test`. This commit should fix the problem by having the new partitioning API's types subclass `EngineAwareParameter`.

For now I've only implemented `debug_hint` for the `TestRequest.Batch` type, because it's the direct replacement for the old `TestFieldSet`-driven API.
  • Loading branch information
danxmoran authored Oct 31, 2022
1 parent d05c437 commit 6c64e9d
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 12 deletions.
10 changes: 5 additions & 5 deletions src/python/pants/base/specs_integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ def test_address_literal() -> None:
assert run(["list", *list_specs]).stdout.splitlines() == list_specs

test_result = run(["test", f"{tmpdir}/py:tests"]).stderr
assert f"{tmpdir}/py/utils/strutil_test.py:../tests succeeded" in test_result
assert f"{tmpdir}/py/base/common_test.py:../tests succeeded" in test_result
assert f"{tmpdir}/py/utils/strutil_test.py:../tests - succeeded." in test_result
assert f"{tmpdir}/py/base/common_test.py:../tests - succeeded." in test_result
assert f"{tmpdir}/py:tests" not in test_result


Expand Down Expand Up @@ -109,14 +109,14 @@ def test_sibling_addresses() -> None:
# Even though no `python_test` targets are explicitly defined in `util/`, a generated
# target is resident there.
test_result = run(["test", f"{tmpdir}/py/utils:"]).stderr
assert f"{tmpdir}/py/utils/strutil_test.py:../tests succeeded" in test_result
assert f"{tmpdir}/py/utils/strutil_test.py:../tests - succeeded." in test_result
assert f"{tmpdir}/py/base/common_test.py:../tests" not in test_result
assert f"{tmpdir}/py:tests" not in test_result

# Even though no `_test.py` files live in this dir, we match the `python_tests` target
# and replace it with its generated targets.
test_result = run(["test", f"{tmpdir}/py:"]).stderr
assert f"{tmpdir}/py/utils/strutil_test.py:../tests succeeded" in test_result
assert f"{tmpdir}/py/utils/strutil_test.py:../tests - succeeded." in test_result
assert f"{tmpdir}/py/base/common_test.py:../tests" in test_result
assert f"{tmpdir}/py:tests" not in test_result

Expand All @@ -137,7 +137,7 @@ def test_descendent_addresses() -> None:
]

test_result = run(["test", f"{tmpdir}/py::"]).stderr
assert f"{tmpdir}/py/utils/strutil_test.py:../tests succeeded" in test_result
assert f"{tmpdir}/py/utils/strutil_test.py:../tests - succeeded." in test_result
assert f"{tmpdir}/py/base/common_test.py:../tests" in test_result
assert f"{tmpdir}/py:tests" not in test_result

Expand Down
20 changes: 16 additions & 4 deletions src/python/pants/core/goals/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
EnvironmentNameRequest,
)
from pants.core.util_rules.partitions import (
PartitionElementT,
PartitionerType,
PartitionMetadataT,
Partitions,
Expand Down Expand Up @@ -311,12 +310,13 @@ def tool_name(cls) -> str:

@distinct_union_type_per_subclass(in_scope_types=[EnvironmentName])
class PartitionRequest(_PartitionFieldSetsRequestBase[_TestFieldSetT]):
pass
def metadata(self) -> dict[str, Any]:
return {"addresses": [field_set.address.spec for field_set in self.field_sets]}

@distinct_union_type_per_subclass(in_scope_types=[EnvironmentName])
class Batch(_BatchBase[PartitionElementT, PartitionMetadataT]):
class Batch(_BatchBase[_TestFieldSetT, PartitionMetadataT]):
@property
def single_element(self) -> PartitionElementT:
def single_element(self) -> _TestFieldSetT:
"""Return the single element of this batch.
NOTE: Accessing this property will raise a `TypeError` if this `Batch` contains
Expand All @@ -334,6 +334,18 @@ def single_element(self) -> PartitionElementT:

return self.elements[0]

def debug_hint(self) -> str:
if len(self.elements) == 1:
return self.elements[0].address.spec

return f"{self.elements[0].address.spec} and {len(self.elements)-1} other files"

def metadata(self) -> dict[str, Any]:
return {
"addresses": [field_set.address.spec for field_set in self.elements],
"partition_description": self.partition_metadata.description,
}

@classmethod
def rules(cls) -> Iterable:
yield from cls.partitioner_type.default_rules(cls, by_file=False)
Expand Down
7 changes: 4 additions & 3 deletions src/python/pants/core/util_rules/partitions.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

from pants.core.goals.multi_tool_goal_helper import SkippableSubsystem
from pants.engine.collection import Collection
from pants.engine.engine_aware import EngineAwareParameter
from pants.engine.internals.selectors import Get, MultiGet
from pants.engine.rules import collect_rules, rule
from pants.engine.target import (
Expand Down Expand Up @@ -154,7 +155,7 @@ def single_partition(
@frozen_after_init
@dataclass(unsafe_hash=True)
@runtime_ignore_subscripts
class _BatchBase(Generic[PartitionElementT, PartitionMetadataT]):
class _BatchBase(Generic[PartitionElementT, PartitionMetadataT], EngineAwareParameter):
"""Base class for a collection of elements that should all be processed together.
For example, a collection of strings pointing to files that should be linted in one process, or
Expand All @@ -168,7 +169,7 @@ class _BatchBase(Generic[PartitionElementT, PartitionMetadataT]):

@dataclass(frozen=True)
@runtime_ignore_subscripts
class _PartitionFieldSetsRequestBase(Generic[_FieldSetT]):
class _PartitionFieldSetsRequestBase(Generic[_FieldSetT], EngineAwareParameter):
"""Returns a unique type per calling type.
This serves us 2 purposes:
Expand All @@ -180,7 +181,7 @@ class _PartitionFieldSetsRequestBase(Generic[_FieldSetT]):


@dataclass(frozen=True)
class _PartitionFilesRequestBase:
class _PartitionFilesRequestBase(EngineAwareParameter):
"""Returns a unique type per calling type.
This serves us 2 purposes:
Expand Down

0 comments on commit 6c64e9d

Please sign in to comment.