diff --git a/coverage_comment/badge.py b/coverage_comment/badge.py index acdb0229..1824d940 100644 --- a/coverage_comment/badge.py +++ b/coverage_comment/badge.py @@ -25,15 +25,16 @@ def get_badge_color( def get_evolution_badge_color( - rate_before: decimal.Decimal | None, - rate_after: decimal.Decimal, + delta: decimal.Decimal | int, + up_is_good: bool, + neutral_color: str = "grey", ) -> str: - if rate_before is None or rate_after > rate_before: + if delta == 0: + return neutral_color + elif (delta > 0) is up_is_good: return "brightgreen" - elif rate_after == rate_before: - return "blue" else: - return "orange" + return "red" def compute_badge_endpoint_data( @@ -66,9 +67,10 @@ def compute_badge_image( def get_static_badge_url(label: str, message: str, color: str) -> str: - return "https://img.shields.io/badge/" + urllib.parse.quote( - f"{label}-{message}-{color}.svg" + code = "-".join( + e.replace("_", "__").replace("-", "--") for e in (label, message, color) ) + return "https://img.shields.io/badge/" + urllib.parse.quote(f"{code}.svg") def get_endpoint_url(endpoint_url: str) -> str: diff --git a/coverage_comment/coverage.py b/coverage_comment/coverage.py index 4ab3ef00..51956c23 100644 --- a/coverage_comment/coverage.py +++ b/coverage_comment/coverage.py @@ -7,19 +7,14 @@ import itertools import json import pathlib -from collections.abc import Sequence +from collections.abc import Iterable, Sequence from coverage_comment import log, subprocess -def collapse_lines(lines: list[int]) -> list[tuple[int, int]]: - # All consecutive line numbers have the same difference between their list index and their value. - # Grouping by this difference therefore leads to buckets of consecutive numbers. - for _, it in itertools.groupby(enumerate(lines), lambda x: x[1] - x[0]): - t = list(it) - yield t[0][1], t[-1][1] - - +# The dataclasses in this module are accessible in the template, which is overridable by the user. +# As a coutesy, we should do our best to keep the existing fields for backward compatibility, +# and if we really can't and can't add properties, at least bump the major version. @dataclasses.dataclass class CoverageMetadata: version: str @@ -67,13 +62,17 @@ class Coverage: class FileDiffCoverage: path: pathlib.Path percent_covered: decimal.Decimal - missing_lines: list[int] + covered_statements: list[int] + missing_statements: list[int] + added_statements: list[int] + # 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] # for backward compatibility @property def violation_lines(self) -> list[int]: - return self.missing_lines + return self.missing_statements @functools.cached_property def violation_lines_collapsed(self): @@ -259,9 +258,9 @@ def get_diff_coverage_info( missing = set(file.missing_lines) & set(added_lines_for_file) count_missing = len(missing) - # Even partially covered lines are considered as covered, no line - # appears in both counts - count_total = count_executed + count_missing + + added = executed | missing + count_total = len(added) total_num_lines += count_total total_num_violations += count_missing @@ -273,7 +272,9 @@ def get_diff_coverage_info( files[path] = FileDiffCoverage( path=path, percent_covered=percent_covered, - missing_lines=sorted(missing), + covered_statements=sorted(executed), + missing_statements=sorted(missing), + added_statements=sorted(added), added_lines=added_lines_for_file, ) final_percentage = compute_coverage( @@ -327,3 +328,11 @@ def parse_line_number_diff_line(line: str) -> Sequence[int]: """ start, length = (int(i) for i in (line.split()[2][1:] + ",1").split(",")[:2]) return range(start, start + length) + + +def collapse_lines(lines: list[int]) -> Iterable[tuple[int, int]]: + # All consecutive line numbers have the same difference between their list index and their value. + # Grouping by this difference therefore leads to buckets of consecutive numbers. + for _, it in itertools.groupby(enumerate(lines), lambda x: x[1] - x[0]): + t = list(it) + yield t[0][1], t[-1][1] diff --git a/coverage_comment/annotations.py b/coverage_comment/diff_grouper.py similarity index 95% rename from coverage_comment/annotations.py rename to coverage_comment/diff_grouper.py index d8f61094..d48043b4 100644 --- a/coverage_comment/annotations.py +++ b/coverage_comment/diff_grouper.py @@ -12,7 +12,7 @@ @dataclasses.dataclass(frozen=True) -class Annotation: +class Group: file: pathlib.Path line_start: int line_end: int @@ -68,10 +68,10 @@ def reducer( return functools.reduce(reducer, contiguous_groups, []) -def group_annotations( +def get_diff_missing_groups( coverage: coverage_module.Coverage, diff_coverage: coverage_module.DiffCoverage, -) -> Iterable[Annotation]: +) -> Iterable[Group]: for path, diff_file in diff_coverage.files.items(): coverage_file = coverage.files[path] @@ -88,11 +88,11 @@ def group_annotations( joiners = set(diff_file.added_lines) - separators for start, end in compute_contiguous_groups( - values=diff_file.missing_lines, + values=diff_file.missing_statements, separators=separators, joiners=joiners, ): - yield Annotation( + yield Group( file=path, line_start=start, line_end=end, diff --git a/coverage_comment/files.py b/coverage_comment/files.py index 69ec48ec..a4cd350f 100644 --- a/coverage_comment/files.py +++ b/coverage_comment/files.py @@ -108,10 +108,17 @@ def compute_datafile( ) -def parse_datafile(contents) -> decimal.Decimal: - return decimal.Decimal(str(json.loads(contents)["coverage"])) / decimal.Decimal( +def parse_datafile(contents) -> tuple[coverage.Coverage | None, decimal.Decimal]: + file_contents = json.loads(contents) + coverage_rate = decimal.Decimal(str(file_contents["coverage"])) / decimal.Decimal( "100" ) + try: + return coverage.extract_info( + data=file_contents["raw_data"], coverage_path=file_contents["coverage_path"] + ), coverage_rate + except KeyError: + return None, coverage_rate class ImageURLs(TypedDict): diff --git a/coverage_comment/main.py b/coverage_comment/main.py index 052ffa3b..5fc6181f 100644 --- a/coverage_comment/main.py +++ b/coverage_comment/main.py @@ -8,12 +8,10 @@ import httpx from coverage_comment import activity as activity_module -from coverage_comment import ( - annotations as annotations_module, -) from coverage_comment import ( comment_file, communication, + diff_grouper, files, github, github_client, @@ -151,16 +149,19 @@ def process_pr( branch=config.FINAL_COVERAGE_DATA_BRANCH, ) - previous_coverage = None + previous_coverage, previous_coverage_rate = None, None if previous_coverage_data_file: - previous_coverage = files.parse_datafile(contents=previous_coverage_data_file) + previous_coverage, previous_coverage_rate = files.parse_datafile( + contents=previous_coverage_data_file + ) marker = template.get_marker(marker_id=config.SUBPROJECT_ID) try: comment = template.get_comment_markdown( coverage=coverage, diff_coverage=diff_coverage, - previous_coverage_rate=previous_coverage, + previous_coverage=previous_coverage, + previous_coverage_rate=previous_coverage_rate, minimum_green=config.MINIMUM_GREEN, minimum_orange=config.MINIMUM_ORANGE, repo_name=config.GITHUB_REPOSITORY, @@ -207,7 +208,7 @@ def process_pr( pr_number = None if pr_number is not None and config.ANNOTATE_MISSING_LINES: - annotations = annotations_module.group_annotations( + annotations = diff_grouper.get_diff_missing_groups( coverage=coverage, diff_coverage=diff_coverage ) github.create_missing_coverage_annotations( diff --git a/coverage_comment/template.py b/coverage_comment/template.py index d1150e91..c22e4cac 100644 --- a/coverage_comment/template.py +++ b/coverage_comment/template.py @@ -1,7 +1,10 @@ from __future__ import annotations +import dataclasses import decimal +import functools import hashlib +import itertools import pathlib from collections.abc import Callable from importlib import resources @@ -9,12 +12,13 @@ import jinja2 from jinja2.sandbox import SandboxedEnvironment -from coverage_comment import badge +from coverage_comment import badge, diff_grouper from coverage_comment import coverage as coverage_module MARKER = ( """""" ) +MAX_FILES = 100 def uptodate(): @@ -50,10 +54,65 @@ def get_marker(marker_id: str | None): return MARKER.format(id_part=f" (id: {marker_id})" if marker_id else "") +def pluralize(number, singular="", plural="s"): + if number == 1: + return singular + else: + return plural + + +def sign(val: int | decimal.Decimal) -> str: + return "+" if val >= 0 else "" if val < 0 else "±" + + +def delta(val: int) -> str: + return f"({sign(val)}{val})" + + +def percentage_value(val: decimal.Decimal, precision: int = 2) -> decimal.Decimal: + return val.quantize( + decimal.Decimal("1." + ("0" * precision)), rounding=decimal.ROUND_DOWN + ).normalize() + + +def pct(val: decimal.Decimal | float, precision: int = 2) -> str: + if isinstance(val, decimal.Decimal): + val *= decimal.Decimal("100") + rounded = percentage_value(val=val, precision=precision) + return f"{rounded:f}%" + else: + return f"{val:.{precision}%}" + + +def pct_delta(val: decimal.Decimal, precision: int = 2) -> str: + """ + Used for percentage point deltas + """ + return f"({sign(val=val)}{percentage_value(val=val, precision=precision)})" + + +@dataclasses.dataclass +class FileInfo: + path: pathlib.Path + coverage: coverage_module.FileCoverage + diff: coverage_module.FileDiffCoverage | None + previous: coverage_module.FileCoverage | None + + @property + def new_missing_lines(self) -> list[int]: + missing_lines = set(self.coverage.missing_lines) + if self.previous: + missing_lines -= set(self.previous.missing_lines) + + return sorted(missing_lines) + + def get_comment_markdown( + *, coverage: coverage_module.Coverage, diff_coverage: coverage_module.DiffCoverage, previous_coverage_rate: decimal.Decimal | None, + previous_coverage: coverage_module.Coverage | None, minimum_green: decimal.Decimal, minimum_orange: decimal.Decimal, repo_name: str, @@ -67,8 +126,10 @@ def get_comment_markdown( loader = CommentLoader(base_template=base_template, custom_template=custom_template) env = SandboxedEnvironment(loader=loader) env.filters["pct"] = pct - env.filters["file_url"] = get_file_url_function( - repo_name=repo_name, pr_number=pr_number + env.filters["delta"] = delta + env.filters["pct_delta"] = pct_delta + env.filters["file_url"] = functools.partial( + get_file_url, repo_name=repo_name, pr_number=pr_number ) env.filters["get_badge_color"] = lambda r: badge.get_badge_color( decimal.Decimal(r) * decimal.Decimal("100"), @@ -77,12 +138,34 @@ def get_comment_markdown( ) env.filters["get_evolution_color"] = badge.get_evolution_badge_color env.filters["generate_badge"] = badge.get_static_badge_url + env.filters["pluralize"] = pluralize + + previous_coverage_files = previous_coverage.files if previous_coverage else {} + + # files contain the MAX_FILES files with the most new missing lines sorted by path + files = select_files( + coverage_files=coverage.files, + diff_coverage_files=diff_coverage.files, + previous_coverage_files=previous_coverage_files, + ) + missing_diff_lines = { + key: list(value) + for key, value in itertools.groupby( + diff_grouper.get_diff_missing_groups( + coverage=coverage, diff_coverage=diff_coverage + ), + lambda x: x.file, + ) + } try: comment = env.get_template("custom" if custom_template else "base").render( previous_coverage_rate=previous_coverage_rate, coverage=coverage, diff_coverage=diff_coverage, + previous_coverage=previous_coverage, + files=files, + missing_diff_lines=missing_diff_lines, subproject_id=subproject_id, marker=marker, pr_targets_default_branch=pr_targets_default_branch, @@ -96,6 +179,41 @@ def get_comment_markdown( return comment +def select_files( + coverage_files: dict[pathlib.Path, coverage_module.FileCoverage], + diff_coverage_files: dict[pathlib.Path, coverage_module.FileDiffCoverage], + previous_coverage_files: dict[pathlib.Path, coverage_module.FileCoverage], + max_files: int = MAX_FILES, +) -> list[FileInfo]: + """ + Selects the MAX_FILES files with the most new missing lines sorted by path + + """ + files = [] + for path, coverage_file in coverage_files.items(): + diff_coverage_file = diff_coverage_files.get(path) + previous_coverage_file = previous_coverage_files.get(path) + + file_info = FileInfo( + path=path, + coverage=coverage_file, + diff=diff_coverage_file, + previous=previous_coverage_file, + ) + has_diff = bool(diff_coverage_file) + has_evolution_from_previous = ( + previous_coverage_file.info != coverage_file.info + if previous_coverage_file + else False + ) + + if has_diff or has_evolution_from_previous: + files.append(file_info) + + files = sorted(files, key=lambda x: x.new_missing_lines)[:max_files] + return sorted(files, key=lambda x: x.path) + + def get_readme_markdown( is_public: bool, readme_url: str, @@ -148,26 +266,17 @@ def read_template_file(template: str) -> str: ).read_text() -def pct(val: decimal.Decimal | float) -> str: - if isinstance(val, decimal.Decimal): - val *= decimal.Decimal("100") - return f"{val.quantize(decimal.Decimal('0.01'), rounding=decimal.ROUND_DOWN).normalize():f}%" - else: - return f"{val:.0%}" - - -def get_file_url_function(repo_name: str, pr_number: int) -> Callable: - def _( - filename: pathlib.Path, - lines: tuple[int, int] | None = None, - ) -> str: - # To link to a file in a PR, GitHub uses the link to the file overview combined with a SHA256 hash of the file path - s = f"https://github.com/{repo_name}/pull/{pr_number}/files#diff-{hashlib.sha256(str(filename).encode('utf-8')).hexdigest()}" - - if lines is not None: - # R stands for Right side of the diff. But since we generate these links for new code we only need the right side. - s += f"R{lines[0]}-R{lines[1]}" +def get_file_url( + repo_name: str, + pr_number: int, + filename: pathlib.Path, + lines: tuple[int, int] | None = None, +) -> str: + # To link to a file in a PR, GitHub uses the link to the file overview combined with a SHA256 hash of the file path + s = f"https://github.com/{repo_name}/pull/{pr_number}/files#diff-{hashlib.sha256(str(filename).encode('utf-8')).hexdigest()}" - return s + if lines is not None: + # R stands for Right side of the diff. But since we generate these links for new code we only need the right side. + s += f"R{lines[0]}-R{lines[1]}" - return _ + return s diff --git a/coverage_comment/template_files/comment.md.j2 b/coverage_comment/template_files/comment.md.j2 index bc99e6ad..28ca57fe 100644 --- a/coverage_comment/template_files/comment.md.j2 +++ b/coverage_comment/template_files/comment.md.j2 @@ -1,42 +1,220 @@ -{% block title %}## Coverage report{% if subproject_id %} ({{ subproject_id }}){% endif %}{% endblock title %} -{% block coverage_badges -%} - -{% block coverage_evolution_badge -%} -{% set text = "Coverage for the whole project went from " + ("unknown" if previous_coverage_rate == None else previous_coverage_rate | pct) + " to " + coverage.info.percent_covered | pct -%} -{{ text }} -{% endblock coverage_evolution_badge -%} -{% block pr_coverage_badge -%} -{% set text = diff_coverage.total_percent_covered | pct + " of the code lines added by this PR are covered" -%} -{{ text }} -{% endblock pr_coverage_badge -%} -{% block branch_coverage_badge -%} -{% if coverage.meta.branch_coverage and coverage.info.num_branches -%} -{% set text = "Branch coverage for the whole project on this PR is " + (coverage.info.covered_branches / coverage.info.num_branches) | pct -%} -{{ text }} -{% endif -%} -{% endblock branch_coverage_badge -%} - -{%- endblock coverage_badges %} - -{% block coverage_by_file -%} -{%if diff_coverage.files -%} -
-{% block coverage_by_file_summary_wording -%}Diff Coverage details (click to unfold){% endblock coverage_by_file_summary_wording -%} -{% for filename, diff_file_coverage in diff_coverage.files.items() -%} -{% block coverage_single_file scoped %} -{% block coverage_single_file_title scoped %}### [{{ filename }}]({{ filename | file_url }}){% endblock coverage_single_file_title %} -{% block diff_coverage_single_file_wording scoped -%} -`{{ diff_file_coverage.percent_covered | pct }}` of new lines are covered (`{{ coverage.files[filename].info.percent_covered | pct }}` of the complete file). -{%- endblock diff_coverage_single_file_wording %} -{%- if diff_file_coverage.violation_lines -%} -{% block single_file_missing_lines_wording scoped -%} -{% set separator = joiner(", ") %} -Missing lines: {% for lines in diff_file_coverage.violation_lines_collapsed %}{{ separator() }}[`{{ lines[0] }}{{ "-" ~ lines[1] if lines[1] > lines[0] }}`]({{ filename | file_url(lines) }}){% endfor %} -{%- endblock single_file_missing_lines_wording %} -{%- endif %} -{% endblock coverage_single_file -%} -{%- endfor %} +{%- block title -%}## Coverage report{%- if subproject_id -%} ({{ subproject_id }}){%- endif -%}{%- endblock title -%} + +{# Coverage evolution badge #} +{%- block coverage_badges -%} +{%- block coverage_evolution_badge -%} +{%- if previous_coverage_rate %} +{%- set text = "Coverage for the whole project went from " ~ (previous_coverage_rate | pct) ~ " to " ~ (coverage.info.percent_covered | pct) -%} +{%- set color = (coverage.info.percent_covered - previous_coverage_rate) | get_evolution_color(neutral_color='blue') -%} +{{ text }} + +{%- else -%} +{%- set text = "Coverage for the whole project is " ~ (coverage.info.percent_covered | pct) ~ ". Previous coverage rate is not available, cannot report on evolution." -%} +{%- set color = coverage.info.percent_covered | get_badge_color -%} +{{ text }} + +{%- endif -%} +{%- endblock coverage_evolution_badge -%} + +{# Coverage diff badge #} +{%- block diff_coverage_badge -%} +{%- set text = (diff_coverage.total_percent_covered | pct) ~ " of the statement lines added by this PR are covered" -%} +{{ text }} + +{%- endblock diff_coverage_badge -%} +{%- endblock coverage_badges -%} + +{# Individual file report #} +{%- block coverage_by_file -%} +{%- if not files -%} +_This PR does not seem to contain any modification to coverable code._ +{%- else -%} +
Click to see where and how coverage changed + + + + +{%- for parent, files_in_folder in groupby(attribute="path.parent") -%} + + + +{%- for file in files_in_folder -%} +{%- set path = file.coverage.path -%} + + + +{#- Statements cell -#} +{{- statements_badge( + path=path, + statements_count=file.coverage.info.num_statements, + previous_statements_count=(file.previous.coverage.info.num_statements if file.previous else None), +) -}} + +{#- Missing cell -#} +{{- missing_lines_badge( + path=path, + statements_count=file.coverage.info.missing_lines, + previous_statements_count=(file.previous.coverage.info.missing_lines if file.previous else None), +) -}} + +{#- Coverage rate -#} +{{- coverage_rate_badge( + path=path, + coverage_rate=file.coverage.info.percent_covered, + previous_coverage_rate=(file.previous.coverage.info.percent_covered if file.previous else None), +) -}} + +{# Coverage of added lines #} +{{- diff_coverage_rate_badge( + path=path, + added_statements_count=(file.diff.added_statements | length if file.diff else None), + covered_statements_count=(file.diff.covered_statements | length if file.diff else None), + percent_covered=(file.diff.percent_covered if file.diff else None) +) -}} + +{# Link to missing lines #} + + +{%- endfor -%} +{%- endfor -%} + + + + + +{#- Statements cell -#} +{{- statements_badge( + path="the whole project", + statements_count=coverage.info.num_statements, + previous_statements_count=(previous_coverage.info.num_statements if previous_coverage else None), +) -}} + +{#- Missing cell -#} +{{- missing_lines_badge( + path="the whole project", + statements_count=coverage.info.missing_lines, + previous_statements_count=(previous_coverage.info.missing_lines if previous_coverage else None), +) -}} + +{#- Coverage rate -#} +{{- coverage_rate_badge( + path="the whole project", + coverage_rate=coverage.info.percent_covered, + previous_coverage_rate=(previous_coverage.info.percent_covered if previous_coverage else None), +) -}} + +{# Coverage of added lines #} +{{- diff_coverage_rate_badge( + path="the whole project", + added_statements_count=(file.diff.added_statements | length if file.diff else None), + covered_statements_count=(file.diff.covered_statements | length if file.diff else None), + percent_covered=(file.diff.percent_covered if file.diff else None) +) -}} + + + + +
FileStatementsMissingCoverageCoverage
(new lines)		Lines missing
  {{ filename }}
  {{ diff_file.path.name }} + +{%- set comma = joiner() -%} +{%- for group in missing_diff_lines.get(path, []) -%} + + +{{- comma() -}} +{{- group.line_start -}} +{%- if group.line_start != group.line_end -%} +- +{{- group.line_end -}} +{%- endif -%} + + +{%- endfor -%} +
Project Total 
+ +{%- endblock coverage_by_file -%} + +This report was generated by [python-coverage-comment-action](https://github.com/py-cov-action/python-coverage-comment-action)
-{%- endif %} -{%- endblock coverage_by_file %} -{{ marker }} + +{{ marker -}} + + +{%- macro statements_badge(path, statements_count, previous_statements_count) -%} +{% if previous_statements_count is not None -%} +{% set statements_diff = statements_countprevious_statements_count - previous_statements_count %} +{% if statements_diff > 0 -%} +{% set text = "This PR adds " ~ statements_diff ~ " to the number of statements in " ~ path ~ ", taking it from " ~ previous_statements_count ~ " to " ~ statements_countprevious_statements_count ~". The number of new statements is at least " ~ statements_diff ~ " but it may be higher if other statements were simultaneously removed." -%} +{% set color = "007ec6" -%} +{% elif statements_diff < 0 -%} +{% set text = "This PR removes " ~ (-statements_diff) ~ " from the number of statements in " ~ path ~ ", taking it from " ~ previous_statements_count ~ " to " ~ statements_countprevious_statements_count ~". The number of deleted statements is at least " ~ (-statements_diff) ~ " but it may be higher if other statements were simultaneously added." -%} +{% set color = "49c2ee" -%} +{% else -%} +{% set text = "This PR doesn't change the number of statements in " ~ path ~ ", which is " ~ statements_countprevious_statements_count ~ ". Either the file was mostly unmodified, or the same number of statements was simultenousely added to and removed from the file." -%} +{% set color = "5d89ba" -%} +{% endif -%} +{% set message = statements_diff %} +{% else -%} +{% set text = "This PR adds " ~ statements_countprevious_statements_count ~ (" statements" | pluralize) ~ " to " ~ path ~ ". The file did not seem to exist on the base branch." -%} +{% set color = "007ec6" -%} +{% set message = statements_countprevious_statements_count %} +{% endif -%} +{{ text }} + +{%- endmacro -%} + +{%- macro missing_lines_badge(path, missing_lines_count, previous_missing_lines_count) -%} +{%- if previous_missing_lines_count is not None -%} +{%- set missing_diff = missing_lines_count - previous_missing_lines_count %} +{%- if missing_diff > 0 -%} +{%- set text = "This PR adds " ~ missing_diff ~ " to the number of statements missing coverage in " ~ path ~ ", taking it from " ~ previous_missing_lines_count ~ " to " ~ missing_lines_count ~ ". This may be the result of adding untested statements and/or removing the tests that were covering existing statements. Also, it's possible that more non-covered statements were added, while other non-covered statements were removed elsewhere in the file." -%} +{%- elif missing_diff < 0 -%} +{%- set text = "This PR removes " ~ (-missing_diff) ~ " from the number of statements missing coverage in " ~ path ~ ", taking it from " ~ previous_missing_lines_count ~ " to " ~ missing_lines_count ~ ". This may be the result of removing untested statements and/or adding tests to cover existing non-covered statements. Also, it's possible that more non-covered statements were removed, while other non-covered statements were added elsewhere in the file." -%} +{%- else -%} +{%- set text = "This PR doesn't change the number of statements missing coverage in " ~ path ~ ", which is " ~ missing_lines_count ~ ". Either the modifications in the file were only to covered lines, or the same number of statements missing coverage was simultenousely added to and removed from the file." -%} +{%- endif -%} +{%- set message = missing_diff -%} +{%- else -%} +{%- set text = "This PR adds " ~ missing_lines_count ~ (" statements" | pluralize) ~ " missing coverage to " ~ path ~ ". The file did not seem to exist on the base branch." -%} +{%- set message = missing_lines_count -%} +{%- endif -%} +{%- set color = message | get_evolution_color(up_is_good=False) -%} +{{ text }} + +{%- endmacro -%} + +{%- macro coverage_rate_badge(path, coverage_rate, previous_coverage_rate) -%} +{%- if previous_coverage_rate is not None -%} +{%- set coverage_diff = coverage_rate - previous_coverage_rate -%} +{%- if coverage_diff > 0 -%} +{%- set text = "This PR adds " ~ (coverage_diff * 100) ~ " percentage points to the coverage rate (number of statements covered by the tests / total number of statements) in " ~ path ~ ", taking it from " ~ previous_coverage_rate | pct ~ " to " ~ coverage_rate | pct ~ "." -%} +{%- elif coverage_diff < 0 -%} +{%- set text = "This PR removes " ~ (-coverage_diff * 100) ~ " percentage points from the coverage rate (number of statements covered by the tests / total number of statements) in " ~ path ~ ", taking it from " ~ previous_coverage_rate | pct ~ " to " ~ coverage_rate | pct ~ "." -%} +{%- else -%} +{%- set text = "This PR doesn't change the coverage rate (number of statements covered by the tests / total number of statements) in " ~ path ~ ", which is " ~ coverage_rate | pct ~ "." -%} +{%- endif -%} +{%- set color = coverage_diff | get_evolution_color(up_is_good=False) -%} +{%- set message = (coverage_diff | pct_delta(precision=0)) -%} +{%- else -%} +{%- set text = "The coverage rate (number of statements covered by the tests / total number of statements) of " ~ path ~ " is " ~ coverage_rate | pct ~ ". The file did not seem to exist on the base branch." -%} +{%- set message = "-" -%} +{%- set color = "grey" -%} +{%- endif -%} +{{ text }} + +{%- endmacro -%} + +{%- macro diff_coverage_rate_badge(path, added_statements_count, covered_statements_count, percent_covered) -%} +{% if added_statements_count -%} +{% set text = "This PR adds " ~ (added_statements_count) ~ " statements to " ~ path ~ ", " ~ (percent_covered) ~ " of which are covered, the coverage rate on the diff is " ~ percent_covered | pct ~ "." -%} +{% set label = (percent_covered | pct(precision=0)) -%} +{% set message = "(" ~ percent_covered ~ "/" ~ added_statements_count ~ ")" -%} +{%- set color = (percent_covered | get_badge_color()) -%} +{% else -%} +{% set text = "This PR does not seem to add statements to " ~ path ~ "." -%} +{% set label = "-" -%} +{%- set color = "grey" -%} +{% set message = "-" -%} +{% endif -%} +{{ text }} + +{%- endmacro -%} diff --git a/pyproject.toml b/pyproject.toml index fa13cd9d..978ddc30 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -64,6 +64,7 @@ extend-select = [ "W", # pycodestyle warnings "RUF", # ruff ] +fixable = ["ALL"] extend-ignore = [ "E501", # line too long ] diff --git a/tests/conftest.py b/tests/conftest.py index ccf67e20..e7700c96 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -245,7 +245,7 @@ def diff_coverage_obj(): pathlib.Path("codebase/code.py"): coverage_module.FileDiffCoverage( path=pathlib.Path("codebase/code.py"), percent_covered=decimal.Decimal("0.8"), - missing_lines=[3, 7, 8, 9, 12], + missing_statements=[3, 7, 8, 9, 12], added_lines=[3, 4, 5, 6, 7, 8, 9, 12], ) }, @@ -263,13 +263,13 @@ def diff_coverage_obj_many_missing_lines(): pathlib.Path("codebase/code.py"): coverage_module.FileDiffCoverage( path=pathlib.Path("codebase/code.py"), percent_covered=decimal.Decimal("0.8"), - missing_lines=[7, 9], + missing_statements=[7, 9], added_lines=[7, 8, 9], ), pathlib.Path("codebase/main.py"): coverage_module.FileDiffCoverage( path=pathlib.Path("codebase/code.py"), percent_covered=decimal.Decimal("0.8"), - missing_lines=[1, 2, 8, 17], + missing_statements=[1, 2, 8, 17], added_lines=[1, 2, 3, 4, 5, 6, 7, 8, 17], ), }, @@ -321,9 +321,7 @@ def request(self, method, path, **kwargs): **response_kwargs, request=httpx.Request(method=method, url=path), ) - assert ( - False - ), f"No response found for kwargs {request_kwargs}\nExpected answers are {self.responses}" + assert False, f"No response found for kwargs {request_kwargs}\nExpected answers are {self.responses}" def __getattr__(self, value): if value in ["get", "post", "patch", "delete", "put"]: @@ -412,9 +410,7 @@ def command(self, command, *args, env=None): call = self.expected_calls[0] exp_args, exp_env, exit_code, stdout = call if not (args == exp_args and (not exp_env or exp_env == env)): - assert ( - False - ), f"Expected command is not `{args}` with env {env}\nExpected command is {self.expected_calls[0]}" + assert False, f"Expected command is not `{args}` with env {env}\nExpected command is {self.expected_calls[0]}" self.expected_calls.pop(0) if exit_code == 0: diff --git a/tests/unit/test_coverage.py b/tests/unit/test_coverage.py index 65ebed38..53fe6000 100644 --- a/tests/unit/test_coverage.py +++ b/tests/unit/test_coverage.py @@ -143,7 +143,7 @@ def test_generate_coverage_markdown(mocker): pathlib.Path("codebase/code.py"): coverage.FileDiffCoverage( path=pathlib.Path("codebase/code.py"), percent_covered=decimal.Decimal("0.5"), - missing_lines=[3], + missing_statements=[3], added_lines=[1, 3], ) }, @@ -180,7 +180,7 @@ def test_generate_coverage_markdown(mocker): pathlib.Path("codebase/code.py"): coverage.FileDiffCoverage( path=pathlib.Path("codebase/code.py"), percent_covered=decimal.Decimal("1"), - missing_lines=[], + missing_statements=[], added_lines=[4, 5, 6], ) }, @@ -213,13 +213,13 @@ def test_generate_coverage_markdown(mocker): pathlib.Path("codebase/code.py"): coverage.FileDiffCoverage( path=pathlib.Path("codebase/code.py"), percent_covered=decimal.Decimal("1"), - missing_lines=[], + missing_statements=[], added_lines=[4, 5, 6], ), pathlib.Path("codebase/other.py"): coverage.FileDiffCoverage( path=pathlib.Path("codebase/other.py"), percent_covered=decimal.Decimal("0.5"), - missing_lines=[13], + missing_statements=[13], added_lines=[10, 13], ), }, diff --git a/tests/unit/test_annotations.py b/tests/unit/test_diff_grouper.py similarity index 91% rename from tests/unit/test_annotations.py rename to tests/unit/test_diff_grouper.py index 45cb465a..23d6f296 100644 --- a/tests/unit/test_annotations.py +++ b/tests/unit/test_diff_grouper.py @@ -4,7 +4,7 @@ import pytest -from coverage_comment import annotations +from coverage_comment import diff_grouper @pytest.mark.parametrize( @@ -49,19 +49,19 @@ ], ) def test_compute_contiguous_groups(values, separators, joiners, expected): - result = annotations.compute_contiguous_groups( + result = diff_grouper.compute_contiguous_groups( values=values, separators=separators, joiners=joiners ) assert result == expected def test_group_annotations(coverage_obj, diff_coverage_obj): - result = annotations.group_annotations( + result = diff_grouper.get_diff_missing_groups( coverage=coverage_obj, diff_coverage=diff_coverage_obj ) assert list(result) == [ - annotations.Annotation( + diff_grouper.Group( file=pathlib.Path("codebase/code.py"), line_start=7, line_end=9 ) ] @@ -70,12 +70,12 @@ def test_group_annotations(coverage_obj, diff_coverage_obj): def test_group_annotations_more_files( coverage_obj, diff_coverage_obj_many_missing_lines ): - result = annotations.group_annotations( + result = diff_grouper.get_diff_missing_groups( coverage=coverage_obj, diff_coverage=diff_coverage_obj_many_missing_lines ) assert list(result) == [ - annotations.Annotation( + diff_grouper.Group( file=pathlib.Path("codebase/code.py"), line_start=7, line_end=9 ) ] diff --git a/tests/unit/test_template.py b/tests/unit/test_template.py index 12329413..0aed8149 100644 --- a/tests/unit/test_template.py +++ b/tests/unit/test_template.py @@ -147,13 +147,13 @@ def test_template_full(): pathlib.Path("codebase/code.py"): coverage.FileDiffCoverage( path=pathlib.Path("codebase/code.py"), percent_covered=decimal.Decimal("0.5"), - missing_lines=[12, 13, 14, 22], + missing_statements=[12, 13, 14, 22], added_lines=[12, 13, 14, 15, 16, 22], ), pathlib.Path("codebase/other.py"): coverage.FileDiffCoverage( path=pathlib.Path("codebase/other.py"), percent_covered=decimal.Decimal("1"), - missing_lines=[], + missing_statements=[], added_lines=[4, 5, 6], ), },