Add call to avoid status + checks at the same time (#756)

services/notification/notifiers/checks/base.py

@@ -94,7 +94,7 @@ def notify(self, comparison: Comparison):
             or comparison.enriched_pull.provider_pull is None
         ):
             log.debug(
-                "Faling back to commit_status: Pull request not in provider",
+                "Falling back to commit_status: Pull request not in provider",
                 extra=dict(
                     notifier=self.name,
                     repoid=comparison.head.commit.repoid,
@@ -111,7 +111,7 @@ def notify(self, comparison: Comparison):
             )
         if comparison.pull.state != "open":
             log.debug(
-                "Faling back to commit_status: Pull request closed",
+                "Falling back to commit_status: Pull request closed",
                 extra=dict(
                     notifier=self.name,
                     repoid=comparison.head.commit.repoid,
@@ -142,6 +142,28 @@ def notify(self, comparison: Comparison):
                 data_sent=None,
                 data_received=None,
             )
+        # Check for existing statuses for this commit. If so, retain
+        # statuses and don't do a check as well
+        statuses = comparison.get_existing_statuses()
+        status_title = self.get_status_external_name()
+        if statuses and statuses.get(status_title):
+            log.debug(
+                "Falling back to commit_status: Status already exists for this commit",
+                extra=dict(
+                    notifier=self.name,
+                    repoid=comparison.head.commit.repoid,
+                    notifier_title=self.title,
+                    status_title=status_title,
+                    commit=comparison.head.commit,
+                ),
+            )
+            return NotificationResult(
+                notification_attempted=False,
+                notification_successful=None,
+                explanation="preexisting_commit_status",
+                data_sent=None,
+                data_received=None,
+            )
         payload = None
         try:
             with nullcontext():

services/notification/notifiers/checks/checks_with_fallback.py

@@ -52,6 +52,7 @@ def notify(self, comparison):
                 res.explanation == "no_pull_request"
                 or res.explanation == "pull_request_not_in_provider"
                 or res.explanation == "pull_request_closed"
+                or res.explanation == "preexisting_commit_status"
             ):
                 log.info(
                     "Couldn't use checks notifier, falling back to status notifiers",

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

@@ -6,6 +6,7 @@
 from shared.reports.readonly import ReadOnlyReport
 from shared.reports.resources import Report, ReportFile, ReportLine
 from shared.torngit.exceptions import TorngitClientGeneralError, TorngitError
+from shared.torngit.status import Status
 from shared.yaml.user_yaml import UserYaml
 
 from services.decoration import Decoration
@@ -307,6 +308,7 @@ def test_checks_403_failure(self, sample_comparison, mocker, mock_repo_provider)
         assert res == "success"
 
     def test_checks_failure(self, sample_comparison, mocker, mock_repo_provider):
+        mock_repo_provider.get_commit_statuses.return_value = Status([])
         mock_repo_provider.create_check_run = Mock(
             side_effect=TorngitClientGeneralError(
                 409, response_data="No Access", message="No Access"
@@ -1022,6 +1024,7 @@ def test_send_notification_annotations_paginations(
     def test_notify(
         self, sample_comparison, mocker, mock_repo_provider, mock_configuration
     ):
+        mock_repo_provider.get_commit_statuses.return_value = Status([])
         mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
         comparison = sample_comparison
         payload = {
@@ -1054,6 +1057,7 @@ def test_notify(
     def test_notify_passing_empty_upload(
         self, sample_comparison, mocker, mock_repo_provider, mock_configuration
     ):
+        mock_repo_provider.get_commit_statuses.return_value = Status([])
         mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
         comparison = sample_comparison
         mock_repo_provider.create_check_run.return_value = 2234563
@@ -1081,6 +1085,7 @@ def test_notify_passing_empty_upload(
     def test_notify_failing_empty_upload(
         self, sample_comparison, mocker, mock_repo_provider, mock_configuration
     ):
+        mock_repo_provider.get_commit_statuses.return_value = Status([])
         mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
         comparison = sample_comparison
         mock_repo_provider.create_check_run.return_value = 2234563
@@ -1108,6 +1113,7 @@ def test_notify_failing_empty_upload(
     def test_notification_exception(
         self, sample_comparison, mock_repo_provider, mock_configuration
     ):
+        mock_repo_provider.get_commit_statuses.return_value = Status([])
         mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
         notifier = PatchChecksNotifier(
             repository=sample_comparison.head.commit.repository,
@@ -1153,6 +1159,34 @@ def test_notification_exception_not_fit(self, sample_comparison, mocker):
         assert result.data_sent is None
         assert result.data_received is None
 
+    def test_notification_exception_preexisting_commit_status(
+        self, sample_comparison, mocker, mock_repo_provider
+    ):
+        comparison = sample_comparison
+        notifier = ProjectChecksNotifier(
+            repository=comparison.head.commit.repository,
+            title="title",
+            notifier_yaml_settings={"paths": ["file_1.go"]},
+            notifier_site_settings=True,
+            current_yaml=UserYaml({}),
+        )
+        mock_repo_provider.get_commit_statuses.return_value = Status(
+            [
+                {
+                    "time": "2024-10-01T22:34:52Z",
+                    "state": "success",
+                    "description": "42.85% (+0.00%) compared to 36be7f3",
+                    "context": "codecov/project/title",
+                }
+            ]
+        )
+        result = notifier.notify(sample_comparison)
+        assert not result.notification_attempted
+        assert result.notification_successful is None
+        assert result.explanation == "preexisting_commit_status"
+        assert result.data_sent is None
+        assert result.data_received is None
+
     def test_checks_with_after_n_builds(self, sample_comparison, mocker):
         notifier = ChecksNotifier(
             repository=sample_comparison.head.commit.repository,
@@ -1690,6 +1724,7 @@ def test_build_payload_no_base_report(
     def test_check_notify_no_path_match(
         self, sample_comparison, mocker, mock_repo_provider, mock_configuration
     ):
+        mock_repo_provider.get_commit_statuses.return_value = Status([])
         mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
         comparison = sample_comparison
         payload = {
@@ -1723,6 +1758,7 @@ def test_check_notify_no_path_match(
     def test_check_notify_single_path_match(
         self, sample_comparison, mocker, mock_repo_provider, mock_configuration
     ):
+        mock_repo_provider.get_commit_statuses.return_value = Status([])
         mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
         comparison = sample_comparison
         payload = {
@@ -1763,6 +1799,7 @@ def test_check_notify_single_path_match(
     def test_check_notify_multiple_path_match(
         self, sample_comparison, mocker, mock_repo_provider, mock_configuration
     ):
+        mock_repo_provider.get_commit_statuses.return_value = Status([])
         mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
         comparison = sample_comparison
         payload = {
@@ -1797,6 +1834,7 @@ def test_check_notify_multiple_path_match(
     def test_check_notify_with_paths(
         self, sample_comparison, mocker, mock_repo_provider, mock_configuration
     ):
+        mock_repo_provider.get_commit_statuses.return_value = Status([])
         mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
         comparison = sample_comparison
         payload = {
@@ -1833,6 +1871,7 @@ def test_notify_pass_behavior_when_coverage_not_uploaded(
         mock_repo_provider,
         mock_configuration,
     ):
+        mock_repo_provider.get_commit_statuses.return_value = Status([])
         mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
         mock_repo_provider.create_check_run.return_value = 2234563
         mock_repo_provider.update_check_run.return_value = "success"
@@ -1878,6 +1917,7 @@ def test_notify_pass_behavior_when_coverage_uploaded(
         mock_repo_provider,
         mock_configuration,
     ):
+        mock_repo_provider.get_commit_statuses.return_value = Status([])
         mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
         mock_repo_provider.create_check_run.return_value = 2234563
         mock_repo_provider.update_check_run.return_value = "success"
@@ -1917,6 +1957,7 @@ def test_notify_include_behavior_when_coverage_not_uploaded(
         mock_repo_provider,
         mock_configuration,
     ):
+        mock_repo_provider.get_commit_statuses.return_value = Status([])
         mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
         mock_repo_provider.create_check_run.return_value = 2234563
         mock_repo_provider.update_check_run.return_value = "success"
@@ -1957,6 +1998,7 @@ def test_notify_exclude_behavior_when_coverage_not_uploaded(
         mock_repo_provider,
         mock_configuration,
     ):
+        mock_repo_provider.get_commit_statuses.return_value = Status([])
         mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
         mock_repo_provider.create_check_run.return_value = 2234563
         mock_repo_provider.update_check_run.return_value = "success"
@@ -1986,6 +2028,7 @@ def test_notify_exclude_behavior_when_coverage_uploaded(
         mock_repo_provider,
         mock_configuration,
     ):
+        mock_repo_provider.get_commit_statuses.return_value = Status([])
         mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
         mock_repo_provider.create_check_run.return_value = 2234563
         mock_repo_provider.update_check_run.return_value = "success"
@@ -2026,6 +2069,7 @@ def test_notify_exclude_behavior_when_some_coverage_uploaded(
         mock_repo_provider,
         mock_configuration,
     ):
+        mock_repo_provider.get_commit_statuses.return_value = Status([])
         mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
         mock_repo_provider.create_check_run.return_value = 2234563
         mock_repo_provider.update_check_run.return_value = "success"
@@ -2070,6 +2114,7 @@ def test_notify_exclude_behavior_no_flags(
         mock_repo_provider,
         mock_configuration,
     ):
+        mock_repo_provider.get_commit_statuses.return_value = Status([])
         mock_configuration.params["setup"]["codecov_dashboard_url"] = "test.example.br"
         mock_repo_provider.create_check_run.return_value = 2234563
         mock_repo_provider.update_check_run.return_value = "success"

services/notification/tests/unit/test_notification_service.py

@@ -7,6 +7,7 @@
 from celery.exceptions import SoftTimeLimitExceeded
 from shared.plan.constants import PlanName
 from shared.reports.resources import Report, ReportFile, ReportLine
+from shared.torngit.status import Status
 from shared.yaml import UserYaml
 
 from database.enums import Decoration, Notification, NotificationState
@@ -461,6 +462,7 @@ def test_notify_individual_notifier_timeout(self, mocker, sample_comparison):
     def test_notify_individual_checks_notifier(
         self, mocker, sample_comparison, mock_repo_provider, mock_configuration
     ):
+        mock_repo_provider.get_commit_statuses.return_value = Status([])
         mock_configuration._params["setup"] = {"codecov_dashboard_url": "test"}
         current_yaml = {}
         commit = sample_comparison.head.commit