From d1e742977fb7a6591a4e972c9aa808e94f3ca4f7 Mon Sep 17 00:00:00 2001 From: Adrian Date: Thu, 28 Mar 2024 10:10:42 -0400 Subject: [PATCH] 1303 team plan pr comment (#341) * Add PR comment for team plan --- .../notifiers/mixins/message/__init__.py | 53 ++++++- .../notifiers/mixins/message/writers.py | 143 ++++++++++++++++++ .../notifiers/tests/unit/test_comment.py | 88 +++++++++++ 3 files changed, 283 insertions(+), 1 deletion(-) create mode 100644 services/notification/notifiers/mixins/message/writers.py diff --git a/services/notification/notifiers/mixins/message/__init__.py b/services/notification/notifiers/mixins/message/__init__.py index 6724e9178..fb393f49b 100644 --- a/services/notification/notifiers/mixins/message/__init__.py +++ b/services/notification/notifiers/mixins/message/__init__.py @@ -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, @@ -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 @@ -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 @@ -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 ): diff --git a/services/notification/notifiers/mixins/message/writers.py b/services/notification/notifiers/mixins/message/writers.py new file mode 100644 index 000000000..b54f34461 --- /dev/null +++ b/services/notification/notifiers/mixins/message/writers.py @@ -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 diff --git a/services/notification/notifiers/tests/unit/test_comment.py b/services/notification/notifiers/tests/unit/test_comment.py index afadfc508..19d5b9d91 100644 --- a/services/notification/notifiers/tests/unit/test_comment.py +++ b/services/notification/notifiers/tests/unit/test_comment.py @@ -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,