Service: kwargs only when sending build statuses (#12007)

A small refactor extracted from #11942
readthedocs · Feb 19, 2025 · b333b97 · b333b97
1 parent bdca0bd
commit b333b97
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 24 deletions.
diff --git a/readthedocs/builds/tasks.py b/readthedocs/builds/tasks.py
@@ -416,9 +416,9 @@ def send_build_status(build_pk, commit, status):
 
     for service in service_class.for_project(build.project):
         success = service.send_build_status(
-            build,
-            commit,
-            status,
+            build=build,
+            commit=commit,
+            status=status,
         )
         if success:
             log.debug("Build status report sent correctly.")

diff --git a/readthedocs/oauth/services/base.py b/readthedocs/oauth/services/base.py
@@ -86,7 +86,7 @@ def update_webhook(self, project, integration):
         """
         raise NotImplementedError
 
-    def send_build_status(self, build, commit, status):
+    def send_build_status(self, *, build, commit, status):
         """
         Create commit status for project.
 

diff --git a/readthedocs/oauth/services/github.py b/readthedocs/oauth/services/github.py
@@ -406,7 +406,7 @@ def update_webhook(self, project, integration):
 
         return (False, resp)
 
-    def send_build_status(self, build, commit, status):
+    def send_build_status(self, *, build, commit, status):
         """
         Create GitHub commit status for project.
 

diff --git a/readthedocs/oauth/services/gitlab.py b/readthedocs/oauth/services/gitlab.py
@@ -495,7 +495,7 @@ def update_webhook(self, project, integration):
 
         return (False, resp)
 
-    def send_build_status(self, build, commit, status):
+    def send_build_status(self, *, build, commit, status):
         """
         Create GitLab commit status for project.
 

diff --git a/readthedocs/rtd_tests/tests/test_celery.py b/readthedocs/rtd_tests/tests/test_celery.py
@@ -103,9 +103,9 @@ def test_send_build_status_with_remote_repo_github(self, send_build_status):
         )
 
         send_build_status.assert_called_once_with(
-            external_build,
-            external_build.commit,
-            BUILD_STATUS_SUCCESS,
+            build=external_build,
+            commit=external_build.commit,
+            status=BUILD_STATUS_SUCCESS,
         )
         self.assertEqual(Notification.objects.count(), 0)
 
@@ -123,9 +123,9 @@ def test_send_build_status_with_social_account_github(self, send_build_status):
         )
 
         send_build_status.assert_called_once_with(
-            external_build,
-            external_build.commit,
-            BUILD_STATUS_SUCCESS,
+            build=external_build,
+            commit=external_build.commit,
+            status=BUILD_STATUS_SUCCESS,
         )
         self.assertEqual(Notification.objects.count(), 0)
 
@@ -176,9 +176,9 @@ def test_send_build_status_with_remote_repo_gitlab(self, send_build_status):
         )
 
         send_build_status.assert_called_once_with(
-            external_build,
-            external_build.commit,
-            BUILD_STATUS_SUCCESS,
+            build=external_build,
+            commit=external_build.commit,
+            status=BUILD_STATUS_SUCCESS,
         )
         self.assertEqual(Notification.objects.count(), 0)
 
@@ -196,9 +196,9 @@ def test_send_build_status_with_social_account_gitlab(self, send_build_status):
         )
 
         send_build_status.assert_called_once_with(
-            external_build,
-            external_build.commit,
-            BUILD_STATUS_SUCCESS,
+            build=external_build,
+            commit=external_build.commit,
+            status=BUILD_STATUS_SUCCESS,
         )
         self.assertEqual(Notification.objects.count(), 0)
 

diff --git a/readthedocs/rtd_tests/tests/test_oauth.py b/readthedocs/rtd_tests/tests/test_oauth.py
@@ -305,7 +305,9 @@ def test_multiple_users_same_repo(self):
     def test_send_build_status_successful(self, session, mock_logger):
         session.post.return_value.status_code = 201
         success = self.service.send_build_status(
-            self.external_build, self.external_build.commit, BUILD_STATUS_SUCCESS
+            build=self.external_build,
+            commit=self.external_build.commit,
+            status=BUILD_STATUS_SUCCESS,
         )
 
         self.assertTrue(success)
@@ -319,7 +321,9 @@ def test_send_build_status_successful(self, session, mock_logger):
     def test_send_build_status_404_error(self, session, mock_logger):
         session.post.return_value.status_code = 404
         success = self.service.send_build_status(
-            self.external_build, self.external_build.commit, BUILD_STATUS_SUCCESS
+            build=self.external_build,
+            commit=self.external_build.commit,
+            status=BUILD_STATUS_SUCCESS,
         )
 
         self.assertFalse(success)
@@ -333,7 +337,9 @@ def test_send_build_status_404_error(self, session, mock_logger):
     def test_send_build_status_value_error(self, session, mock_logger):
         session.post.side_effect = ValueError
         success = self.service.send_build_status(
-            self.external_build, self.external_build.commit, BUILD_STATUS_SUCCESS
+            build=self.external_build,
+            commit=self.external_build.commit,
+            status=BUILD_STATUS_SUCCESS,
         )
 
         self.assertFalse(success)
@@ -1163,7 +1169,9 @@ def test_send_build_status_successful(self, repo_id, session, mock_logger):
         repo_id().return_value = "9999"
 
         success = self.service.send_build_status(
-            self.external_build, self.external_build.commit, BUILD_STATUS_SUCCESS
+            build=self.external_build,
+            commit=self.external_build.commit,
+            status=BUILD_STATUS_SUCCESS,
         )
 
         self.assertTrue(success)
@@ -1180,7 +1188,9 @@ def test_send_build_status_404_error(self, repo_id, session, mock_logger):
         repo_id.return_value = "9999"
 
         success = self.service.send_build_status(
-            self.external_build, self.external_build.commit, BUILD_STATUS_SUCCESS
+            build=self.external_build,
+            commit=self.external_build.commit,
+            status=BUILD_STATUS_SUCCESS,
         )
 
         self.assertFalse(success)
@@ -1197,7 +1207,9 @@ def test_send_build_status_value_error(self, repo_id, session, mock_logger):
         repo_id().return_value = "9999"
 
         success = self.service.send_build_status(
-            self.external_build, self.external_build.commit, BUILD_STATUS_SUCCESS
+            build=self.external_build,
+            commit=self.external_build.commit,
+            status=BUILD_STATUS_SUCCESS,
         )
 
         self.assertFalse(success)