Browse Source

chore(hybridcloud) Remove mocks from bitbucket parser test and add parser validation (#62868)

- Convert the bitbucket parser tests to not use mocks.
- Add tests to ensure that all routes under /extensions have a parser
module that is registered with the `IntegrationClassifier`.

Refs HC-815
Mark Story 1 year ago
parent
commit
88bc44bb1d

+ 41 - 27
tests/sentry/middleware/integrations/parsers/test_bitbucket.py

@@ -1,39 +1,43 @@
-from unittest import mock
-from unittest.mock import MagicMock
-
-from django.http import HttpResponse
+from django.http import HttpRequest, HttpResponse
 from django.test import RequestFactory, override_settings
 from django.urls import reverse
 
 from sentry.middleware.integrations.parsers.bitbucket import BitbucketRequestParser
+from sentry.models.integrations.integration import Integration
 from sentry.models.organizationmapping import OrganizationMapping
 from sentry.models.outbox import WebhookProviderIdentifier
 from sentry.silo.base import SiloMode
 from sentry.testutils.cases import TestCase
-from sentry.testutils.outbox import assert_webhook_outboxes
+from sentry.testutils.outbox import assert_no_webhook_outboxes, assert_webhook_outboxes
 from sentry.testutils.region import override_regions
 from sentry.testutils.silo import control_silo_test
 from sentry.types.region import Region, RegionCategory
 
+region = Region("us", 1, "http://us.testserver", RegionCategory.MULTI_TENANT)
+region_config = (region,)
+
 
 @control_silo_test
 class BitbucketRequestParserTest(TestCase):
-    get_response = MagicMock(return_value=HttpResponse(content=b"no-error", status=200))
+    def get_response(self, req: HttpRequest) -> HttpResponse:
+        return HttpResponse(status=200, content="passthrough")
+
     factory = RequestFactory()
     region = Region("us", 1, "https://us.testserver", RegionCategory.MULTI_TENANT)
     region_config = (region,)
 
     def setUp(self):
         super().setUp()
-        self.path = reverse(
-            "sentry-extensions-bitbucket-webhook", kwargs={"organization_id": self.organization.id}
-        )
-        self.integration = self.create_integration(
+
+    def get_integration(self) -> Integration:
+        return self.create_integration(
             organization=self.organization, external_id="bitbucket:1", provider="bitbucket"
         )
 
     @override_settings(SILO_MODE=SiloMode.CONTROL)
+    @override_regions(region_config)
     def test_routing_endpoints(self):
+        self.get_integration()
         control_routes = [
             reverse("sentry-extensions-bitbucket-descriptor"),
             reverse("sentry-extensions-bitbucket-installed"),
@@ -49,16 +53,16 @@ class BitbucketRequestParserTest(TestCase):
         for route in control_routes:
             request = self.factory.post(route)
             parser = BitbucketRequestParser(request=request, response_handler=self.get_response)
-            with mock.patch.object(
-                parser, "get_response_from_control_silo"
-            ) as get_response_from_control_silo:
-                assert not get_response_from_control_silo.called
-                parser.get_response()
-                assert get_response_from_control_silo.called
+            response = parser.get_response()
+
+            assert isinstance(response, HttpResponse)
+            assert response.status_code == 200
+            assert response.content == b"passthrough"
+            assert_no_webhook_outboxes()
 
-    @override_regions(region_config)
     @override_settings(SILO_MODE=SiloMode.CONTROL)
-    def test_routing_webhook(self):
+    @override_regions(region_config)
+    def test_routing_webhook_no_regions(self):
         region_route = reverse(
             "sentry-extensions-bitbucket-webhook", kwargs={"organization_id": self.organization.id}
         )
@@ -69,17 +73,27 @@ class BitbucketRequestParserTest(TestCase):
         OrganizationMapping.objects.get(organization_id=self.organization.id).update(
             region_name="eu"
         )
-        with mock.patch.object(
-            parser, "get_response_from_control_silo"
-        ) as get_response_from_control_silo:
-            parser.get_response()
-            assert get_response_from_control_silo.called
+        response = parser.get_response()
 
-        # Valid region
-        OrganizationMapping.objects.get(organization_id=self.organization.id).update(
-            region_name="us"
+        assert isinstance(response, HttpResponse)
+        assert response.status_code == 200
+        assert response.content == b"passthrough"
+        assert_no_webhook_outboxes()
+
+    @override_settings(SILO_MODE=SiloMode.CONTROL)
+    @override_regions(region_config)
+    def test_routing_webhook_with_regions(self):
+        self.get_integration()
+        region_route = reverse(
+            "sentry-extensions-bitbucket-webhook", kwargs={"organization_id": self.organization.id}
         )
-        parser.get_response()
+        request = self.factory.post(region_route)
+        parser = BitbucketRequestParser(request=request, response_handler=self.get_response)
+
+        response = parser.get_response()
+        assert isinstance(response, HttpResponse)
+        assert response.status_code == 202
+        assert response.content == b""
         assert_webhook_outboxes(
             factory_request=request,
             webhook_identifier=WebhookProviderIdentifier.BITBUCKET,

+ 57 - 0
tests/sentry/middleware/integrations/test_parsers_defined.py

@@ -0,0 +1,57 @@
+import importlib
+
+from django.http import HttpResponse
+from django.urls import URLResolver, get_resolver
+
+from sentry.middleware.integrations.classifications import IntegrationClassification
+
+
+def extract_matching_url_patterns(urlpatterns, base: str = ""):
+    for pattern in urlpatterns:
+        if isinstance(pattern, URLResolver):
+            yield from extract_matching_url_patterns(
+                pattern.url_patterns, base + str(pattern.pattern)
+            )
+        else:
+            url_pattern = base + str(pattern.pattern)
+            url_pattern = url_pattern.replace("/^", "/")
+            if "extensions/" in url_pattern:
+                yield url_pattern
+
+
+def test_parsers_for_all_extension_urls():
+    """
+    Ensure that we have a request parser for all defined integrations.
+    """
+    classifier = IntegrationClassification(lambda req: HttpResponse())
+    url_patterns = extract_matching_url_patterns(get_resolver().url_patterns)
+    for pattern in url_patterns:
+        [_, provider, _trailing] = pattern.split("/", maxsplit=2)
+
+        # Ignore dynamic segments
+        if provider[0] in {"(", "["} or provider == "external-install":
+            continue
+
+        # Ensure the expected module exists
+        provider = provider.replace("-", "_")
+        parser_module = f"sentry.middleware.integrations.parsers.{provider}"
+        try:
+            importlib.import_module(parser_module)
+        except ImportError:
+            msg = f"""
+        Could not import parser module `{parser_module}` for provider `{provider}`.
+
+        Without a request parser for the {provider} integration, the integration
+        will not behave correctly in saas.
+            """
+            assert False, msg
+
+        # Ensure that the parser is wired into the integration classifier
+        msg = f"""
+        Could not access the parser for {provider}.
+
+        Ensure that the {provider} is registered with
+
+        sentry.middleware.integrations.classifications.IntegrationClassification
+        """
+        assert classifier.integration_parsers.get(provider), msg