From abfa2695e589cf1d7e46ccbf3ac4a1633eec82e5 Mon Sep 17 00:00:00 2001 From: Pradeep Tammali Date: Mon, 15 Jun 2026 10:21:38 +0200 Subject: [PATCH] refactor: improve coverage handler typing and registry Introduce BaseCoverage and generic handlers, consolidate coverage loading into get_coverage, and add tests for compute_contiguous_groups. --- codecov/config.py | 1 - codecov/coverage/base.py | 41 ++++++++++++++++--------- codecov/coverage/jest.py | 6 ++-- codecov/coverage/pytest.py | 7 ++--- codecov/groups.py | 1 - codecov/main.py | 4 +-- codecov/template.py | 4 +-- codecov/template_files/comment.md.j2 | 2 +- tests/coverage/test_base.py | 11 ++++--- tests/test_groups.py | 46 +++++++++++++++++++++++++++- 10 files changed, 89 insertions(+), 34 deletions(-) diff --git a/codecov/config.py b/codecov/config.py index 4e77a16..edc1729 100644 --- a/codecov/config.py +++ b/codecov/config.py @@ -59,7 +59,6 @@ class Config: SKIP_COVERED_FILES_IN_REPORT: bool = True COMPLETE_PROJECT_REPORT: bool = False COVERAGE_REPORT_URL: str | None = None - # TODO: Change this to LOG_LEVEL. INFO; DEBUG; NONE; DEBUG: bool = False def __post_init__(self) -> None: diff --git a/codecov/coverage/base.py b/codecov/coverage/base.py index 47d49a8..c1e7368 100644 --- a/codecov/coverage/base.py +++ b/codecov/coverage/base.py @@ -3,12 +3,16 @@ import json import pathlib from abc import ABC, abstractmethod -from typing import Any, ClassVar +from typing import TYPE_CHECKING, Any, ClassVar, Generic, TypeVar, cast from codecov.config import Config, TestFramework from codecov.exceptions import ConfigurationException from codecov.log import log +if TYPE_CHECKING: + from codecov.coverage.jest import JestCoverage + from codecov.coverage.pytest import PytestCoverage + @dataclasses.dataclass class FileDiffCoverage: @@ -31,13 +35,20 @@ class DiffCoverage: files: dict[pathlib.Path, FileDiffCoverage] -class BaseCoverageHandler(ABC): +class BaseCoverage: + pass + + +T = TypeVar('T', bound=BaseCoverage) + + +class BaseCoverageHandler(ABC, Generic[T]): TEST_FRAMEWORK: TestFramework - REGISTRY: ClassVar[dict[TestFramework, type['BaseCoverageHandler']]] = {} + REGISTRY: ClassVar[dict[TestFramework, type['BaseCoverageHandler[Any]']]] = {} def __init_subclass__(cls) -> None: - cls.REGISTRY[cls.TEST_FRAMEWORK] = cls super().__init_subclass__() + BaseCoverageHandler.REGISTRY[cls.TEST_FRAMEWORK] = cls def convert_to_decimal(self, value: float | decimal.Decimal, precision: int = 2) -> decimal.Decimal: if not isinstance(value, decimal.Decimal): @@ -47,8 +58,8 @@ def convert_to_decimal(self, value: float | decimal.Decimal, precision: int = 2) rounding=decimal.ROUND_DOWN, ) - # TODO: Fix the typing and rename this to get_coverage_json - def get_coverage_info(self, coverage_path: pathlib.Path) -> Any: + def get_coverage(self, config: Config) -> T: + coverage_path = config.COVERAGE_PATH try: with coverage_path.open() as coverage_data: json_coverage = json.loads(coverage_data.read()) @@ -59,7 +70,6 @@ def get_coverage_info(self, coverage_path: pathlib.Path) -> Any: log.error('Invalid JSON format in coverage report file: %s', coverage_path) raise ConfigurationException from exc - # TODO: Move the below code to a separate function try: return self.extract_info(data=json_coverage) except KeyError as exc: @@ -67,25 +77,28 @@ def get_coverage_info(self, coverage_path: pathlib.Path) -> Any: raise ConfigurationException from exc @abstractmethod - def extract_info(self, data: dict) -> Any: + def extract_info(self, data: dict) -> T: raise NotImplementedError # pragma: no cover - def get_coverage(self, config: Config) -> Any: - return self.get_coverage_info(coverage_path=config.COVERAGE_PATH) - @abstractmethod def get_diff_coverage( self, added_lines: dict[pathlib.Path, list[int]], - coverage: Any, + coverage: T, config: Config, ) -> DiffCoverage: raise NotImplementedError # pragma: no cover @classmethod - def get_coverage_handler(cls, test_framework: TestFramework) -> type['BaseCoverageHandler']: + def get_coverage_handler( + cls, + test_framework: TestFramework, + ) -> 'BaseCoverageHandler[PytestCoverage | JestCoverage]': try: - return cls.REGISTRY[test_framework] + return cast( + 'BaseCoverageHandler[PytestCoverage | JestCoverage]', + cls.REGISTRY[test_framework](), + ) except KeyError as exc: log.error('No coverage handler found for test framework: %s', test_framework.value) raise ConfigurationException from exc diff --git a/codecov/coverage/jest.py b/codecov/coverage/jest.py index 6252757..85ae1a9 100644 --- a/codecov/coverage/jest.py +++ b/codecov/coverage/jest.py @@ -3,7 +3,7 @@ import pathlib from codecov.config import Config, TestFramework -from codecov.coverage.base import BaseCoverageHandler, DiffCoverage, FileDiffCoverage +from codecov.coverage.base import BaseCoverage, BaseCoverageHandler, DiffCoverage, FileDiffCoverage @dataclasses.dataclass @@ -29,12 +29,12 @@ class JestFileCoverage: @dataclasses.dataclass -class JestCoverage: +class JestCoverage(BaseCoverage): info: JestCoverageInfo files: dict[pathlib.Path, JestFileCoverage] -class JestCoverageHandler(BaseCoverageHandler): +class JestCoverageHandler(BaseCoverageHandler[JestCoverage]): TEST_FRAMEWORK: TestFramework = TestFramework.JEST """ diff --git a/codecov/coverage/pytest.py b/codecov/coverage/pytest.py index fa2cfe3..cf35d39 100644 --- a/codecov/coverage/pytest.py +++ b/codecov/coverage/pytest.py @@ -5,7 +5,7 @@ from codecov import diff_grouper from codecov.config import Config, TestFramework -from codecov.coverage.base import BaseCoverageHandler, DiffCoverage, FileDiffCoverage +from codecov.coverage.base import BaseCoverage, BaseCoverageHandler, DiffCoverage, FileDiffCoverage @dataclasses.dataclass @@ -42,13 +42,13 @@ class PytestCoverageMetadata: @dataclasses.dataclass -class PytestCoverage: +class PytestCoverage(BaseCoverage): info: PytestCoverageInfo files: dict[pathlib.Path, PytestFileCoverage] meta: PytestCoverageMetadata -class PytestCoverageHandler(BaseCoverageHandler): +class PytestCoverageHandler(BaseCoverageHandler[PytestCoverage]): TEST_FRAMEWORK: TestFramework = TestFramework.PYTEST def compute_coverage( @@ -97,7 +97,6 @@ def extract_file_coverage(self, path: str, file_data: dict) -> PytestFileCoverag info=self.extract_coverage_info(file_data['summary']), ) - # TODO: Fix the typing of data def extract_info(self, data: dict) -> PytestCoverage: """ { diff --git a/codecov/groups.py b/codecov/groups.py index e2db9e6..52d7984 100644 --- a/codecov/groups.py +++ b/codecov/groups.py @@ -70,7 +70,6 @@ def create_missing_coverage_annotations( return formatted_annotations -# TODO: Write tests for this function def compute_contiguous_groups( values: list[int], separators: set[int], diff --git a/codecov/main.py b/codecov/main.py index e40be18..aa49ee4 100644 --- a/codecov/main.py +++ b/codecov/main.py @@ -39,9 +39,9 @@ def _init_github(self) -> Github: ) return github - def _init_coverage_module(self) -> BaseCoverageHandler: + def _init_coverage_module(self): try: - return BaseCoverageHandler.get_coverage_handler(test_framework=self.config.TEST_FRAMEWORK)() + return BaseCoverageHandler.get_coverage_handler(test_framework=self.config.TEST_FRAMEWORK) except ConfigurationException as e: log.error('Error initializing coverage module. Please check the test framework and try again.') raise CoreProcessingException from e diff --git a/codecov/template.py b/codecov/template.py index 5f754ed..0460ec7 100644 --- a/codecov/template.py +++ b/codecov/template.py @@ -17,11 +17,10 @@ from codecov.exceptions import MissingMarker, TemplateException from codecov.log import log -# TODO: Move all the helpers to utils.py MARKER = """""" -def pluralize(number, singular='', plural='s'): +def pluralize(number: int, singular: str = '', plural: str = 's') -> str: if number == 1: return singular @@ -96,7 +95,6 @@ def get_comment_markdown( # pylint: disable=too-many-arguments,too-many-locals, ) } - # TODO: Check this code and see the groups are correctly generated missing_lines_for_whole_project = { key: list(value) for key, value in itertools.groupby( diff --git a/codecov/template_files/comment.md.j2 b/codecov/template_files/comment.md.j2 index 9bea4c3..728e2c9 100644 --- a/codecov/template_files/comment.md.j2 +++ b/codecov/template_files/comment.md.j2 @@ -34,7 +34,7 @@ {%- block full_coverage_report_link -%} {%- if coverage_report_url %} -

See the full coverage report of the project here.

+

See the full coverage report of the project here.

{%- endif -%} {%- endblock full_coverage_report_link %} diff --git a/tests/coverage/test_base.py b/tests/coverage/test_base.py index a0e173a..aad18d2 100644 --- a/tests/coverage/test_base.py +++ b/tests/coverage/test_base.py @@ -26,20 +26,23 @@ class TestBase: def test_compute_coverage(self, num_covered, num_total, expected_coverage): assert PytestCoverageHandler().compute_coverage(num_covered, num_total) == decimal.Decimal(expected_coverage) - def test_get_coverage_info(self, coverage_json): + def test_get_coverage_info(self, test_config, coverage_json): handler = PytestCoverageHandler() with patch('pathlib.Path.open') as mock_open: mock_open.return_value.__enter__.return_value.read.return_value = json.dumps(coverage_json) - result = handler.get_coverage_info(pathlib.Path(tempfile.mkstemp(suffix='.json')[1])) + test_config.COVERAGE_PATH = pathlib.Path(tempfile.mkstemp(suffix='.json')[1]) + result = handler.get_coverage(config=test_config) assert result == handler.extract_info(coverage_json) with patch('pathlib.Path.open') as mock_open: mock_open.return_value.__enter__.return_value.read.return_value = b'invalid json' with pytest.raises(ConfigurationException): - handler.get_coverage_info(pathlib.Path(tempfile.mkstemp(suffix='.json')[1])) + test_config.COVERAGE_PATH = pathlib.Path(tempfile.mkstemp(suffix='.json')[1]) + handler.get_coverage(config=test_config) with pytest.raises(ConfigurationException): - handler.get_coverage_info(pathlib.Path('path/to/file.json')) + test_config.COVERAGE_PATH = pathlib.Path('path/to/file.json') + handler.get_coverage(config=test_config) @pytest.mark.parametrize( 'added_lines, update_obj, expected', diff --git a/tests/test_groups.py b/tests/test_groups.py index 52bad5e..43b19e8 100644 --- a/tests/test_groups.py +++ b/tests/test_groups.py @@ -2,7 +2,7 @@ import pytest -from codecov.groups import Annotation, Group, create_missing_coverage_annotations +from codecov.groups import Annotation, Group, compute_contiguous_groups, create_missing_coverage_annotations def test_annotation_str(): @@ -100,3 +100,47 @@ def test_annotation_to_dict(): ) def test_create_missing_coverage_annotations(annotation_type, annotations, expected_annotations): assert create_missing_coverage_annotations(annotation_type, annotations) == expected_annotations + + +@pytest.mark.parametrize( + 'values, separators, joiners, max_gap, expected', + [ + # empty input returns no groups + ([], set(), set(), 3, []), + # single value forms a one-line group + ([5], set(), set(), 3, [(5, 5)]), + # consecutive values are grouped in pass 1 + ([10, 11, 12], set(), set(), 3, [(10, 12)]), + # gap larger than max_gap without joiners does not merge + ([1, 2, 7, 8], set(), set(), 3, [(1, 2), (7, 8)]), + # gap lines that are all joiners are bridged and groups merge + ([5, 6, 10, 11], set(), {7, 8, 9}, 3, [(5, 11)]), + # multiple single-line groups merge across joiner-only gaps + ([1, 3, 5], set(), {2, 4}, 3, [(1, 5)]), + # chained merges across several joiner-only gaps + ([1, 2, 5, 6, 10], set(), {3, 4, 7, 8, 9}, 3, [(1, 10)]), + # gap exactly at max_gap merges when there are no separators + ([1, 5], set(), set(), 3, [(1, 5)]), + # gap one line over max_gap does not merge + ([1, 6], set(), set(), 3, [(1, 1), (6, 6)]), + # separator line in the gap blocks merging + ([10, 11, 12, 15, 16], {13}, {14}, 3, [(10, 12), (15, 16)]), + # large meaningful gap (covered lines) exceeds max_gap and does not merge + ( + [5, 6, 20, 21], + {10, 11, 12, 13, 14, 15}, + {7, 8, 9, 16, 17, 18, 19}, + 3, + [(5, 6), (20, 21)], + ), + ], +) +def test_compute_contiguous_groups(values, separators, joiners, max_gap, expected): + """ + values: sorted line numbers to group (e.g. missing coverage lines) + separators: lines that block merging (e.g. covered or excluded lines) + joiners: lines that do not count toward gap size (e.g. blanks, comments) + max_gap: maximum number of non-joiner lines allowed in a gap when merging + expected: resulting list of (start, end) inclusive ranges + """ + assert compute_contiguous_groups(values, separators, joiners, max_gap) == expected