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

Add support for coverage config file (.coveragerc) #10289

Merged
merged 4 commits into from Jul 9, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
68 changes: 54 additions & 14 deletions src/python/pants/backend/python/rules/coverage.py
Expand Up @@ -5,7 +5,6 @@
from dataclasses import dataclass
from io import StringIO
from pathlib import PurePath
from textwrap import dedent
from typing import List, Optional, Sequence, Tuple

from pants.backend.python.rules.pex import (
Expand All @@ -30,12 +29,23 @@
FilesystemCoverageReport,
)
from pants.engine.addresses import Address
from pants.engine.fs import AddPrefix, CreateDigest, Digest, FileContent, MergeDigests
from pants.engine.fs import (
AddPrefix,
CreateDigest,
Digest,
DigestContents,
FileContent,
MergeDigests,
PathGlobs,
Snapshot,
)
from pants.engine.process import Process, ProcessResult
from pants.engine.rules import SubsystemRule, rule
from pants.engine.selectors import Get, MultiGet
from pants.engine.target import TransitiveTargets
from pants.engine.unions import UnionRule
from pants.option.custom_types import file_option
from pants.option.global_options import GlobMatchErrorBehavior
from pants.python.python_setup import PythonSetup


Expand Down Expand Up @@ -98,6 +108,13 @@ def register_options(cls, register):
advanced=True,
help="Path to write the Pytest Coverage report to. Must be relative to build root.",
)
register(
"--config",
type=file_option,
default=None,
advanced=True,
help="Path to `.coveragerc` or alternative coverage config file",
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"alternative coverage config file"

I'm not sure what this means... does it mean something like "or other file in the .coveragerc format"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That wording comes from the other subsystems. It's meant to say that it could for example be pyproject.toml. See all the options they have at https://coverage.readthedocs.io/en/coverage-5.2/config.html.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

)

@property
def filter(self) -> Tuple[str, ...]:
Expand Down Expand Up @@ -127,20 +144,43 @@ class CoverageConfig:
digest: Digest


def _validate_and_update_config(
coverage_config: configparser.ConfigParser, config_path: Optional[str]
) -> None:
if not coverage_config.has_section("run"):
coverage_config.add_section("run")
coverage_config.set("run", "branch", "True")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we still want this turned on by us? I personally don't like when Pants makes decisions like this - who are we to say that you want branch data?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am just trying to preserve existing behavior....

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged. To clarify, I'm proposing breaking with existing behavior.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this kind of change should be part of its own PR.... especially since this change breaks existing integration tests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run_section = coverage_config["run"]
relative_files_str = run_section.get("relative_files", "True")
if relative_files_str.lower() != "true":
raise ValueError(
f"relative_files under the 'run' section must be set to True. config file: {config_path}"
)
coverage_config.set("run", "relative_files", "True")
omit_elements = [em for em in run_section.get("omit", "").split("\n")] or ["\n"]
if "test_runner.pex/*" not in omit_elements:
omit_elements.append("test_runner.pex/*")
asherf marked this conversation as resolved.
Show resolved Hide resolved
run_section["omit"] = "\n".join(omit_elements)


@rule
async def create_coverage_config() -> CoverageConfig:
default_config = dedent(
"""
[run]
branch = True
relative_files = True
"""
)
cp = configparser.ConfigParser()
cp.read_string(default_config)
cp.set("run", "omit", "test_runner.pex/*")
async def create_coverage_config(coverage: CoverageSubsystem) -> CoverageConfig:
config_path: Optional[str] = coverage.options.config
coverage_config = configparser.ConfigParser()
if config_path:
config_snapshot = await Get(
Snapshot,
PathGlobs(
globs=config_path,
glob_match_error_behavior=GlobMatchErrorBehavior.error,
description_of_origin=f"the option `--{coverage.options_scope}-config`",
),
)
config_contents = await Get(DigestContents, Digest, config_snapshot.digest)
coverage_config.read_string(config_contents[0].content.decode())
_validate_and_update_config(coverage_config, config_path)
config_stream = StringIO()
cp.write(config_stream)
coverage_config.write(config_stream)
config_content = config_stream.getvalue()
digest = await Get(Digest, CreateDigest([FileContent(".coveragerc", config_content.encode())]))
return CoverageConfig(digest)
Expand Down
141 changes: 141 additions & 0 deletions src/python/pants/backend/python/rules/coverage_test.py
@@ -0,0 +1,141 @@
# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from textwrap import dedent
from typing import List, Optional

import pytest

from pants.backend.python.rules.coverage import CoverageSubsystem, create_coverage_config
from pants.engine.fs import CreateDigest, Digest, DigestContents, FileContent, PathGlobs, Snapshot
from pants.testutil.engine.util import MockGet, create_subsystem, run_rule
from pants.testutil.test_base import TestBase


class TestCoverageConfig(TestBase):
def run_create_coverage_config_rule(self, coverage_config: Optional[str]) -> str:
coverage = create_subsystem(
CoverageSubsystem, config="some_file" if coverage_config else None
)
resolved_config: List[str] = []

def fake_handle_config(fcs):
assert len(fcs) == 1
assert fcs[0].path == ".coveragerc"
assert fcs[0].is_executable is False
resolved_config.append(fcs[0].content.decode())
return Digest("jerry", 30)

def fake_read_config(digest):
# fake_read_config shouldn't get called if no config file provided
assert coverage_config is not None
return DigestContents(
[FileContent(path="/dev/null/prelude", content=coverage_config.encode())]
)

mock_gets = [
MockGet(
product_type=Snapshot,
subject_type=PathGlobs,
mock=lambda _: Snapshot(Digest("bosco", 30), ("/dev/null/someconfig",), ()),
),
MockGet(product_type=DigestContents, subject_type=Digest, mock=fake_read_config,),
MockGet(product_type=Digest, subject_type=CreateDigest, mock=fake_handle_config),
]

result = run_rule(create_coverage_config, rule_args=[coverage], mock_gets=mock_gets)
assert result.digest.fingerprint == "jerry"
assert len(resolved_config) == 1
return resolved_config[0]

def test_default_no_config(self) -> None:
resolved_config = self.run_create_coverage_config_rule(coverage_config=None)
assert resolved_config == dedent(
"""\
[run]
branch = True
relative_files = True
omit =
\ttest_runner.pex/*

""" # noqa: W291
)

def test_simple_config(self) -> None:
config = dedent(
"""
[run]
branch = False
relative_files = True
jerry = HELLO
"""
)
resolved_config = self.run_create_coverage_config_rule(coverage_config=config)
assert resolved_config == dedent(
"""\
[run]
branch = False
relative_files = True
jerry = HELLO
omit =
\ttest_runner.pex/*

""" # noqa: W291
)

def test_config_no_run_section(self) -> None:
config = dedent(
"""
[report]
ignore_errors = True
"""
)
resolved_config = self.run_create_coverage_config_rule(coverage_config=config)
assert resolved_config == dedent(
"""\
[report]
ignore_errors = True

[run]
branch = True
relative_files = True
omit =
\ttest_runner.pex/*

""" # noqa: W291
)

def test_append_omits(self) -> None:
config = dedent(
"""
[run]
omit =
jerry/seinfeld/*.py
# I find tinsel distracting
festivus/tinsel/*.py
"""
)
resolved_config = self.run_create_coverage_config_rule(coverage_config=config)
assert resolved_config == dedent(
"""\
[run]
omit =
\tjerry/seinfeld/*.py
\tfestivus/tinsel/*.py
\ttest_runner.pex/*
relative_files = True

""" # noqa: W291
)

def test_invalid_relative_files_setting(self) -> None:
config = dedent(
"""
[run]
relative_files = False
"""
)
with pytest.raises(
ValueError, match="relative_files under the 'run' section must be set to True"
):
self.run_create_coverage_config_rule(coverage_config=config)