Skip to content

Commit 6b0c691

Browse files
authoredJan 31, 2025··
fix github checks annotations (#1052)
1 parent 9d423d7 commit 6b0c691

File tree

2 files changed

+79
-34
lines changed

2 files changed

+79
-34
lines changed
 

‎services/notification/notifiers/checks/patch.py

+6-6
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,12 @@ def build_payload(
5252
title = "Codecov Report"
5353

5454
checks_yaml_field = read_yaml_field(self.current_yaml, ("github_checks",))
55-
56-
should_annotate = (
57-
checks_yaml_field.get("annotations", False)
58-
if checks_yaml_field is not None
59-
else True
60-
)
55+
try:
56+
# checks_yaml_field can be dict, bool, None
57+
# should_annotate defaults to False as of Jan 30 2025
58+
should_annotate = checks_yaml_field.get("annotations", False)
59+
except AttributeError:
60+
should_annotate = False
6161

6262
flags = self.notifier_yaml_settings.get("flags")
6363
paths = self.notifier_yaml_settings.get("paths")

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

+73-28
Original file line numberDiff line numberDiff line change
@@ -762,15 +762,7 @@ def test_build_default_payload(
762762
"output": {
763763
"title": "66.67% of diff hit (target 50.00%)",
764764
"summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_default_payload/{sample_comparison.head.commit.repository.name}/pull/{sample_comparison.pull.pullid}?dropdown=coverage&src=pr&el=h1)\n\n66.67% of diff hit (target 50.00%)",
765-
"annotations": [
766-
{
767-
"path": "file_1.go",
768-
"start_line": 10,
769-
"end_line": 10,
770-
"annotation_level": "warning",
771-
"message": "Added line #L10 was not covered by tests",
772-
}
773-
],
765+
"annotations": [],
774766
},
775767
"included_helper_text": {},
776768
}
@@ -823,7 +815,7 @@ def test_build_payload_without_base_report(
823815
title="default",
824816
notifier_yaml_settings={},
825817
notifier_site_settings=True,
826-
current_yaml=UserYaml({}),
818+
current_yaml=UserYaml({"github_checks": {"annotations": True}}),
827819
repository_service=mock_repo_provider,
828820
)
829821
expected_result = {
@@ -869,15 +861,7 @@ def test_build_payload_target_coverage_failure_within_threshold(
869861
"output": {
870862
"title": "66.67% of diff hit (within 5.00% threshold of 70.00%)",
871863
"summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_payload_target_coverage_failure_within_threshold/{sample_comparison.head.commit.repository.name}/pull/{sample_comparison.pull.pullid}?dropdown=coverage&src=pr&el=h1)\n\n66.67% of diff hit (within 5.00% threshold of 70.00%)",
872-
"annotations": [
873-
{
874-
"path": "file_1.go",
875-
"start_line": 10,
876-
"end_line": 10,
877-
"annotation_level": "warning",
878-
"message": "Added line #L10 was not covered by tests",
879-
}
880-
],
864+
"annotations": [],
881865
},
882866
"included_helper_text": {},
883867
}
@@ -912,15 +896,7 @@ def test_build_payload_with_multiple_changes(
912896
"output": {
913897
"title": "50.00% of diff hit (target 76.92%)",
914898
"summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_payload_with_multiple_changes/{comparison_with_multiple_changes.head.commit.repository.name}/pull/{comparison_with_multiple_changes.pull.pullid}?dropdown=coverage&src=pr&el=h1)\n\n50.00% of diff hit (target 76.92%)",
915-
"annotations": [
916-
{
917-
"path": "modified.py",
918-
"start_line": 23,
919-
"end_line": 23,
920-
"annotation_level": "warning",
921-
"message": "Added line #L23 was not covered by tests",
922-
}
923-
],
899+
"annotations": [],
924900
},
925901
"included_helper_text": {}, # not a custom target, no helper text
926902
}
@@ -934,6 +910,75 @@ def test_build_payload_with_multiple_changes(
934910
"segments"
935911
) == multiple_diff_changes["files"][filename].get("segments")
936912

913+
def test_github_checks_annotations_yaml(
914+
self,
915+
comparison_with_multiple_changes,
916+
mock_repo_provider,
917+
mock_configuration,
918+
multiple_diff_changes,
919+
):
920+
mock_repo_provider.get_compare.return_value = {"diff": multiple_diff_changes}
921+
mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
922+
923+
empty_annotations = []
924+
925+
# checks_yaml_field can be None
926+
notifier = PatchChecksNotifier(
927+
repository=comparison_with_multiple_changes.head.commit.repository,
928+
title="default",
929+
notifier_yaml_settings={},
930+
notifier_site_settings=True,
931+
current_yaml=UserYaml({}),
932+
repository_service=mock_repo_provider,
933+
)
934+
result = notifier.build_payload(comparison_with_multiple_changes)
935+
assert empty_annotations == result["output"]["annotations"]
936+
937+
# checks_yaml_field can be dict
938+
notifier = PatchChecksNotifier(
939+
repository=comparison_with_multiple_changes.head.commit.repository,
940+
title="default",
941+
notifier_yaml_settings={},
942+
notifier_site_settings=True,
943+
current_yaml=UserYaml({"github_checks": {"one": "two"}}),
944+
repository_service=mock_repo_provider,
945+
)
946+
result = notifier.build_payload(comparison_with_multiple_changes)
947+
assert empty_annotations == result["output"]["annotations"]
948+
949+
# checks_yaml_field can be bool
950+
notifier = PatchChecksNotifier(
951+
repository=comparison_with_multiple_changes.head.commit.repository,
952+
title="default",
953+
notifier_yaml_settings={},
954+
notifier_site_settings=True,
955+
current_yaml=UserYaml({"github_checks": False}),
956+
repository_service=mock_repo_provider,
957+
)
958+
result = notifier.build_payload(comparison_with_multiple_changes)
959+
assert empty_annotations == result["output"]["annotations"]
960+
961+
# checks_yaml_field with annotations
962+
notifier = PatchChecksNotifier(
963+
repository=comparison_with_multiple_changes.head.commit.repository,
964+
title="default",
965+
notifier_yaml_settings={},
966+
notifier_site_settings=True,
967+
current_yaml=UserYaml({"github_checks": {"annotations": True}}),
968+
repository_service=mock_repo_provider,
969+
)
970+
expected_annotations = [
971+
{
972+
"path": "modified.py",
973+
"start_line": 23,
974+
"end_line": 23,
975+
"annotation_level": "warning",
976+
"message": "Added line #L23 was not covered by tests",
977+
}
978+
]
979+
result = notifier.build_payload(comparison_with_multiple_changes)
980+
assert expected_annotations == result["output"]["annotations"]
981+
937982
def test_build_payload_no_diff(
938983
self, sample_comparison, mock_repo_provider, mock_configuration
939984
):

0 commit comments

Comments
 (0)
Please sign in to comment.