fix github checks annotations

codecov · Jan 30, 2025 · 190cc01 · 190cc01
1 parent 6302730
commit 190cc01
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 34 deletions.
diff --git a/services/notification/notifiers/checks/patch.py b/services/notification/notifiers/checks/patch.py
@@ -52,12 +52,12 @@ def build_payload(
             title = "Codecov Report"
 
         checks_yaml_field = read_yaml_field(self.current_yaml, ("github_checks",))
-
-        should_annotate = (
-            checks_yaml_field.get("annotations", False)
-            if checks_yaml_field is not None
-            else True
-        )
+        try:
+            # checks_yaml_field can be dict, bool, None
+            # should_annotate defaults to False as of Jan 30 2025
+            should_annotate = checks_yaml_field.get("annotations", False)
+        except AttributeError:
+            should_annotate = False
 
         flags = self.notifier_yaml_settings.get("flags")
         paths = self.notifier_yaml_settings.get("paths")

diff --git a/services/notification/notifiers/tests/unit/test_checks.py b/services/notification/notifiers/tests/unit/test_checks.py
@@ -762,15 +762,7 @@ def test_build_default_payload(
             "output": {
                 "title": "66.67% of diff hit (target 50.00%)",
                 "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%)",
-                "annotations": [
-                    {
-                        "path": "file_1.go",
-                        "start_line": 10,
-                        "end_line": 10,
-                        "annotation_level": "warning",
-                        "message": "Added line #L10 was not covered by tests",
-                    }
-                ],
+                "annotations": [],
             },
             "included_helper_text": {},
         }
@@ -823,7 +815,7 @@ def test_build_payload_without_base_report(
             title="default",
             notifier_yaml_settings={},
             notifier_site_settings=True,
-            current_yaml=UserYaml({}),
+            current_yaml=UserYaml({"github_checks": {"annotations": True}}),
             repository_service=mock_repo_provider,
         )
         expected_result = {
@@ -869,15 +861,7 @@ def test_build_payload_target_coverage_failure_within_threshold(
             "output": {
                 "title": "66.67% of diff hit (within 5.00% threshold of 70.00%)",
                 "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%)",
-                "annotations": [
-                    {
-                        "path": "file_1.go",
-                        "start_line": 10,
-                        "end_line": 10,
-                        "annotation_level": "warning",
-                        "message": "Added line #L10 was not covered by tests",
-                    }
-                ],
+                "annotations": [],
             },
             "included_helper_text": {},
         }
@@ -912,15 +896,7 @@ def test_build_payload_with_multiple_changes(
             "output": {
                 "title": "50.00% of diff hit (target 76.92%)",
                 "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%)",
-                "annotations": [
-                    {
-                        "path": "modified.py",
-                        "start_line": 23,
-                        "end_line": 23,
-                        "annotation_level": "warning",
-                        "message": "Added line #L23 was not covered by tests",
-                    }
-                ],
+                "annotations": [],
             },
             "included_helper_text": {},  # not a custom target, no helper text
         }
@@ -934,6 +910,75 @@ def test_build_payload_with_multiple_changes(
                 "segments"
             ) == multiple_diff_changes["files"][filename].get("segments")
 
+    def test_github_checks_annotations_yaml(
+        self,
+        comparison_with_multiple_changes,
+        mock_repo_provider,
+        mock_configuration,
+        multiple_diff_changes,
+    ):
+        mock_repo_provider.get_compare.return_value = {"diff": multiple_diff_changes}
+        mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
+
+        empty_annotations = []
+
+        # checks_yaml_field can be None
+        notifier = PatchChecksNotifier(
+            repository=comparison_with_multiple_changes.head.commit.repository,
+            title="default",
+            notifier_yaml_settings={},
+            notifier_site_settings=True,
+            current_yaml=UserYaml({}),
+            repository_service=mock_repo_provider,
+        )
+        result = notifier.build_payload(comparison_with_multiple_changes)
+        assert empty_annotations == result["output"]["annotations"]
+
+        # checks_yaml_field can be dict
+        notifier = PatchChecksNotifier(
+            repository=comparison_with_multiple_changes.head.commit.repository,
+            title="default",
+            notifier_yaml_settings={},
+            notifier_site_settings=True,
+            current_yaml=UserYaml({"github_checks": {"one": "two"}}),
+            repository_service=mock_repo_provider,
+        )
+        result = notifier.build_payload(comparison_with_multiple_changes)
+        assert empty_annotations == result["output"]["annotations"]
+
+        # checks_yaml_field can be bool
+        notifier = PatchChecksNotifier(
+            repository=comparison_with_multiple_changes.head.commit.repository,
+            title="default",
+            notifier_yaml_settings={},
+            notifier_site_settings=True,
+            current_yaml=UserYaml({"github_checks": False}),
+            repository_service=mock_repo_provider,
+        )
+        result = notifier.build_payload(comparison_with_multiple_changes)
+        assert empty_annotations == result["output"]["annotations"]
+
+        # checks_yaml_field with annotations
+        notifier = PatchChecksNotifier(
+            repository=comparison_with_multiple_changes.head.commit.repository,
+            title="default",
+            notifier_yaml_settings={},
+            notifier_site_settings=True,
+            current_yaml=UserYaml({"github_checks": {"annotations": True}}),
+            repository_service=mock_repo_provider,
+        )
+        expected_annotations = [
+            {
+                "path": "modified.py",
+                "start_line": 23,
+                "end_line": 23,
+                "annotation_level": "warning",
+                "message": "Added line #L23 was not covered by tests",
+            }
+        ]
+        result = notifier.build_payload(comparison_with_multiple_changes)
+        assert expected_annotations == result["output"]["annotations"]
+
     def test_build_payload_no_diff(
         self, sample_comparison, mock_repo_provider, mock_configuration
     ):