Browse Source

chore(issues): Opt in a few more endpoint tests to stronger types (#82382)

`tests.sentry.issues.endpoints.test_organization_group_index` is the
only remaining before we can just fully opt in
`tests.sentry.issues.endpoints.*`.
Matt Duncan 2 months ago
parent
commit
557ada4ff5

+ 5 - 0
pyproject.toml

@@ -511,8 +511,12 @@ module = [
     "tests.sentry.issues.endpoints.test_group_activities",
     "tests.sentry.issues.endpoints.test_group_details",
     "tests.sentry.issues.endpoints.test_group_event_details",
+    "tests.sentry.issues.endpoints.test_group_events",
     "tests.sentry.issues.endpoints.test_group_hashes",
     "tests.sentry.issues.endpoints.test_group_notes",
+    "tests.sentry.issues.endpoints.test_group_notes_details",
+    "tests.sentry.issues.endpoints.test_group_participants",
+    "tests.sentry.issues.endpoints.test_group_similar_issues_embeddings",
     "tests.sentry.issues.endpoints.test_group_tombstone",
     "tests.sentry.issues.endpoints.test_group_tombstone_details",
     "tests.sentry.issues.endpoints.test_organization_group_search_views",
@@ -521,6 +525,7 @@ module = [
     "tests.sentry.issues.endpoints.test_project_group_stats",
     "tests.sentry.issues.endpoints.test_project_stacktrace_link",
     "tests.sentry.issues.endpoints.test_related_issues",
+    "tests.sentry.issues.endpoints.test_shared_group_details",
     "tests.sentry.issues.endpoints.test_source_map_debug",
     "tests.sentry.issues.endpoints.test_team_groups_old",
     "tests.sentry.issues.test_attributes",

+ 3 - 2
tests/sentry/issues/endpoints/test_group_events.py

@@ -1,6 +1,7 @@
 from datetime import timedelta
 
 from django.utils import timezone
+from rest_framework.response import Response
 
 from sentry.issues.grouptype import ProfileFileIOGroupType
 from sentry.testutils.cases import APITestCase, PerformanceIssueTestCase, SnubaTestCase
@@ -15,10 +16,10 @@ class GroupEventsTest(APITestCase, SnubaTestCase, SearchIssueTestMixin, Performa
         self.min_ago = before_now(minutes=1)
         self.two_min_ago = before_now(minutes=2)
 
-    def do_request(self, url: str):
+    def do_request(self, url: str) -> Response:
         return self.client.get(url, format="json")
 
-    def _parse_links(self, header):
+    def _parse_links(self, header: str) -> dict[str | None, dict[str, str | None]]:
         # links come in {url: {...attrs}}, but we need {rel: {...attrs}}
         links = {}
         for url, attrs in parse_link_header(header).items():

+ 9 - 9
tests/sentry/issues/endpoints/test_group_notes_details.py

@@ -1,5 +1,5 @@
 from functools import cached_property
-from unittest.mock import patch
+from unittest.mock import MagicMock, patch
 
 import responses
 
@@ -16,7 +16,7 @@ from sentry.types.activity import ActivityType
 
 
 class GroupNotesDetailsTest(APITestCase):
-    def setUp(self):
+    def setUp(self) -> None:
         super().setUp()
         self.activity.data["external_id"] = "123"
         self.activity.save()
@@ -43,10 +43,10 @@ class GroupNotesDetailsTest(APITestCase):
         )
 
     @cached_property
-    def url(self):
+    def url(self) -> str:
         return f"/api/0/issues/{self.group.id}/comments/{self.activity.id}/"
 
-    def test_delete(self):
+    def test_delete(self) -> None:
         self.login_as(user=self.user)
 
         url = self.url
@@ -59,7 +59,7 @@ class GroupNotesDetailsTest(APITestCase):
 
         assert Group.objects.get(id=self.group.id).num_comments == 0
 
-    def test_delete_comment_and_subscription(self):
+    def test_delete_comment_and_subscription(self) -> None:
         """Test that if a user deletes their comment on an issue, we delete the subscription too"""
         self.login_as(user=self.user)
         event = self.store_event(data={}, project_id=self.project.id)
@@ -91,7 +91,7 @@ class GroupNotesDetailsTest(APITestCase):
             reason=GroupSubscriptionReason.comment,
         ).exists()
 
-    def test_delete_multiple_comments(self):
+    def test_delete_multiple_comments(self) -> None:
         """Test that if a user has commented multiple times on an issue and deletes one, we don't remove the subscription"""
         self.login_as(user=self.user)
         event = self.store_event(data={}, project_id=self.project.id)
@@ -130,7 +130,7 @@ class GroupNotesDetailsTest(APITestCase):
 
     @patch("sentry.integrations.mixins.issues.IssueBasicIntegration.update_comment")
     @responses.activate
-    def test_put(self, mock_update_comment):
+    def test_put(self, mock_update_comment: MagicMock) -> None:
         self.login_as(user=self.user)
 
         url = self.url
@@ -154,7 +154,7 @@ class GroupNotesDetailsTest(APITestCase):
         assert mock_update_comment.call_args[0][2] == activity
 
     @responses.activate
-    def test_put_ignore_mentions(self):
+    def test_put_ignore_mentions(self) -> None:
         GroupLink.objects.filter(group_id=self.group.id).delete()
         self.login_as(user=self.user)
 
@@ -179,7 +179,7 @@ class GroupNotesDetailsTest(APITestCase):
         }
 
     @patch("sentry.integrations.mixins.issues.IssueBasicIntegration.update_comment")
-    def test_put_no_external_id(self, mock_update_comment):
+    def test_put_no_external_id(self, mock_update_comment: MagicMock) -> None:
         del self.activity.data["external_id"]
         self.activity.save()
         self.login_as(user=self.user)

+ 6 - 3
tests/sentry/issues/endpoints/test_group_participants.py

@@ -1,13 +1,16 @@
+from collections.abc import Callable
+
+from sentry.models.group import Group
 from sentry.models.groupsubscription import GroupSubscription
 from sentry.testutils.cases import APITestCase
 
 
 class GroupParticipantsTest(APITestCase):
-    def setUp(self):
+    def setUp(self) -> None:
         super().setUp()
         self.login_as(self.user)
 
-    def _get_path_functions(self):
+    def _get_path_functions(self) -> tuple[Callable[[Group], str], Callable[[Group], str]]:
         # The urls for group participants are supported both with an org slug and without.
         # We test both as long as we support both.
         # Because removing old urls takes time and consideration of the cost of breaking lingering references, a
@@ -17,7 +20,7 @@ class GroupParticipantsTest(APITestCase):
             lambda group: f"/api/0/organizations/{self.organization.slug}/issues/{group.id}/participants/",
         )
 
-    def test_simple(self):
+    def test_simple(self) -> None:
         group = self.create_group()
         GroupSubscription.objects.create(
             user_id=self.user.id, group=group, project=group.project, is_active=True

+ 38 - 102
tests/sentry/issues/endpoints/test_group_similar_issues_embeddings.py

@@ -43,7 +43,7 @@ EVENT_WITH_THREADS_STACKTRACE = {
 
 
 class GroupSimilarIssuesEmbeddingsTest(APITestCase):
-    def setUp(self):
+    def setUp(self) -> None:
         super().setUp()
         self.login_as(self.user)
         self.org = self.create_organization(owner=self.user)
@@ -81,82 +81,6 @@ class GroupSimilarIssuesEmbeddingsTest(APITestCase):
             data={"message": "Dogs are great!"}, project_id=self.project
         )
 
-    def create_exception(
-        self, exception_type_str="Exception", exception_value="it broke", frames=None
-    ):
-        frames = frames or []
-        return {
-            "id": "exception",
-            "name": "exception",
-            "contributes": True,
-            "hint": None,
-            "values": [
-                {
-                    "id": "stacktrace",
-                    "name": "stack-trace",
-                    "contributes": True,
-                    "hint": None,
-                    "values": frames,
-                },
-                {
-                    "id": "type",
-                    "name": None,
-                    "contributes": True,
-                    "hint": None,
-                    "values": [exception_type_str],
-                },
-                {
-                    "id": "value",
-                    "name": None,
-                    "contributes": False,
-                    "hint": None,
-                    "values": [exception_value],
-                },
-            ],
-        }
-
-    def create_frames(
-        self,
-        num_frames,
-        contributes=True,
-        start_index=1,
-        context_line_factory=lambda i: f"test = {i}!",
-    ):
-        frames = []
-        for i in range(start_index, start_index + num_frames):
-            frames.append(
-                {
-                    "id": "frame",
-                    "name": None,
-                    "contributes": contributes,
-                    "hint": None,
-                    "values": [
-                        {
-                            "id": "filename",
-                            "name": None,
-                            "contributes": contributes,
-                            "hint": None,
-                            "values": ["hello.py"],
-                        },
-                        {
-                            "id": "function",
-                            "name": None,
-                            "contributes": contributes,
-                            "hint": None,
-                            "values": ["hello_there"],
-                        },
-                        {
-                            "id": "context-line",
-                            "name": None,
-                            "contributes": contributes,
-                            "hint": None,
-                            "values": [context_line_factory(i)],
-                        },
-                    ],
-                }
-            )
-        return frames
-
     def get_expected_response(
         self,
         group_ids: Sequence[int],
@@ -179,7 +103,7 @@ class GroupSimilarIssuesEmbeddingsTest(APITestCase):
             )
         return response
 
-    def test_get_formatted_results(self):
+    def test_get_formatted_results(self) -> None:
         event_from_second_similar_group = save_new_event(
             {"message": "Adopt don't shop"}, self.project
         )
@@ -215,7 +139,12 @@ class GroupSimilarIssuesEmbeddingsTest(APITestCase):
     @mock.patch("sentry.seer.similarity.similar_issues.metrics.incr")
     @mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
     @mock.patch("sentry.issues.endpoints.group_similar_issues_embeddings.logger")
-    def test_simple(self, mock_logger, mock_seer_request, mock_metrics_incr):
+    def test_simple(
+        self,
+        mock_logger: mock.MagicMock,
+        mock_seer_request: mock.MagicMock,
+        mock_metrics_incr: mock.MagicMock,
+    ) -> None:
         seer_return_value: SimilarIssuesEmbeddingsResponse = {
             "responses": [
                 {
@@ -271,7 +200,7 @@ class GroupSimilarIssuesEmbeddingsTest(APITestCase):
         )
 
     @mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
-    def test_simple_threads(self, mock_seer_request):
+    def test_simple_threads(self, mock_seer_request: mock.MagicMock) -> None:
         event = self.store_event(data=EVENT_WITH_THREADS_STACKTRACE, project_id=self.project)
         data = {
             "parent_hash": self.similar_event.get_primary_hash(),
@@ -293,7 +222,7 @@ class GroupSimilarIssuesEmbeddingsTest(APITestCase):
 
     @mock.patch("sentry.analytics.record")
     @mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
-    def test_multiple(self, mock_seer_request, mock_record):
+    def test_multiple(self, mock_seer_request: mock.MagicMock, mock_record: mock.MagicMock) -> None:
         over_threshold_group_event = save_new_event({"message": "Maisey is silly"}, self.project)
         under_threshold_group_event = save_new_event({"message": "Charlie is goofy"}, self.project)
 
@@ -347,7 +276,7 @@ class GroupSimilarIssuesEmbeddingsTest(APITestCase):
         )
 
     @mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
-    def test_parent_hash_in_group_hashes(self, mock_seer_request):
+    def test_parent_hash_in_group_hashes(self, mock_seer_request: mock.MagicMock) -> None:
         """
         Test that the request group's hashes are filtered out of the returned similar parent hashes
         """
@@ -377,7 +306,12 @@ class GroupSimilarIssuesEmbeddingsTest(APITestCase):
     @mock.patch("sentry.seer.similarity.similar_issues.metrics.incr")
     @mock.patch("sentry.seer.similarity.similar_issues.logger")
     @mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
-    def test_incomplete_return_data(self, mock_seer_request, mock_logger, mock_metrics_incr):
+    def test_incomplete_return_data(
+        self,
+        mock_seer_request: mock.MagicMock,
+        mock_logger: mock.MagicMock,
+        mock_metrics_incr: mock.MagicMock,
+    ) -> None:
         # Two suggested groups, one with valid data, one missing parent hash. We should log the
         # second and return the first.
         seer_return_value: Any = {
@@ -438,11 +372,11 @@ class GroupSimilarIssuesEmbeddingsTest(APITestCase):
     @mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
     def test_nonexistent_grouphash(
         self,
-        mock_seer_similarity_request,
-        mock_logger,
-        mock_metrics_incr,
-        mock_seer_deletion_request,
-    ):
+        mock_seer_similarity_request: mock.MagicMock,
+        mock_logger: mock.MagicMock,
+        mock_metrics_incr: mock.MagicMock,
+        mock_seer_deletion_request: mock.MagicMock,
+    ) -> None:
         """
         The seer API can return grouphashes that do not exist if their groups have been deleted/merged.
         Test info about these groups is not returned.
@@ -499,11 +433,11 @@ class GroupSimilarIssuesEmbeddingsTest(APITestCase):
     @mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
     def test_grouphash_with_no_group(
         self,
-        mock_seer_similarity_request,
-        mock_logger,
-        mock_metrics_incr,
-        mock_seer_deletion_request,
-    ):
+        mock_seer_similarity_request: mock.MagicMock,
+        mock_logger: mock.MagicMock,
+        mock_metrics_incr: mock.MagicMock,
+        mock_seer_deletion_request: mock.MagicMock,
+    ) -> None:
         """
         The seer API can return groups that do not exist if they have been deleted/merged.
         Test that these groups are not returned.
@@ -549,7 +483,9 @@ class GroupSimilarIssuesEmbeddingsTest(APITestCase):
 
     @mock.patch("sentry.analytics.record")
     @mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
-    def test_empty_seer_return(self, mock_seer_request, mock_record):
+    def test_empty_seer_return(
+        self, mock_seer_request: mock.MagicMock, mock_record: mock.MagicMock
+    ) -> None:
         mock_seer_request.return_value = HTTPResponse([], status=200)
         response = self.client.get(self.path)
         assert response.data == []
@@ -564,7 +500,7 @@ class GroupSimilarIssuesEmbeddingsTest(APITestCase):
             user_id=self.user.id,
         )
 
-    def test_no_contributing_exception(self):
+    def test_no_contributing_exception(self) -> None:
         data_no_contributing_exception = {
             "fingerprint": ["message"],
             "message": "Message",
@@ -604,7 +540,7 @@ class GroupSimilarIssuesEmbeddingsTest(APITestCase):
 
         assert response.data == []
 
-    def test_no_exception(self):
+    def test_no_exception(self) -> None:
         event_no_exception = self.store_event(data={}, project_id=self.project)
         group_no_exception = event_no_exception.group
         assert group_no_exception
@@ -616,7 +552,7 @@ class GroupSimilarIssuesEmbeddingsTest(APITestCase):
         assert response.data == []
 
     @mock.patch("sentry.models.group.Group.get_latest_event")
-    def test_no_latest_event(self, mock_get_latest_event):
+    def test_no_latest_event(self, mock_get_latest_event: mock.MagicMock) -> None:
         mock_get_latest_event.return_value = None
 
         response = self.client.get(
@@ -627,7 +563,7 @@ class GroupSimilarIssuesEmbeddingsTest(APITestCase):
         assert response.data == []
 
     @mock.patch("sentry.issues.endpoints.group_similar_issues_embeddings.get_stacktrace_string")
-    def test_no_stacktrace_string(self, mock_get_stacktrace_string):
+    def test_no_stacktrace_string(self, mock_get_stacktrace_string: mock.MagicMock) -> None:
         mock_get_stacktrace_string.return_value = ""
 
         response = self.client.get(
@@ -638,7 +574,7 @@ class GroupSimilarIssuesEmbeddingsTest(APITestCase):
         assert response.data == []
 
     @mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
-    def test_no_optional_params(self, mock_seer_request):
+    def test_no_optional_params(self, mock_seer_request: mock.MagicMock) -> None:
         """
         Test that optional parameters, k, threshold, and read_only can not be included.
         """
@@ -739,7 +675,7 @@ class GroupSimilarIssuesEmbeddingsTest(APITestCase):
         )
 
     @mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
-    def test_obeys_useReranking_query_param(self, mock_seer_request):
+    def test_obeys_useReranking_query_param(self, mock_seer_request: mock.MagicMock) -> None:
         for incoming_value, outgoing_value in [("true", True), ("false", False)]:
             self.client.get(self.path, data={"useReranking": incoming_value})
 
@@ -749,7 +685,7 @@ class GroupSimilarIssuesEmbeddingsTest(APITestCase):
 
             mock_seer_request.reset_mock()
 
-    def test_too_many_system_frames(self):
+    def test_too_many_system_frames(self) -> None:
         type = "FailedToFetchError"
         value = "Charlie didn't bring the ball back"
         context_line = f"raise {type}('{value}')"
@@ -782,7 +718,7 @@ class GroupSimilarIssuesEmbeddingsTest(APITestCase):
         )
         assert response.data == []
 
-    def test_no_filename_or_module(self):
+    def test_no_filename_or_module(self) -> None:
         type = "FailedToFetchError"
         value = "Charlie didn't bring the ball back"
         context_line = f"raise {type}('{value}')"

+ 9 - 5
tests/sentry/issues/endpoints/test_shared_group_details.py

@@ -1,3 +1,5 @@
+from collections.abc import Callable
+
 from sentry.models.groupshare import GroupShare
 from sentry.testutils.cases import APITestCase
 from sentry.testutils.helpers.datetime import before_now
@@ -7,7 +9,9 @@ pytestmark = [requires_snuba]
 
 
 class SharedGroupDetailsTest(APITestCase):
-    def _get_path_functions(self):
+    def _get_path_functions(
+        self,
+    ) -> tuple[Callable[[str], str], Callable[[str], str], Callable[[str], str]]:
         # The urls for shared group details are supported both with an org slug and without.
         # We test both as long as we support both.
         # Because removing old urls takes time and consideration of the cost of breaking lingering references, a
@@ -18,7 +22,7 @@ class SharedGroupDetailsTest(APITestCase):
             lambda share_id: f"/api/0/organizations/{self.organization.id}/shared/issues/{share_id}/",
         )
 
-    def test_simple(self):
+    def test_simple(self) -> None:
         self.login_as(user=self.user)
 
         min_ago = before_now(minutes=1).isoformat()
@@ -44,7 +48,7 @@ class SharedGroupDetailsTest(APITestCase):
             assert response.data["project"]["slug"] == group.project.slug
             assert response.data["project"]["organization"]["slug"] == group.organization.slug
 
-    def test_does_not_leak_assigned_to(self):
+    def test_does_not_leak_assigned_to(self) -> None:
         self.login_as(user=self.user)
 
         min_ago = before_now(minutes=1).isoformat()
@@ -71,7 +75,7 @@ class SharedGroupDetailsTest(APITestCase):
             assert response.data["project"]["organization"]["slug"] == group.organization.slug
             assert "assignedTo" not in response.data
 
-    def test_feature_disabled(self):
+    def test_feature_disabled(self) -> None:
         self.login_as(user=self.user)
 
         group = self.create_group()
@@ -93,7 +97,7 @@ class SharedGroupDetailsTest(APITestCase):
 
             assert response.status_code == 404
 
-    def test_permalink(self):
+    def test_permalink(self) -> None:
         group = self.create_group()
 
         share_id = group.get_share_id()