diff --git a/.cursor/rules/create-pr.mdc b/.cursor/rules/create-pr.mdc index f79baa9..4369de9 100644 --- a/.cursor/rules/create-pr.mdc +++ b/.cursor/rules/create-pr.mdc @@ -45,6 +45,7 @@ docs: update environment variable docs - Allowed types: `feature`, `fix`, `docs`, `refactor` (optional `!` for breaking changes). - Only commit when the user asks. Never amend pushed commits unless explicitly requested. +- If there are additional changes added after the agent changes, **do not rollback** to previous changes. Continue with the changes and commit. ## Pre-PR checks diff --git a/README.md b/README.md index ae01fea..f9222e0 100644 --- a/README.md +++ b/README.md @@ -50,12 +50,21 @@ Note: Either `GITHUB_PR_NUMBER` or `GITHUB_REF` is required. `GITHUB_PR_NUMBER` 1. The coverage report displays only files that have missing coverage. If all files are fully covered, the report will be empty. -2. When branch coverage is enabled, the coverage percentage is calculated based on the uncovered branches in - the affected files. -3. If the complete project report option is enabled, the report is included as-is in the comment, without any - modifications or recalculations. If you notice discrepancies between the PR coverage and the complete - project coverage, this may be expected. For consistent results, it is recommended to enable branch - coverage when your report includes it. +2. When branch coverage is enabled, the pull request coverage percentage counts branch arcs whose source + line is among the added lines in the diff (alongside the added statements). The `Missing branches` + column lists those arcs as `source -> destination`. An arc that leaves its enclosing scope is shown as + `source -> exit`. +3. In the pull request table, the `Branches` / `Missing` badges are whole-file totals from the coverage + report, while the `Missing branches` links list only arcs on added lines. A Missing count can therefore + be larger than the number of links shown for that file. The same pattern already applies to `Statements` / + `Missing stmts`. +4. Pull request coverage is recalculated from the statement and branch arc lists. The report's overall + coverage percentage comes from coverage.py's summary counts, which can still credit + `# pragma: no branch` arcs. So, the two percentages can differ slightly even when the PR adds an entire + file. Enable `BRANCH_COVERAGE` when your report includes branch data so both sides include branches. +5. If the complete project report option is enabled, file totals and the project coverage percentage are + taken from the report as-is. Differences versus pull request coverage are expected when the scopes or + formulas differ as above. ## Dev Setup diff --git a/codecov/coverage/base.py b/codecov/coverage/base.py index c1e7368..a9d39a0 100644 --- a/codecov/coverage/base.py +++ b/codecov/coverage/base.py @@ -24,6 +24,10 @@ class FileDiffCoverage: # Added lines tracks all the lines that were added in the diff, not just # the statements (so it includes comments, blank lines, etc.) added_lines: list[int] + # Branch arcs ([source line, destination line]) that start on an added line. + # Both stay empty unless the report contains branch coverage and it is enabled. + covered_branches: list[list[int]] = dataclasses.field(default_factory=list) + missing_branches: list[list[int]] = dataclasses.field(default_factory=list) @dataclasses.dataclass diff --git a/codecov/coverage/pytest.py b/codecov/coverage/pytest.py index cf35d39..96a5071 100644 --- a/codecov/coverage/pytest.py +++ b/codecov/coverage/pytest.py @@ -3,7 +3,6 @@ import decimal import pathlib -from codecov import diff_grouper from codecov.config import Config, TestFramework from codecov.coverage.base import BaseCoverage, BaseCoverageHandler, DiffCoverage, FileDiffCoverage @@ -150,6 +149,18 @@ def extract_info(self, data: dict) -> PytestCoverage: info=self.extract_coverage_info(data['totals']), ) + @staticmethod + def select_diff_branches( + branches: list[list[int]] | None, + added_lines: set[int], + ) -> list[list[int]]: + """ + Branches are ``[source line, destination line]`` arcs, where a negative destination + means the branch leaves the enclosing scope. Only the source line says where the + branch is written, so that is what decides whether the arc belongs to the diff. + """ + return [branch for branch in branches or [] if branch[0] in added_lines] + def get_diff_coverage( # pylint: disable=too-many-locals self, added_lines: dict[pathlib.Path, list[int]], @@ -184,14 +195,22 @@ def get_diff_coverage( # pylint: disable=too-many-locals total_num_lines += count_total total_num_violations += count_missing + covered_branches: list[list[int]] = [] + missing_branches: list[list[int]] = [] if config.BRANCH_COVERAGE: - total_num_branches_covered += file.info.covered_branches or 0 - total_num_branches += file.info.num_branches or 0 + added_lines_set = set(added_lines_for_file) + covered_branches = self.select_diff_branches(file.executed_branches, added_lines_set) + missing_branches = self.select_diff_branches(file.missing_branches, added_lines_set) + count_branches_covered = len(covered_branches) + count_branches = count_branches_covered + len(missing_branches) + + total_num_branches_covered += count_branches_covered + total_num_branches += count_branches percent_covered = self.compute_coverage( num_covered=count_executed, num_total=count_total, - num_branches_covered=file.info.covered_branches or 0, - num_branches_total=file.info.num_branches or 0, + num_branches_covered=count_branches_covered, + num_branches_total=count_branches, ) else: percent_covered = self.compute_coverage(num_covered=count_executed, num_total=count_total) @@ -203,6 +222,8 @@ def get_diff_coverage( # pylint: disable=too-many-locals missing_statements=sorted(missing), added_statements=sorted(added), added_lines=added_lines_for_file, + covered_branches=covered_branches, + missing_branches=missing_branches, ) if config.BRANCH_COVERAGE: final_percentage = self.compute_coverage( @@ -224,11 +245,3 @@ def get_diff_coverage( # pylint: disable=too-many-locals num_changed_lines=num_changed_lines, files=files, ) - - def get_coverage(self, config: Config) -> PytestCoverage: - coverage = super().get_coverage(config=config) - - if config.BRANCH_COVERAGE: - coverage = diff_grouper.fill_branch_missing_groups(coverage=coverage) - - return coverage diff --git a/codecov/diff_grouper.py b/codecov/diff_grouper.py index d0dc6cd..70ecdbc 100644 --- a/codecov/diff_grouper.py +++ b/codecov/diff_grouper.py @@ -11,20 +11,6 @@ MAX_GROUP_GAP = 3 -def _flatten_branches(branches: list[list[int]] | None) -> list[int]: - flattened_branches: list[int] = [] - if not branches: - return flattened_branches - - for branch in branches: - start, end = abs(branch[0]), abs(branch[1]) - if start == end: - flattened_branches.append(start) - else: - flattened_branches.extend(range(min(start, end), max(start, end) + 1)) - return flattened_branches - - def get_missing_groups( coverage: 'PytestCoverage | JestCoverage', ) -> Iterable[groups.Group]: @@ -77,23 +63,3 @@ def get_diff_missing_groups( line_start=start, line_end=end, ) - - -def fill_branch_missing_groups(coverage: 'PytestCoverage') -> 'PytestCoverage': - for file_coverage in coverage.files.values(): - separators = { - *_flatten_branches(file_coverage.executed_branches), - *file_coverage.excluded_lines, - } - joiners = set(range(1, file_coverage.info.num_statements)) - separators - - file_coverage.missing_branches = [ - [start, end] - for start, end in groups.compute_contiguous_groups( - values=_flatten_branches(branches=file_coverage.missing_branches), - separators=separators, - joiners=joiners, - max_gap=MAX_GROUP_GAP, - ) - ] - return coverage diff --git a/codecov/template_files/comment.md.j2 b/codecov/template_files/comment.md.j2 index 2bc19de..96c6919 100644 --- a/codecov/template_files/comment.md.j2 +++ b/codecov/template_files/comment.md.j2 @@ -6,14 +6,16 @@ {%- block coverage_evolution_badge -%} {%- if coverage %} {%- set color = coverage.info.percent_covered | x100 | get_badge_color -%} - + {%- set precise = coverage.info.percent_covered | pct(precision=2) -%} + {%- endif -%} {%- endblock coverage_evolution_badge -%}    {#- PR coverage badge -#} {%- block diff_coverage_badge -%} {%- set color = diff_coverage.total_percent_covered | x100 | get_badge_color -%} - + {%- set precise = diff_coverage.total_percent_covered | pct(precision=2) -%} + {%- endblock diff_coverage_badge -%} {%- endblock coverage_badges -%} diff --git a/codecov/template_files/macros.md.j2 b/codecov/template_files/macros.md.j2 index 9251a93..c8161a9 100644 --- a/codecov/template_files/macros.md.j2 +++ b/codecov/template_files/macros.md.j2 @@ -4,8 +4,8 @@ : in a GitHub Flavoured Markdown table the alignment belongs to the delimiter row of the table. -#} -{%- macro badge(path, label, message, color, base=false) -%} -[![]({{ label | generate_badge(message=message, color=color) }})]({{ path | file_url(base=base) }}) +{%- macro badge(path, label, message, color, base=false, title=none) -%} +[![]({{ label | generate_badge(message=message, color=color) }})]({{ path | file_url(base=base) }}{% if title %} "{{ title }}"{% endif %}) {%- endmacro -%} {%- macro statements_badge(path, statements_count, base=false) -%} @@ -27,14 +27,16 @@ {%- macro coverage_rate_badge(path, percent_covered, percent_covered_display, covered_statements_count, statements_count, base=false) -%} {%- set label = percent_covered_display ~ "%" -%} {%- set message = "(" ~ covered_statements_count ~ "/" ~ statements_count ~ ")" -%} - {{- badge(path, label, message, percent_covered | x100 | get_badge_color, base) -}} + {%- set title = percent_covered | pct(precision=2) -%} + {{- badge(path, label, message, percent_covered | x100 | get_badge_color, base, title) -}} {%- endmacro -%} {%- macro diff_coverage_rate_badge(path, added_statements_count, covered_statements_count, percent_covered) -%} {%- if added_statements_count -%} {%- set label = (percent_covered | pct(precision=0)) -%} {%- set message = "(" ~ covered_statements_count ~ "/" ~ added_statements_count ~ ")" -%} - {{- badge(path, label, message, percent_covered | x100 | get_badge_color()) -}} + {%- set title = percent_covered | pct(precision=2) -%} + {{- badge(path, label, message, percent_covered | x100 | get_badge_color(), title=title) -}} {%- else -%} {{- badge(path, "", "N/A", "grey") -}} {%- endif -%} @@ -48,11 +50,24 @@ {%- endfor -%} {%- endmacro -%} +{#- + A branch is a `[source line, destination line]` arc. A negative destination means the + branch leaves the enclosing scope, which coverage.py renders as `exit`, and there is no + destination line to link to in that case. The arc of a loop points backwards, so the + link range is normalised to keep the lower line first. +-#} {%- macro missing_branches_links(path, branches, base=false) -%} {%- set comma = joiner() -%} {%- for branch in branches -%} + {%- set source = branch[0] -%} + {%- set destination = branch[1] -%} {{- comma() -}} - [{{ branch[0] | abs }} -> {{ branch[1] | abs }}]({{ path | file_url(lines=(branch[0] | abs, branch[1] | abs), base=base) }}) + {%- if destination < 0 -%} + [{{ source }} -> exit]({{ path | file_url(lines=(source, source), base=base) }}) + {%- else -%} + {%- set lines = (source, destination) if destination > source else (destination, source) -%} + [{{ source }} -> {{ destination }}]({{ path | file_url(lines=lines, base=base) }}) + {%- endif -%} {%- endfor -%} {%- endmacro -%} @@ -71,10 +86,13 @@ | :-- | :-: | :-: |{% if branch_coverage %} :-: | :-: |{% endif %} :-: |{% if with_diff %} :-: |{% endif %} :-- |{% if branch_coverage %} :-- |{% endif %} {%- endmacro -%} +{#- The pull request table lists the branches missing coverage among the added lines only, + the whole project table lists every branch missing coverage in the file. -#} {%- macro file_row(file, missing_map, branch_coverage, base, with_diff) -%} {%- set path = file.coverage.path -%} {%- set info = file.coverage.info -%} -|   [{{ path.name }}]({{ path | file_url(base=base) }}) | {{ statements_badge(path, info.num_statements, base) }} | {{ missing_lines_badge(path, info.missing_lines, base) }} |{% if branch_coverage %} {{ branches_badge(path, info.num_branches, base) }} | {{ missing_branches_badge(path, info.missing_branches, base) }} |{% endif %} {{ coverage_rate_badge(path, info.percent_covered, info.percent_covered_display, info.covered_lines, info.num_statements, base) }} |{% if with_diff %} {{ diff_coverage_rate_badge(path, (file.diff.added_statements | length) if file.diff else none, (file.diff.covered_statements | length) if file.diff else none, file.diff.percent_covered if file.diff else none) }} |{% endif %} {{ missing_lines_links(path, missing_map.get(path, []), base) }} |{% if branch_coverage %} {{ missing_branches_links(path, file.coverage.missing_branches or [], base) }} |{% endif %} +{%- set missing_branches = (file.diff.missing_branches if with_diff and file.diff else file.coverage.missing_branches) or [] -%} +|   [{{ path.name }}]({{ path | file_url(base=base) }}) | {{ statements_badge(path, info.num_statements, base) }} | {{ missing_lines_badge(path, info.missing_lines, base) }} |{% if branch_coverage %} {{ branches_badge(path, info.num_branches, base) }} | {{ missing_branches_badge(path, info.missing_branches, base) }} |{% endif %} {{ coverage_rate_badge(path, info.percent_covered, info.percent_covered_display, info.covered_lines, info.num_statements, base) }} |{% if with_diff %} {{ diff_coverage_rate_badge(path, (file.diff.added_statements | length) if file.diff else none, (file.diff.covered_statements | length) if file.diff else none, file.diff.percent_covered if file.diff else none) }} |{% endif %} {{ missing_lines_links(path, missing_map.get(path, []), base) }} |{% if branch_coverage %} {{ missing_branches_links(path, missing_branches, base) }} |{% endif %} {%- endmacro -%} {%- macro total_row(totals, diff_totals, branch_coverage, with_diff) -%} diff --git a/tests/conftest.py b/tests/conftest.py index 4f80141..f8541e5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -123,20 +123,32 @@ def _(code: str, has_branches: bool = True) -> PytestCoverage: coverage_obj.info.excluded_lines += 1 if has_branches and 'branch' in line: - coverage_obj.files[current_file].info.num_branches += 1 - coverage_obj.info.num_branches += 1 - coverage_obj.files[current_file].executed_branches.append([line_number, line_number + 1]) + file_coverage = coverage_obj.files[current_file] + # A branch is a `[source line, destination line]` arc, and an arc is either + # taken or missing, never both. A partial branch is a line with one arc of + # each, the missing one leaving the enclosing scope (a negative destination). if 'branch partial' in line: - # Even if it's partially covered, it's still considered as a missing branch - coverage_obj.files[current_file].missing_branches.append([line_number, line_number + 1]) - coverage_obj.files[current_file].info.num_partial_branches += 1 + file_coverage.executed_branches.append([line_number, line_number + 1]) + file_coverage.missing_branches.append([line_number, -line_number]) + file_coverage.info.num_branches += 2 + coverage_obj.info.num_branches += 2 + file_coverage.info.covered_branches += 1 + coverage_obj.info.covered_branches += 1 + file_coverage.info.missing_branches += 1 + coverage_obj.info.missing_branches += 1 + file_coverage.info.num_partial_branches += 1 coverage_obj.info.num_partial_branches += 1 elif 'branch covered' in line: - coverage_obj.files[current_file].info.covered_branches += 1 + file_coverage.executed_branches.append([line_number, line_number + 1]) + file_coverage.info.num_branches += 1 + coverage_obj.info.num_branches += 1 + file_coverage.info.covered_branches += 1 coverage_obj.info.covered_branches += 1 elif 'branch missing' in line: - coverage_obj.files[current_file].missing_branches.append([line_number, line_number + 1]) - coverage_obj.files[current_file].info.missing_branches += 1 + file_coverage.missing_branches.append([line_number, line_number + 1]) + file_coverage.info.num_branches += 1 + coverage_obj.info.num_branches += 1 + file_coverage.info.missing_branches += 1 coverage_obj.info.missing_branches += 1 info = coverage_obj.files[current_file].info @@ -221,15 +233,17 @@ def coverage_json(): 'percent_covered_display': '60%', 'missing_lines': 4, 'excluded_lines': 0, - 'num_branches': 3, + 'num_branches': 7, + # Line 5 is partial: one of its two arcs was taken. 'num_partial_branches': 1, - 'covered_branches': 1, - 'missing_branches': 1, + 'covered_branches': 4, + 'missing_branches': 3, }, 'missing_lines': [6, 8, 10, 11], 'excluded_lines': [], - 'executed_branches': [[1, 0], [2, 1], [3, 0], [5, 1], [13, 0], [14, 0]], - 'missing_branches': [[6, 0], [8, 1], [10, 0], [11, 0]], + 'executed_branches': [[2, 3], [3, 5], [5, 6], [13, 14]], + # A negative destination is a branch leaving the enclosing scope. + 'missing_branches': [[5, -1], [10, 11], [11, -1]], } }, 'totals': { @@ -239,10 +253,10 @@ def coverage_json(): 'percent_covered_display': '60%', 'missing_lines': 4, 'excluded_lines': 0, - 'num_branches': 3, + 'num_branches': 7, 'num_partial_branches': 1, - 'covered_branches': 1, - 'missing_branches': 1, + 'covered_branches': 4, + 'missing_branches': 3, }, } @@ -329,6 +343,15 @@ def diff_coverage_obj(coverage_obj, make_diff_coverage): ) +@pytest.fixture +def diff_coverage_obj_branch(coverage_obj, make_diff_coverage): + return make_diff_coverage( + added_lines={pathlib.Path('codebase/code.py'): [3, 4, 5, 6, 7, 8, 9, 12]}, + coverage=coverage_obj, + branch_coverage=True, + ) + + @pytest.fixture def diff_coverage_obj_more_files(coverage_obj_more_files, make_diff_coverage): return make_diff_coverage( diff --git a/tests/coverage/test_base.py b/tests/coverage/test_base.py index aad18d2..bdedbc6 100644 --- a/tests/coverage/test_base.py +++ b/tests/coverage/test_base.py @@ -167,7 +167,7 @@ def test_get_diff_coverage_info(self, test_config, make_coverage_obj, added_line 'added_lines, update_obj, expected', [ # A similar example to the previous one, but with branch coverage enabled. - # The statements are covered, but the branches are not. + # Only the branches written on an added line count ( { pathlib.Path('codebase/code.py'): [4, 5, 6], @@ -177,25 +177,33 @@ def test_get_diff_coverage_info(self, test_config, make_coverage_obj, added_line DiffCoverage( total_num_lines=1, total_num_violations=1, - total_percent_covered=decimal.Decimal('0.25'), + # 2 of the 4 added branches are covered, and the single added + # statement is not: (0 + 2) / (1 + 4). + total_percent_covered=decimal.Decimal('0.4'), num_changed_lines=5, - # Percent is due to the fact that the branches are not covered. files={ pathlib.Path('codebase/code.py'): FileDiffCoverage( path=pathlib.Path('codebase/code.py'), - percent_covered=decimal.Decimal('0.25'), + # (0 + 1) / (1 + 2) + percent_covered=decimal.Decimal(1) / decimal.Decimal(3), added_statements=[6], covered_statements=[], missing_statements=[6], added_lines=[4, 5, 6], + covered_branches=[[5, 6]], + missing_branches=[[5, -5]], ), pathlib.Path('codebase/other.py'): FileDiffCoverage( path=pathlib.Path('codebase/other.py'), - percent_covered=decimal.Decimal('0.25'), + # No statement was added, so only the branches count: + # (0 + 1) / (0 + 2) + percent_covered=decimal.Decimal('0.5'), added_statements=[], covered_statements=[], missing_statements=[], added_lines=[10, 13], + covered_branches=[[13, 14]], + missing_branches=[[10, 11]], ), }, ), @@ -219,25 +227,55 @@ def test_get_diff_coverage_info(self, test_config, make_coverage_obj, added_line DiffCoverage( total_num_lines=9, total_num_violations=4, - total_percent_covered=decimal.Decimal('0.4375'), + # (9 - 4 + 2) / (9 + 4) + total_percent_covered=decimal.Decimal(7) / decimal.Decimal(13), num_changed_lines=9, - # Percent is due to the fact that the branches are not covered. files={ pathlib.Path('codebase/code.py'): FileDiffCoverage( path=pathlib.Path('codebase/code.py'), - percent_covered=decimal.Decimal('0.625'), + # (4 + 1) / (5 + 2) + percent_covered=decimal.Decimal(5) / decimal.Decimal(7), added_statements=[2, 3, 4, 5, 6], covered_statements=[2, 3, 5, 6], missing_statements=[4, 5], added_lines=[2, 3, 4, 5, 6], + covered_branches=[[5, 6]], + missing_branches=[[5, -5]], ), pathlib.Path('codebase/other.py'): FileDiffCoverage( path=pathlib.Path('codebase/other.py'), - percent_covered=decimal.Decimal('0.625'), + # (4 + 1) / (4 + 2) + percent_covered=decimal.Decimal(5) / decimal.Decimal(6), added_statements=[10, 11, 12, 13], covered_statements=[10, 11, 12, 13], missing_statements=[10, 13], added_lines=[10, 11, 12, 13], + covered_branches=[[13, 14]], + missing_branches=[[10, 11]], + ), + }, + ), + ), + # A diff that adds no branch at all is fully covered, even though the file + # still holds branches missing coverage outside of the diff. + ( + {pathlib.Path('codebase/code.py'): [1, 2, 3]}, + {}, + DiffCoverage( + total_num_lines=3, + total_num_violations=0, + total_percent_covered=decimal.Decimal('1'), + num_changed_lines=3, + files={ + pathlib.Path('codebase/code.py'): FileDiffCoverage( + path=pathlib.Path('codebase/code.py'), + percent_covered=decimal.Decimal('1'), + added_statements=[1, 2, 3], + covered_statements=[1, 2, 3], + missing_statements=[], + added_lines=[1, 2, 3], + covered_branches=[], + missing_branches=[], ), }, ), diff --git a/tests/coverage/test_pytest.py b/tests/coverage/test_pytest.py index 96bec92..45dbf8b 100644 --- a/tests/coverage/test_pytest.py +++ b/tests/coverage/test_pytest.py @@ -4,6 +4,8 @@ import pathlib from unittest.mock import patch +import pytest + from codecov.coverage.pytest import ( PytestCoverage, PytestCoverageHandler, @@ -35,13 +37,13 @@ def test_extract_info(self, coverage_json): percent_covered_display='60%', missing_lines=4, excluded_lines=0, - num_branches=3, + num_branches=7, num_partial_branches=1, - covered_branches=1, - missing_branches=1, + covered_branches=4, + missing_branches=3, ), - executed_branches=[[1, 0], [2, 1], [3, 0], [5, 1], [13, 0], [14, 0]], - missing_branches=[[6, 0], [8, 1], [10, 0], [11, 0]], + executed_branches=[[2, 3], [3, 5], [5, 6], [13, 14]], + missing_branches=[[5, -1], [10, 11], [11, -1]], ) }, info=PytestCoverageInfo( @@ -51,16 +53,17 @@ def test_extract_info(self, coverage_json): percent_covered_display='60%', missing_lines=4, excluded_lines=0, - num_branches=3, + num_branches=7, num_partial_branches=1, - covered_branches=1, - missing_branches=1, + covered_branches=4, + missing_branches=3, ), ) assert PytestCoverageHandler().extract_info(coverage_json) == expected_coverage def test_get_coverage_with_branch_coverage(self, test_config, coverage_json): + """Branch arcs are reported as they come from the coverage report, never grouped.""" config = dataclasses.replace(test_config, BRANCH_COVERAGE=True) handler = PytestCoverageHandler() with patch('pathlib.Path.open') as mock_open: @@ -68,4 +71,24 @@ def test_get_coverage_with_branch_coverage(self, test_config, coverage_json): coverage = handler.get_coverage(config=config) assert coverage.meta.branch_coverage is True - assert coverage.files[pathlib.Path('codebase/code.py')].missing_branches == [[0, 11]] + code = coverage.files[pathlib.Path('codebase/code.py')] + assert code.missing_branches == [[5, -1], [10, 11], [11, -1]] + assert code.executed_branches == [[2, 3], [3, 5], [5, 6], [13, 14]] + + @pytest.mark.parametrize( + 'branches, added_lines, expected', + [ + (None, {1, 2}, []), + ([], {1, 2}, []), + # The source line decides whether the arc belongs to the diff, so an arc + # pointing at an added line from an untouched line is left out. + ([[2, 3], [4, 5]], {2, 3}, [[2, 3]]), + # An arc leaving the enclosing scope has a negative destination. + ([[2, -1], [4, -1]], {4}, [[4, -1]]), + # A loop branches backwards. + ([[7, 6]], {7}, [[7, 6]]), + ([[2, 3], [4, 5]], set(), []), + ], + ) + def test_select_diff_branches(self, branches, added_lines, expected): + assert PytestCoverageHandler.select_diff_branches(branches, added_lines) == expected diff --git a/tests/test_diff_grouper.py b/tests/test_diff_grouper.py index 5a31c19..e0d04c1 100644 --- a/tests/test_diff_grouper.py +++ b/tests/test_diff_grouper.py @@ -42,30 +42,8 @@ def test_get_missing_groups_more_files(coverage_obj_more_files): ] -def test_flatten_branches(): - assert not diff_grouper._flatten_branches(branches=None) +def test_get_missing_groups_ignores_branches(coverage_obj): + """Branches are arcs, not line ranges, so grouping never touches them.""" + list(diff_grouper.get_missing_groups(coverage=coverage_obj)) - flattened_branches = diff_grouper._flatten_branches([[1, 2], [3, 4]]) - assert flattened_branches == [1, 2, 3, 4] - - flattened_branches = diff_grouper._flatten_branches([[1, 1]]) - assert flattened_branches == [1] - - flattened_branches = diff_grouper._flatten_branches([[-1, -2], [3, 4]]) - assert flattened_branches == [1, 2, 3, 4] - - flattened_branches = diff_grouper._flatten_branches([[-1, -2], [3, 4], [5, 5]]) - assert flattened_branches == [1, 2, 3, 4, 5] - - -def test_fill_branch_missing_groups(coverage_obj): - result = diff_grouper.fill_branch_missing_groups(coverage=coverage_obj) - - assert result.files[pathlib.Path('codebase/code.py')].missing_branches == [[5, 11]] - - -def test_fill_branch_missing_groups_more_files(coverage_obj_more_files): - result = diff_grouper.fill_branch_missing_groups(coverage=coverage_obj_more_files) - - assert result.files[pathlib.Path('codebase/code.py')].missing_branches == [[5, 11]] - assert result.files[pathlib.Path('codebase/other.py')].missing_branches == [[3, 11]] + assert coverage_obj.files[pathlib.Path('codebase/code.py')].missing_branches == [[5, -5], [10, 11]] diff --git a/tests/test_template.py b/tests/test_template.py index dd238f5..77931d4 100644 --- a/tests/test_template.py +++ b/tests/test_template.py @@ -169,19 +169,26 @@ def test_comment_template(coverage_obj, diff_coverage_obj): assert 'Missing stmts' in result assert 'Branches' not in result assert 'img.shields.io/badge/' in result + # Coarse badge labels keep their rounded text; hover titles carry two decimals. + precise_coverage = template.pct(coverage_obj.info.percent_covered, precision=2) + precise_diff = template.pct(diff_coverage_obj.total_percent_covered, precision=2) + precise_file_diff = template.pct(chaned_files[0].diff.percent_covered, precision=2) + assert f'title="{precise_coverage}"' in result + assert f'title="{precise_diff}"' in result + assert f' "{precise_file_diff}")' in result -def test_comment_template_branch_coverage(coverage_obj, diff_coverage_obj): +def test_comment_template_branch_coverage(coverage_obj, diff_coverage_obj_branch): chaned_files, total = template.select_changed_files( coverage=coverage_obj, - diff_coverage=diff_coverage_obj, + diff_coverage=diff_coverage_obj_branch, max_files=25, skip_covered_files_in_report=True, ) result = template.get_comment_markdown( template.read_template_file('comment.md.j2'), coverage_obj, - diff_coverage_obj, + diff_coverage_obj_branch, decimal.Decimal('100'), decimal.Decimal('70'), 'org/repo', @@ -202,6 +209,10 @@ def test_comment_template_branch_coverage(coverage_obj, diff_coverage_obj): assert '(new stmts)' in result # The delimiter row must always declare as many columns as the header row assert '| :-- | :-: | :-: | :-: | :-: | :-: | :-: | :-- | :-- |' in result + # The partial branch on line 5 is in the diff and its missing arc leaves the scope. + assert '[5 -> exit]' in result + # The branch on line 10 is missing coverage but is outside the diff. + assert '[10 -> 11]' not in result def test_comment_template_project_report(coverage_obj, diff_coverage_obj): @@ -234,6 +245,39 @@ def test_comment_template_project_report(coverage_obj, diff_coverage_obj): assert '(new stmts)' not in result assert 'https://github.com/org/repo/blob/main/codebase/code.py' in result assert '| :-- | :-: | :-: | :-: | :-: | :-: | :-- | :-- |' in result + # The whole project table lists every branch missing coverage in the file, and links + # an arc leaving the scope to its source line only. + assert '[5 -> exit](https://github.com/org/repo/blob/main/codebase/code.py#L5-L5)' in result + assert '[10 -> 11](https://github.com/org/repo/blob/main/codebase/code.py#L10-L11)' in result + + +def test_comment_template_backward_branch(make_coverage_obj, diff_coverage_obj): + """A loop branches back to its own header, so the link range has to be reordered.""" + coverage = make_coverage_obj(**{'codebase/code.py': {'missing_branches': [[7, 6]]}}) + coverage_files, total = template.select_files( + coverage=coverage, + max_files=25, + skip_covered_files_in_report=True, + ) + result = template.get_comment_markdown( + template.read_template_file('comment.md.j2'), + coverage, + diff_coverage_obj, + decimal.Decimal('100'), + decimal.Decimal('70'), + 'org/repo', + 1, + 'main', + '', + coverage_files=coverage_files, + count_coverage_files=total, + files=[], + count_files=0, + max_files=25, + branch_coverage=True, + complete_project_report=True, + ) + assert '[7 -> 6](https://github.com/org/repo/blob/main/codebase/code.py#L6-L7)' in result def test_template_no_files(coverage_obj):