Skip to content

Commit

Permalink
1303 team plan pr comment (#341)
Browse files Browse the repository at this point in the history
* Add PR comment for team plan
  • Loading branch information
adrian-codecov authored Mar 28, 2024
1 parent d2be8d1 commit d1e7429
Show file tree
Hide file tree
Showing 3 changed files with 283 additions and 1 deletion.
53 changes: 52 additions & 1 deletion services/notification/notifiers/mixins/message/__init__.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import logging
from typing import Callable
from typing import Callable, List

from shared.reports.resources import Report, ReportTotals
from shared.validation.helpers import LayoutStructure

from database.models.core import Owner
from helpers.environment import is_enterprise
from helpers.metrics import metrics
from services.billing import BillingPlan
from services.comparison import ComparisonProxy
from services.notification.notifiers.mixins.message.helpers import (
should_message_be_compact,
Expand All @@ -14,6 +16,7 @@
NullSectionWriter,
get_section_class_from_layout_name,
)
from services.notification.notifiers.mixins.message.writers import TeamPlanWriter
from services.urls import get_commit_url, get_pull_url
from services.yaml.reader import read_yaml_field

Expand Down Expand Up @@ -69,6 +72,23 @@ async def create_message(
f'## [Codecov]({links["pull"]}?dropdown=coverage&src=pr&el=h1) Report',
]

repo = comparison.head.commit.repository
owner: Owner = repo.owner

# Separate PR comment based on plan that can't/won't be tweaked by codecov.yml settings
if (
owner.plan == BillingPlan.team_monthly.value
or owner.plan == BillingPlan.team_yearly.value
):
return self._team_plan_notification(
comparison=comparison,
message=message,
diff=diff,
settings=settings,
links=links,
current_yaml=current_yaml,
)

write = message.append
# note: since we're using append, calling write("") will add a newline to the message

Expand Down Expand Up @@ -176,6 +196,37 @@ async def _possibly_write_gh_app_login_announcement(
write(f":exclamation: {message_to_display}")
write("")

def _team_plan_notification(
self,
comparison: ComparisonProxy,
message: List[str],
diff,
settings,
links,
current_yaml,
) -> List[str]:
writer_class = TeamPlanWriter()

with metrics.timer(
f"worker.services.notifications.notifiers.comment.section.{writer_class.name}"
):
# Settings here enable failed tests results for now as a new product
for line in writer_class.header_lines(
comparison=comparison, diff=diff, settings=settings
):
message.append(line)
for line in writer_class.middle_lines(
comparison=comparison,
diff=diff,
links=links,
current_yaml=current_yaml,
):
message.append(line)
for line in writer_class.footer_lines():
message.append(line)

return message

async def write_section_to_msg(
self, comparison, changes, diff, links, write, section_writer, behind_by=None
):
Expand Down
143 changes: 143 additions & 0 deletions services/notification/notifiers/mixins/message/writers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
import logging
from base64 import b64encode
from decimal import Decimal
from typing import List

from shared.reports.resources import Report

from helpers.reports import get_totals_from_file_in_reports
from services.comparison import ComparisonProxy
from services.notification.notifiers.mixins.message.helpers import (
ellipsis,
escape_markdown,
make_patch_only_metrics,
)

log = logging.getLogger(__name__)

# Unlike sections.py, this is an alternative take for creating messages based functionality.
# This is a plan specific section, so it doesn't adhere to the settings/yaml configurations
# like other writers do, hence the new file
class TeamPlanWriter:
@property
def name(self):
return self.__class__.__name__

def header_lines(self, comparison: ComparisonProxy, diff, settings) -> List[str]:
lines = []

head_report = comparison.head.report
diff_totals = head_report.apply_diff(diff)

if diff_totals:
misses_and_partials = diff_totals.misses + diff_totals.partials
patch_coverage = diff_totals.coverage
else:
misses_and_partials = None
patch_coverage = None
if misses_and_partials:
lines.append(
f"Attention: Patch coverage is `{patch_coverage}%` with `{misses_and_partials} lines` in your changes are missing coverage. Please review."
)
else:
lines.append(
"All modified and coverable lines are covered by tests :white_check_mark:"
)

hide_project_coverage = settings.get("hide_project_coverage", False)
if hide_project_coverage:
if comparison.all_tests_passed():
lines.append("")
lines.append(
":white_check_mark: All tests successful. No failed tests found :relaxed:"
)

return lines

def middle_lines(
self, comparison: ComparisonProxy, diff, links, current_yaml
) -> List[str]:
lines = []

# create list of files changed in diff
base_report = comparison.project_coverage_base.report
head_report = comparison.head.report
if base_report is None:
base_report = Report()
files_in_diff = [
(
_diff["type"],
path,
make_patch_only_metrics(
get_totals_from_file_in_reports(base_report, path) or False,
get_totals_from_file_in_reports(head_report, path) or False,
_diff["totals"],
# show_complexity defaulted to none
None,
current_yaml,
links["pull"],
),
int(_diff["totals"].misses + _diff["totals"].partials),
)
for path, _diff in (diff["files"] if diff else {}).items()
if _diff.get("totals")
]

if files_in_diff:
table_header = "| Patch % | Lines |"
table_layout = "|---|---|---|"

# get limit of results to show
limit = 10
mentioned = []

def tree_cell(typ, path, metrics, _=None):
if path not in mentioned:
# mentioned: for files that are in diff and changes
mentioned.append(path)
return "| {rm}[{path}]({compare}?src=pr&el=tree#diff-{hash}){rm} {metrics}".format(
rm="~~" if typ == "deleted" else "",
path=escape_markdown(ellipsis(path, 50, False)),
compare=links["pull"],
hash=b64encode(path.encode()).decode(),
metrics=metrics,
)

remaining_files = 0
printed_files = 0
changed_files = sorted(
files_in_diff, key=lambda a: a[3] or Decimal("0"), reverse=True
)
changed_files_with_missing_lines = [f for f in changed_files if f[3] > 0]
if changed_files_with_missing_lines:
lines.append(
"| [Files]({0}?dropdown=coverage&src=pr&el=tree) {1}".format(
links["pull"], table_header
)
)
lines.append(table_layout)
for file in changed_files_with_missing_lines:
if printed_files == limit:
remaining_files += 1
else:
printed_files += 1
lines.append(tree_cell(file[0], file[1], file[2]))
if remaining_files:
lines.append(
"| ... and [{n} more]({href}?src=pr&el=tree-more) | |".format(
n=remaining_files, href=links["pull"]
)
)

return lines

def footer_lines(self) -> List[str]:
lines = []
lines.append("")
lines.append(
":loudspeaker: Thoughts on this report? [Let us know!]({0})".format(
"https://about.codecov.io/pull-request-comment-report/"
)
)

return lines
88 changes: 88 additions & 0 deletions services/notification/notifiers/tests/unit/test_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -4773,6 +4773,94 @@ async def test_build_message_head_and_pull_head_differ_with_components(
assert exp == res
assert result == expected_result

@pytest.mark.asyncio
async def test_build_message_team_plan_customer_missing_lines(
self,
dbsession,
mock_configuration,
mock_repo_provider,
sample_comparison_head_and_pull_head_differ,
):
mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
comparison = sample_comparison_head_and_pull_head_differ
comparison.repository_service.service = "github"
# relevant part of this test
comparison.head.commit.repository.owner.plan = "users-teamm"
notifier = CommentNotifier(
repository=comparison.head.commit.repository,
title="title",
notifier_yaml_settings={
# Irrelevant but they don't overwrite Owner's plan
"layout": "newheader, reach, diff, flags, components, newfooter",
"hide_project_coverage": True,
},
notifier_site_settings=True,
current_yaml={
# Irrelevant but here to ensure they don't overwrite Owner's plan
"component_management": {
"individual_components": [
{"component_id": "go_files", "paths": [r".*\.go"]},
{"component_id": "unit_flags", "flag_regexes": [r"unit.*"]},
]
}
},
)

pull = comparison.pull
repository = sample_comparison_head_and_pull_head_differ.head.commit.repository
result = await notifier.build_message(comparison)
expected_result = [
f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report",
"Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.",
f"| [Files](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=tree) | Patch % | Lines |",
"|---|---|---|",
f"| [file\\_1.go](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree#diff-ZmlsZV8xLmdv) | 66.67% | [1 Missing :warning: ](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree) |",
"",
":loudspeaker: Thoughts on this report? [Let us know!](https://about.codecov.io/pull-request-comment-report/)",
]
assert result == expected_result

@pytest.mark.asyncio
async def test_build_message_team_plan_customer_all_lines_covered(
self,
dbsession,
mock_configuration,
mock_repo_provider,
sample_comparison_coverage_carriedforward,
):
mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
sample_comparison_coverage_carriedforward.context = NotificationContext(
all_tests_passed=True
)
comparison = sample_comparison_coverage_carriedforward
comparison.repository_service.service = "github"
# relevant part of this test
comparison.head.commit.repository.owner.plan = "users-teamm"
notifier = CommentNotifier(
repository=comparison.head.commit.repository,
title="title",
notifier_yaml_settings={
# Irrelevant but they don't overwrite Owner's plan
"layout": "newheader, reach, diff, flags, components, newfooter",
"hide_project_coverage": True,
},
notifier_site_settings=True,
current_yaml={},
)
pull = comparison.pull
repository = sample_comparison_coverage_carriedforward.head.commit.repository
result = await notifier.build_message(comparison)

expected_result = [
f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?dropdown=coverage&src=pr&el=h1) Report",
"All modified and coverable lines are covered by tests :white_check_mark:",
"",
":white_check_mark: All tests successful. No failed tests found :relaxed:",
"",
":loudspeaker: Thoughts on this report? [Let us know!](https://about.codecov.io/pull-request-comment-report/)",
]
assert result == expected_result

@pytest.mark.asyncio
async def test_build_message_no_patch_or_proj_change(
self,
Expand Down

0 comments on commit d1e7429

Please sign in to comment.