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

show patch coverage only for changed files, and don't show indirect changed files #60

Merged
merged 3 commits into from
Aug 15, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions services/notification/notifiers/mixins/message/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,53 @@
return "".join(("|", coverage, complexity, icon))


def make_patch_only_metrics(before, after, relative, show_complexity, yaml):
if after is None:
# e.g. missing flags
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for these helpful comments

coverage = " `?` |"

Check warning on line 122 in services/notification/notifiers/mixins/message/helpers.py

View check run for this annotation

Codecov / codecov/patch

services/notification/notifiers/mixins/message/helpers.py#L122

Added line #L122 was not covered by tests

elif after is False:
# e.g. file deleted
coverage = " |"

Check warning on line 126 in services/notification/notifiers/mixins/message/helpers.py

View check run for this annotation

Codecov / codecov/patch

services/notification/notifiers/mixins/message/helpers.py#L126

Added line #L126 was not covered by tests

else:
layout = " `{relative}` |"
coverage = layout.format(
relative=format_number_to_str(
yaml, relative.coverage if relative else 0, style="{0}%", if_null="\xF8"
),
)
return "".join(("|", coverage))


def get_metrics_function(hide_project_coverage):
if hide_project_coverage:
metrics = make_patch_only_metrics
else:
metrics = make_metrics
return metrics


def get_table_header(hide_project_coverage, show_complexity):
if not hide_project_coverage:
table_header = (
"| Coverage \u0394 |"
+ (" Complexity \u0394 |" if show_complexity else "")
+ " |"
)
else:
table_header = "| Coverage |"

return table_header


def get_table_layout(hide_project_coverage, show_complexity):
if hide_project_coverage:
return "|---|---|"
else:
return "|---|---|---|" + ("---|" if show_complexity else "")


def format_number_to_str(
yml, value, if_zero=None, if_null=None, plus=False, style="{0}"
) -> str:
Expand Down
17 changes: 9 additions & 8 deletions services/notification/notifiers/mixins/message/sections.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
diff_to_string,
ellipsis,
escape_markdown,
get_metrics_function,
get_table_header,
get_table_layout,
make_metrics,
sort_by_importance,
)
Expand Down Expand Up @@ -464,11 +467,13 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non
head_report = comparison.head.report
if base_report is None:
base_report = Report()
hide_project_coverage = self.settings.get("hide_project_coverage", False)
make_metrics_fn = get_metrics_function(hide_project_coverage)
files_in_diff = [
(
_diff["type"],
path,
make_metrics(
make_metrics_fn(
get_totals_from_file_in_reports(base_report, path) or False,
get_totals_from_file_in_reports(head_report, path) or False,
_diff["totals"],
Expand All @@ -487,12 +492,8 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non
c.path for c in changes or []
)
if files_in_diff:
table_header = (
"| Coverage \u0394 |"
+ (" Complexity \u0394 |" if self.show_complexity else "")
+ " |"
)
table_layout = "|---|---|---|" + ("---|" if self.show_complexity else "")
table_header = get_table_header(hide_project_coverage, self.show_complexity)
table_layout = get_table_layout(hide_project_coverage, self.show_complexity)
# add table headers
yield (
"| [Files Changed]({0}?src=pr&el=tree) {1}".format(
Expand Down Expand Up @@ -539,7 +540,7 @@ def tree_cell(typ, path, metrics, _=None):
)
)

if changes:
if changes and not hide_project_coverage:
len_changes_not_in_diff = len(all_files or []) - len(files_in_diff or [])
if files_in_diff and len_changes_not_in_diff > 0:
yield ("")
Expand Down
8 changes: 3 additions & 5 deletions services/notification/notifiers/tests/unit/test_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -4163,11 +4163,9 @@ async def test_build_message_no_project_coverage(
f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report",
f"Patch coverage is **`66.67%`** of modified lines.",
f"",
f"| [Files Changed](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree) | Coverage Δ | Complexity Δ | |",
f"|---|---|---|---|",
f"| [file\\_1.go](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree#diff-ZmlsZV8xLmdv) | `62.50% <66.67%> (+12.50%)` | `10.00 <0.00> (-1.00)` | :arrow_up: |",
f"",
f"... and [1 file with indirect coverage changes](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/indirect-changes?src=pr&el=tree-more)",
f"| [Files Changed](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree) | Coverage |",
f"|---|---|",
f"| [file\\_1.go](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree#diff-ZmlsZV8xLmdv) | `66.67%` |",
f"",
f"",
f"[:umbrella: View full report in Codecov by Sentry](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=continue). ",
Expand Down
Loading