Skip to content
This repository was archived by the owner on May 5, 2025. It is now read-only.

Commit 5e92d54

Browse files
nora-shapnora-shap
authored
Failed check and status messaging in comment, part V (#1044)
1 parent 5f3494b commit 5e92d54

File tree

2 files changed

+110
-11
lines changed

2 files changed

+110
-11
lines changed

‎services/notification/notifiers/mixins/status.py

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,23 @@ class StatusResult(TypedDict):
2727

2828
CUSTOM_TARGET_TEXT_PATCH_KEY = "custom_target_helper_text_patch"
2929
CUSTOM_TARGET_TEXT_PROJECT_KEY = "custom_target_helper_text_project"
30+
CUSTOM_RCB_INDIRECT_CHANGES_KEY = "custom_rcb_indirect_changes_helper_text"
3031
CUSTOM_TARGET_TEXT_VALUE = (
3132
"Your {context} {notification_type} has failed because the {point_of_comparison} coverage ({coverage}%) is below the target coverage ({target}%). "
3233
"You can increase the {point_of_comparison} coverage or adjust the "
3334
"[target](https://docs.codecov.com/docs/commit-status#target) coverage."
3435
)
36+
CUSTOM_RCB_INDIRECT_CHANGES_VALUE = (
37+
"Your {context} {notification_type} has failed because you have indirect coverage changes. "
38+
"Learn more about [Unexpected Coverage Changes](https://docs.codecov.com/docs/unexpected-coverage-changes) "
39+
"and [reasons for indirect coverage changes](https://docs.codecov.com/docs/unexpected-coverage-changes#reasons-for-indirect-changes)."
40+
)
3541

3642

3743
HELPER_TEXT_MAP = {
3844
CUSTOM_TARGET_TEXT_PATCH_KEY: CUSTOM_TARGET_TEXT_VALUE,
3945
CUSTOM_TARGET_TEXT_PROJECT_KEY: CUSTOM_TARGET_TEXT_VALUE,
46+
CUSTOM_RCB_INDIRECT_CHANGES_KEY: CUSTOM_RCB_INDIRECT_CHANGES_VALUE,
4047
}
4148

4249

@@ -311,12 +318,15 @@ def _apply_adjust_base_behavior(
311318
return None
312319

313320
def _apply_fully_covered_patch_behavior(
314-
self, comparison: ComparisonProxy | FilteredComparison
315-
) -> tuple[str, str] | None:
321+
self,
322+
comparison: ComparisonProxy | FilteredComparison,
323+
notification_type: str,
324+
) -> tuple[tuple[str, str] | None, dict]:
316325
"""
317326
Rule for passing project status on fully_covered_patch behavior:
318-
Pass if patch coverage is 100% and there are no unexpected changes
327+
Pass if patch coverage is 100% and there are no indirect changes
319328
"""
329+
helper_text = {}
320330
log.info(
321331
"Applying fully_covered_patch behavior to project status",
322332
extra=dict(commit=comparison.head.commit.commitid),
@@ -332,23 +342,37 @@ def _apply_fully_covered_patch_behavior(
332342
"Unexpected changes when applying patch_100 behavior",
333343
extra=dict(commit=comparison.head.commit.commitid),
334344
)
335-
return None
345+
346+
# their comparison failed because of unexpected/indirect changes, give them helper text about it
347+
helper_text[CUSTOM_RCB_INDIRECT_CHANGES_KEY] = HELPER_TEXT_MAP[
348+
CUSTOM_RCB_INDIRECT_CHANGES_KEY
349+
].format(
350+
context=self.context,
351+
notification_type=notification_type,
352+
)
353+
return None, helper_text
336354

337355
diff = comparison.get_diff(use_original_base=True)
338356
patch_totals = comparison.head.report.apply_diff(diff)
339357
if patch_totals is None or patch_totals.lines == 0:
340358
# Coverage was not changed by patch
341359
return (
342-
StatusState.success.value,
343-
", passed because coverage was not affected by patch",
360+
(
361+
StatusState.success.value,
362+
", passed because coverage was not affected by patch",
363+
),
364+
helper_text,
344365
)
345366
coverage = Decimal(patch_totals.coverage)
346367
if coverage == 100.0:
347368
return (
348-
StatusState.success.value,
349-
", passed because patch was fully covered by tests, and no indirect coverage changes",
369+
(
370+
StatusState.success.value,
371+
", passed because patch was fully covered by tests, and no indirect coverage changes",
372+
),
373+
helper_text,
350374
)
351-
return None
375+
return None, helper_text
352376

353377
def get_project_status(
354378
self, comparison: ComparisonProxy | FilteredComparison, notification_type: str
@@ -373,9 +397,14 @@ def get_project_status(
373397
elif removed_code_behavior == "adjust_base":
374398
removed_code_result = self._apply_adjust_base_behavior(comparison)
375399
elif removed_code_behavior == "fully_covered_patch":
376-
removed_code_result = self._apply_fully_covered_patch_behavior(
377-
comparison
400+
removed_code_result, helper_text = (
401+
self._apply_fully_covered_patch_behavior(
402+
comparison,
403+
notification_type=notification_type,
404+
)
378405
)
406+
# if user set this in their yaml, give them helper text related to it
407+
result["included_helper_text"].update(helper_text)
379408
else:
380409
if removed_code_behavior not in [False, "off"]:
381410
log.warning(

‎services/notification/notifiers/tests/unit/test_status.py

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,11 @@
2121
from services.decoration import Decoration
2222
from services.notification.notifiers.base import NotificationResult
2323
from services.notification.notifiers.mixins.status import (
24+
CUSTOM_RCB_INDIRECT_CHANGES_KEY,
2425
CUSTOM_TARGET_TEXT_PATCH_KEY,
2526
CUSTOM_TARGET_TEXT_PROJECT_KEY,
2627
CUSTOM_TARGET_TEXT_VALUE,
28+
HELPER_TEXT_MAP,
2729
)
2830
from services.notification.notifiers.status import (
2931
ChangesStatusNotifier,
@@ -1909,6 +1911,74 @@ def test_notify_fully_covered_patch_behavior_fail(
19091911
assert result == expected_result
19101912
mock_get_impacted_files.assert_called()
19111913

1914+
def test_notify_fully_covered_patch_behavior_fail_indirect_changes(
1915+
self,
1916+
comparison_with_multiple_changes,
1917+
mock_repo_provider,
1918+
mock_configuration,
1919+
multiple_diff_changes,
1920+
mocker,
1921+
):
1922+
json_diff = multiple_diff_changes
1923+
mock_repo_provider.get_compare.return_value = {"diff": json_diff}
1924+
mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
1925+
mock_get_impacted_files = mocker.patch.object(
1926+
ComparisonProxy,
1927+
"get_impacted_files",
1928+
return_value={
1929+
"files": [
1930+
{
1931+
"base_name": "tests/file1.py",
1932+
"head_name": "tests/file1.py",
1933+
# Not complete, but we only care about these fields
1934+
"removed_diff_coverage": [[1, "h"]],
1935+
"added_diff_coverage": [[2, "h"], [3, "h"]],
1936+
"unexpected_line_changes": "any value in this field",
1937+
},
1938+
{
1939+
"base_name": "tests/file2.go",
1940+
"head_name": "tests/file2.go",
1941+
"removed_diff_coverage": [[1, "h"], [3, None]],
1942+
"added_diff_coverage": [],
1943+
"unexpected_line_changes": [],
1944+
},
1945+
],
1946+
},
1947+
)
1948+
notifier = ProjectStatusNotifier(
1949+
repository=comparison_with_multiple_changes.head.commit.repository,
1950+
title="title",
1951+
notifier_yaml_settings={
1952+
"target": "70%",
1953+
"removed_code_behavior": "fully_covered_patch",
1954+
},
1955+
notifier_site_settings=True,
1956+
current_yaml=UserYaml({}),
1957+
repository_service=mock_repo_provider,
1958+
)
1959+
expected_result = {
1960+
"message": "50.00% (target 70.00%)",
1961+
"state": "failure",
1962+
"included_helper_text": {
1963+
CUSTOM_TARGET_TEXT_PROJECT_KEY: CUSTOM_TARGET_TEXT_VALUE.format(
1964+
context="project",
1965+
notification_type="status",
1966+
point_of_comparison="head",
1967+
coverage="50.00",
1968+
target="70.00",
1969+
),
1970+
CUSTOM_RCB_INDIRECT_CHANGES_KEY: HELPER_TEXT_MAP[
1971+
CUSTOM_RCB_INDIRECT_CHANGES_KEY
1972+
].format(
1973+
context="project",
1974+
notification_type="status",
1975+
),
1976+
},
1977+
}
1978+
result = notifier.build_payload(comparison_with_multiple_changes)
1979+
assert result == expected_result
1980+
mock_get_impacted_files.assert_called()
1981+
19121982
def test_notify_fully_covered_patch_behavior_success(
19131983
self,
19141984
comparison_100_percent_patch,

0 commit comments

Comments
 (0)