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

feat: customize comment based on status #694

Merged
merged 1 commit into from
Sep 9, 2024
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
12 changes: 12 additions & 0 deletions services/bundle_analysis/notify/contexts/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from enum import Enum, auto
from functools import cached_property
from typing import Generic, Literal, Self, TypeVar

Expand Down Expand Up @@ -31,6 +32,17 @@
pass


class CommitStatusLevel(Enum):
INFO = auto()
WARNING = auto()
ERROR = auto()

def to_str(self) -> Literal["success"] | Literal["failure"]:
if self.value == "ERROR":
return "failure"

Check warning on line 42 in services/bundle_analysis/notify/contexts/__init__.py

View check run for this annotation

Codecov Notifications / codecov/patch

services/bundle_analysis/notify/contexts/__init__.py#L42

Added line #L42 was not covered by tests
return "success"


class NotificationContextField(Generic[T]):
"""NotificationContextField is a descriptor akin to a Django model field.
If you create one as a class member named `foo`, it will define the behavior to get and set an instance member named `foo`.
Expand Down
21 changes: 21 additions & 0 deletions services/bundle_analysis/notify/contexts/comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
)
from services.bundle_analysis.notify.contexts import (
BaseBundleAnalysisNotificationContext,
CommitStatusLevel,
NotificationContextBuilder,
NotificationContextBuildError,
NotificationContextField,
Expand All @@ -45,6 +46,9 @@ class BundleAnalysisPRCommentNotificationContext(BaseBundleAnalysisNotificationC
bundle_analysis_comparison: BundleAnalysisComparison = NotificationContextField[
BundleAnalysisComparison
]()
commit_status_level: CommitStatusLevel = NotificationContextField[
CommitStatusLevel
]()
should_use_upgrade_comment: bool


Expand Down Expand Up @@ -183,13 +187,30 @@ def evaluate_should_use_upgrade_message(self) -> Self:
self._notification_context.should_use_upgrade_comment = False
return self

def load_commit_status_level(self) -> Self:
bundle_analysis_comparison = (
self._notification_context.bundle_analysis_comparison
)
user_config = self._notification_context.user_config

if is_bundle_change_within_bundle_threshold(
bundle_analysis_comparison, user_config.warning_threshold
):
self._notification_context.commit_status_level = CommitStatusLevel.INFO
elif user_config.status_level == "informational":
self._notification_context.commit_status_level = CommitStatusLevel.WARNING
else:
self._notification_context.commit_status_level = CommitStatusLevel.ERROR
return self

def build_context(self) -> Self:
super().build_context()
async_to_sync(self.load_enriched_pull)()
return (
self.load_bundle_comparison()
.evaluate_has_enough_changes()
.evaluate_should_use_upgrade_message()
.load_commit_status_level()
)

def get_result(self) -> BundleAnalysisPRCommentNotificationContext:
Expand Down
15 changes: 1 addition & 14 deletions services/bundle_analysis/notify/contexts/commit_status.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
from enum import Enum, auto
from typing import Literal

import sentry_sdk
from asgiref.sync import async_to_sync
from shared.bundle_analysis import (
Expand All @@ -19,6 +16,7 @@
)
from services.bundle_analysis.notify.contexts import (
BaseBundleAnalysisNotificationContext,
CommitStatusLevel,
NotificationContextBuilder,
NotificationContextBuildError,
NotificationContextField,
Expand All @@ -34,17 +32,6 @@
from services.urls import get_bundle_analysis_pull_url, get_commit_url


class CommitStatusLevel(Enum):
INFO = auto()
WARNING = auto()
ERROR = auto()

def to_str(self) -> Literal["success"] | Literal["failure"]:
if self.value == "ERROR":
return "failure"
return "success"


class CommitStatusNotificationContext(BaseBundleAnalysisNotificationContext):
notification_type = NotificationType.COMMIT_STATUS

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ def test_initialize_from_context(self, dbsession, mocker):
with pytest.raises(ContextNotLoadedError):
other_context.bundle_analysis_comparison

fake_comparison = MagicMock(name="fake_comparison")
fake_comparison = MagicMock(name="fake_comparison", percentage_delta=10.0)
mocker.patch.object(
ComparisonLoader, "get_comparison", return_value=fake_comparison
)
Expand Down
16 changes: 15 additions & 1 deletion services/bundle_analysis/notify/messages/comment.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import logging
from typing import TypedDict
from typing import Literal, TypedDict

import sentry_sdk
from django.template import loader
Expand Down Expand Up @@ -36,6 +36,9 @@ class BundleCommentTemplateContext(TypedDict):
pull_url: str
total_size_delta: int
total_size_readable: str
total_percentage: str
status_level: Literal["INFO"] | Literal["WARNING"] | Literal["ERROR"]
warning_threshold_readable: str
bundle_rows: list[BundleRow]
has_cached_bundles: bool

Expand All @@ -62,12 +65,23 @@ def build_default_message(
template = loader.get_template("bundle_analysis_notify/bundle_comment.md")
total_size_delta = context.bundle_analysis_comparison.total_size_delta
bundle_rows = self._create_bundle_rows(context.bundle_analysis_comparison)
warning_threshold = context.user_config.warning_threshold
if warning_threshold.type == "absolute":
warning_threshold_readable = bytes_readable(warning_threshold.threshold)
else:
warning_threshold_readable = str(round(warning_threshold.threshold)) + "%"
context = BundleCommentTemplateContext(
has_cached=any(row["is_cached"] for row in bundle_rows),
bundle_rows=bundle_rows,
pull_url=get_bundle_analysis_pull_url(pull=context.pull.database_pull),
total_size_delta=total_size_delta,
status_level=context.commit_status_level.name,
total_percentage=str(
round(context.bundle_analysis_comparison.percentage_delta, 2)
)
+ "%",
total_size_readable=bytes_readable(total_size_delta),
warning_threshold_readable=warning_threshold_readable,
)
return template.render(context)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
## [Bundle]({{pull_url}}) Report
{% if total_size_delta == 0 %}
Bundle size has no change :white_check_mark:
{% elif total_size_delta > 0 %}
Changes will increase total bundle size by {{total_size_readable}} :arrow_up:
{% else %}
Changes will decrease total bundle size by {{total_size_readable}} :arrow_down:
{% if status_level == "ERROR" %}:x: Check failed: c{% else %}C{% endif %}hanges will {% if total_size_delta > 0 %}increase{% else %}decrease{% endif %} total bundle size by {{total_size_readable}} ({{total_percentage}}) {% if total_size_delta > 0 %}:arrow_up:{% else %}:arrow_down:{% endif %}{% if status_level == "WARNING" %}:warning:, exceeding the [configured](https://docs.codecov.com/docs/javascript-bundle-analysis#main-features) threshold of {{warning_threshold_readable}}.{% elif status_level == "ERROR" %}, **exceeding** the [configured](https://docs.codecov.com/docs/javascript-bundle-analysis#main-features) threshold of {{warning_threshold_readable}}.{% else %}. This is within the [configured](https://docs.codecov.com/docs/javascript-bundle-analysis#main-features) threshold :white_check_mark:{% endif %}
{% endif %}
{% if bundle_rows %}{% include "bundle_analysis_notify/bundle_table.md" %}{% if has_cached %}

ℹ️ *Bundle size includes cached data from a previous commit
{%endif%}{% endif %}
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
| Bundle name | Size | Change |
{% if status_level == "INFO" %}<details><summary>Detailed changes</summary>

{% endif %}| Bundle name | Size | Change |
| ----------- | ---- | ------ |{% for bundle_row in bundle_rows %}
| {{bundle_row.bundle_name}}{% if bundle_row.is_cached %}*{% endif %} | {{bundle_row.bundle_size}} | {{bundle_row.change_size_readable}} {{bundle_row.change_icon}} |{% endfor %}
| {{bundle_row.bundle_name}}{% if bundle_row.is_cached %}*{% endif %} | {{bundle_row.bundle_size}} | {{bundle_row.change_size_readable}} {{bundle_row.change_icon}} |{% endfor %}{% if status_level == "INFO" %}

</details>{% endif %}
122 changes: 17 additions & 105 deletions services/bundle_analysis/notify/messages/tests/test_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
from unittest.mock import AsyncMock, MagicMock

import pytest
from django.template import loader
from shared.torngit.exceptions import TorngitClientError
from shared.typings.torngit import TorngitInstanceData
from shared.validation.types import BundleThreshold
Expand All @@ -20,102 +19,14 @@
BundleAnalysisPRCommentContextBuilder,
BundleAnalysisPRCommentNotificationContext,
)
from services.bundle_analysis.notify.helpers import bytes_readable
from services.bundle_analysis.notify.messages.comment import (
BundleAnalysisCommentMarkdownStrategy,
BundleCommentTemplateContext,
BundleRow,
)
from services.bundle_analysis.notify.types import NotificationUserConfig
from services.notification.notifiers.base import NotificationResult


class TestCommentMesage:
@pytest.mark.parametrize(
"total_size_delta, summary_line",
[
pytest.param(
100,
"Changes will increase total bundle size by 100 bytes :arrow_up:",
id="increase_100b",
),
pytest.param(
1234,
"Changes will increase total bundle size by 1.23kB :arrow_up:",
id="increase_1.23kB",
),
pytest.param(
1e6 + 500,
"Changes will increase total bundle size by 1.0MB :arrow_up:",
id="increase_1MB",
),
pytest.param(
0, "Bundle size has no change :white_check_mark:", id="no_change"
),
pytest.param(
-100,
"Changes will decrease total bundle size by 100 bytes :arrow_down:",
id="decrease_100b",
),
pytest.param(
-1234,
"Changes will decrease total bundle size by 1.23kB :arrow_down:",
id="decrease_1.23kB",
),
pytest.param(
-1e6 - 500,
"Changes will decrease total bundle size by 1.0MB :arrow_down:",
id="decrease_1MB",
),
],
)
def test_summary_change_line_template(self, total_size_delta, summary_line):
template = loader.get_template("bundle_analysis_notify/bundle_comment.md")
context = BundleCommentTemplateContext(
pull_url="example.url",
bundle_rows=[],
total_size_delta=total_size_delta,
total_size_readable=bytes_readable(total_size_delta),
)
expected = (
"## [Bundle](example.url) Report\n" + "\n" + summary_line + "\n" + "\n"
)
assert template.render(context) == expected

def test_bundle_change_row_template(self):
template = loader.get_template("bundle_analysis_notify/bundle_table.md")
context = {
"bundle_rows": [
BundleRow(
bundle_name="@test/increase",
bundle_size="50kB",
change_size_readable="100 bytes",
change_icon=":arrow_up:",
),
BundleRow(
bundle_name="@test/decrease",
bundle_size="50kB",
change_size_readable="100 bytes",
change_icon=":arrow_down:",
),
BundleRow(
bundle_name="@test/removed",
bundle_size="(removed)",
change_size_readable="100 bytes",
change_icon=":arrow_down:",
),
]
}
expected = (
"| Bundle name | Size | Change |"
+ "\n| ----------- | ---- | ------ |"
+ "\n| @test/increase | 50kB | 100 bytes :arrow_up: |"
+ "\n| @test/decrease | 50kB | 100 bytes :arrow_down: |"
+ "\n| @test/removed | (removed) | 100 bytes :arrow_down: |"
+ "\n"
)
assert template.render(context) == expected

def test_build_message_from_samples(self, dbsession, mocker, mock_storage):
head_commit, base_commit = get_commit_pair(dbsession)
repository = head_commit.repository
Expand All @@ -141,24 +52,25 @@ def test_build_message_from_samples(self, dbsession, mocker, mock_storage):
)
context = builder.build_context().get_result()
message = BundleAnalysisCommentMarkdownStrategy().build_message(context)
assert (
message
== """## [Bundle](https://app.codecov.io/gh/{owner}/{repo}/pull/{pullid}?dropdown=bundle) Report
assert message == dedent("""\
## [Bundle](https://app.codecov.io/gh/{owner}/{repo}/pull/{pullid}?dropdown=bundle) Report

Changes will decrease total bundle size by 372.56kB :arrow_down:
Changes will decrease total bundle size by 372.56kB (-48.89%) :arrow_down:. This is within the [configured](https://docs.codecov.com/docs/javascript-bundle-analysis#main-features) threshold :white_check_mark:

| Bundle name | Size | Change |
| ----------- | ---- | ------ |
| @codecov/sveltekit-plugin-esm | 1.1kB | 188 bytes :arrow_up: |
| @codecov/rollup-plugin-esm | 1.32kB | 1.01kB :arrow_down: |
| @codecov/bundler-plugin-core-esm | 8.2kB | 30.02kB :arrow_down: |
| @codecov/bundler-plugin-core-cjs | 43.32kB | 611 bytes :arrow_up: |
| @codecov/example-next-app-server-cjs | (removed) | 342.32kB :arrow_down: |
""".format(
pullid=enriched_pull.database_pull.pullid,
owner=head_commit.repository.owner.username,
repo=head_commit.repository.name,
)
<details><summary>Detailed changes</summary>

| Bundle name | Size | Change |
| ----------- | ---- | ------ |
| @codecov/sveltekit-plugin-esm | 1.1kB | 188 bytes :arrow_up: |
| @codecov/rollup-plugin-esm | 1.32kB | 1.01kB :arrow_down: |
| @codecov/bundler-plugin-core-esm | 8.2kB | 30.02kB :arrow_down: |
| @codecov/bundler-plugin-core-cjs | 43.32kB | 611 bytes :arrow_up: |
| @codecov/example-next-app-server-cjs | (removed) | 342.32kB :arrow_down: |

</details>""").format(
pullid=enriched_pull.database_pull.pullid,
owner=head_commit.repository.owner.username,
repo=head_commit.repository.name,
)

def _setup_send_message_tests(
Expand Down
Loading
Loading