refactor: improve coverage handler typing and registry

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:

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,33 +70,35 @@ 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:
             log.error('Unable to extract coverage info from coverage report file: %s', coverage_path)
             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

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
 
     """

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:
         """
         {

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],

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

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 = """<!-- This comment was generated by codecov -->"""
 
 
-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(

codecov/template_files/comment.md.j2

@@ -34,7 +34,7 @@
 
 {%- block full_coverage_report_link -%}
     {%- if coverage_report_url %}
-        <h4> See the full coverage report of the project <a href="{{ coverage_report_url }}">here</a>.</h4>
+<h4> See the full coverage report of the project <a href="{{ coverage_report_url }}">here</a>.</h4>
     {%- endif -%}
 {%- endblock full_coverage_report_link %}
 

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',

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