Skip to content

Commit

Permalink
[internal] Python coverage report generation uses precomputed address…
Browse files Browse the repository at this point in the history
…es. (#12965)

`generate_coverage_reports` requested `Addresses` as an argument, and in common usage, these were computed from the `Specs`.

But that will represent an interface leak under #12934 (the only one in the pantsbuild/pants codebase, apparently): it is not expected that the `await Get(CoverageReports, CoverageDataCollection, ..)` union usage would consume the `Specs`; rather: it should operate for its inputs, which might not precisely match the `Specs`.

To fix this, consume the `Addresses` associated with the coverage data instead.

[ci skip-build-wheels]
[ci skip-rust]
  • Loading branch information
stuhood authored Sep 21, 2021
1 parent 1dbb84f commit 3120ccd
Showing 1 changed file with 11 additions and 5 deletions.
16 changes: 11 additions & 5 deletions src/python/pants/backend/python/goals/coverage_py.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
FilesystemCoverageReport,
)
from pants.core.util_rules.config_files import ConfigFiles, ConfigFilesRequest
from pants.engine.addresses import Address, Addresses
from pants.engine.addresses import Address
from pants.engine.fs import (
EMPTY_DIGEST,
AddPrefix,
Expand Down Expand Up @@ -364,6 +364,7 @@ async def setup_coverage(coverage: CoverageSubsystem) -> CoverageSetup:
@dataclass(frozen=True)
class MergedCoverageData:
coverage_data: Digest
addresses: tuple[Address, ...]


@rule(desc="Merge Pytest coverage data", level=LogLevel.DEBUG)
Expand All @@ -374,16 +375,19 @@ async def merge_coverage_data(
source_roots: AllSourceRoots,
) -> MergedCoverageData:
if len(data_collection) == 1 and not coverage.global_report:
return MergedCoverageData(data_collection[0].digest)
coverage_data = data_collection[0]
return MergedCoverageData(coverage_data.digest, (coverage_data.address,))

coverage_digest_gets = []
coverage_data_file_paths = []
addresses = []
for data in data_collection:
# We prefix each .coverage file with its corresponding address to avoid collisions.
coverage_digest_gets.append(
Get(Digest, AddPrefix(data.digest, prefix=data.address.path_safe_spec))
)
coverage_data_file_paths.append(f"{data.address.path_safe_spec}/.coverage")
addresses.append(data.address)

if coverage.global_report:
global_coverage_base_dir = PurePath("__global_coverage__")
Expand Down Expand Up @@ -461,7 +465,8 @@ async def merge_coverage_data(
),
)
return MergedCoverageData(
await Get(Digest, MergeDigests((result.output_digest, extra_sources_digest)))
await Get(Digest, MergeDigests((result.output_digest, extra_sources_digest))),
tuple(addresses),
)


Expand All @@ -471,10 +476,11 @@ async def generate_coverage_reports(
coverage_setup: CoverageSetup,
coverage_config: CoverageConfig,
coverage_subsystem: CoverageSubsystem,
all_used_addresses: Addresses,
) -> CoverageReports:
"""Takes all Python test results and generates a single coverage report."""
transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest(all_used_addresses))
transitive_targets = await Get(
TransitiveTargets, TransitiveTargetsRequest(merged_coverage_data.addresses)
)
sources = await Get(
PythonSourceFiles,
# Coverage sometimes includes non-Python files in its `.coverage` data. We need to
Expand Down

0 comments on commit 3120ccd

Please sign in to comment.