test(hc): Generate silo-mode tests at class level (#53028)

Instead of adding siloed test methods within a class, generate a new
siloed test case class. Previously, the setup and teardown methods were
always run in monolith mode. This change ensures that the entire test
run including setup runs in the proper mode.

Add the option to inject a set of Region objects at the decorator level,
which overrides the global region map during setup.

Fix tests that now require stricter setup.

---------

Co-authored-by: Zachary Collins <zachary.collins@sentry.io>

fixtures/gitlab.py

@@ -1,7 +1,9 @@
 from time import time
 
 from sentry.models import Identity, IdentityProvider, Integration, Repository
+from sentry.silo import SiloMode
 from sentry.testutils import APITestCase
+from sentry.testutils.silo import assume_test_silo_mode
 
 EXTERNAL_ID = "example.gitlab.com:group-x"
 WEBHOOK_SECRET = "secret-token-value"
@@ -13,28 +15,34 @@ class GitLabTestCase(APITestCase):
 
     def setUp(self):
         self.login_as(self.user)
-        self.integration = Integration.objects.create(
-            provider=self.provider,
-            name="Example Gitlab",
-            external_id=EXTERNAL_ID,
-            metadata={
-                "instance": "example.gitlab.com",
-                "base_url": "https://example.gitlab.com",
-                "domain_name": "example.gitlab.com/group-x",
-                "verify_ssl": False,
-                "webhook_secret": WEBHOOK_SECRET,
-                "group_id": 1,
-            },
-        )
-        identity = Identity.objects.create(
-            idp=IdentityProvider.objects.create(type=self.provider, config={}),
-            user=self.user,
-            external_id="gitlab123",
-            data={"access_token": "123456789", "created_at": time(), "refresh_token": "0987654321"},
-        )
-        self.integration.add_organization(self.organization, self.user, identity.id)
+        with assume_test_silo_mode(SiloMode.CONTROL):
+            self.integration = Integration.objects.create(
+                provider=self.provider,
+                name="Example Gitlab",
+                external_id=EXTERNAL_ID,
+                metadata={
+                    "instance": "example.gitlab.com",
+                    "base_url": "https://example.gitlab.com",
+                    "domain_name": "example.gitlab.com/group-x",
+                    "verify_ssl": False,
+                    "webhook_secret": WEBHOOK_SECRET,
+                    "group_id": 1,
+                },
+            )
+            identity = Identity.objects.create(
+                idp=IdentityProvider.objects.create(type=self.provider, config={}),
+                user=self.user,
+                external_id="gitlab123",
+                data={
+                    "access_token": "123456789",
+                    "created_at": time(),
+                    "refresh_token": "0987654321",
+                },
+            )
+            self.integration.add_organization(self.organization, self.user, identity.id)
         self.installation = self.integration.get_installation(self.organization.id)
 
+    @assume_test_silo_mode(SiloMode.REGION)
     def create_repo(self, name, external_id=15, url=None, organization_id=None):
         instance = self.integration.metadata["instance"]
         organization_id = organization_id or self.organization.id

src/sentry/integrations/msteams/utils.py

@@ -53,15 +53,17 @@ def get_user_conversation_id(integration: Integration, user_id: str) -> str:
 
 
 def get_channel_id(organization, integration_id, name):
-    try:
-        integration = Integration.objects.get(
-            provider="msteams",
-            organizationintegration__organization_id=organization.id,
-            id=integration_id,
-        )
-    except Integration.DoesNotExist:
+    integrations = integration_service.get_integrations(
+        providers=["msteams"],
+        organization_id=organization.id,
+        integration_ids=[integration_id],
+    )
+    if not integrations:
         return None
 
+    assert len(integrations) == 1, "Found multiple msteams integrations for org!"
+    integration = integrations[0]
+
     team_id = integration.external_id
     client = MsTeamsClient(integration)
 

src/sentry/interfaces/stacktrace.py

@@ -7,7 +7,7 @@ from django.utils.translation import gettext as _
 
 from sentry.app import env
 from sentry.interfaces.base import DataPath, Interface
-from sentry.models import UserOption
+from sentry.services.hybrid_cloud.user_option import get_option_from_list, user_option_service
 from sentry.utils.json import prune_empty_keys
 from sentry.web.helpers import render_to_string
 
@@ -90,9 +90,10 @@ def is_newest_frame_first(event):
     newest_first = event.platform not in ("python", None)
 
     if env.request and env.request.user.is_authenticated:
-        display = UserOption.objects.get_value(
-            user=env.request.user, key="stacktrace_order", default=None
+        options = user_option_service.get_many(
+            filter=dict(user_ids=[env.request.user.id], keys=["stacktrace_order"])
         )
+        display = get_option_from_list(options, default=None)
         if display == "1":
             newest_first = False
         elif display == "2":

src/sentry/testutils/cases.py

@@ -2261,12 +2261,15 @@ class TestMigrations(TransactionTestCase):
 class SCIMTestCase(APITestCase):
     def setUp(self, provider="dummy"):
         super().setUp()
-        self.auth_provider_inst = AuthProviderModel(
-            organization_id=self.organization.id, provider=provider
-        )
-        self.auth_provider_inst.enable_scim(self.user)
-        self.auth_provider_inst.save()
-        self.scim_user = ApiToken.objects.get(token=self.auth_provider_inst.get_scim_token()).user
+        with assume_test_silo_mode(SiloMode.CONTROL):
+            self.auth_provider_inst = AuthProviderModel(
+                organization_id=self.organization.id, provider=provider
+            )
+            self.auth_provider_inst.enable_scim(self.user)
+            self.auth_provider_inst.save()
+            self.scim_user = ApiToken.objects.get(
+                token=self.auth_provider_inst.get_scim_token()
+            ).user
         self.login_as(user=self.scim_user)
 
 
@@ -2278,6 +2281,7 @@ class SCIMAzureTestCase(SCIMTestCase):
 
 
 class ActivityTestCase(TestCase):
+    @assume_test_silo_mode(SiloMode.CONTROL)
     def another_user(self, email_string, team=None, alt_email_string=None):
         user = self.create_user(email_string)
         if alt_email_string:
@@ -2334,34 +2338,37 @@ class SlackActivityNotificationTest(ActivityTestCase):
         return mail_adapter
 
     def setUp(self):
-        NotificationSetting.objects.update_settings(
-            ExternalProviders.SLACK,
-            NotificationSettingTypes.WORKFLOW,
-            NotificationSettingOptionValues.ALWAYS,
-            user_id=self.user.id,
-        )
-        NotificationSetting.objects.update_settings(
-            ExternalProviders.SLACK,
-            NotificationSettingTypes.DEPLOY,
-            NotificationSettingOptionValues.ALWAYS,
-            user_id=self.user.id,
-        )
-        NotificationSetting.objects.update_settings(
-            ExternalProviders.SLACK,
-            NotificationSettingTypes.ISSUE_ALERTS,
-            NotificationSettingOptionValues.ALWAYS,
-            user_id=self.user.id,
-        )
-        UserOption.objects.create(user=self.user, key="self_notifications", value="1")
-        self.integration = install_slack(self.organization)
-        self.idp = IdentityProvider.objects.create(type="slack", external_id="TXXXXXXX1", config={})
-        self.identity = Identity.objects.create(
-            external_id="UXXXXXXX1",
-            idp=self.idp,
-            user=self.user,
-            status=IdentityStatus.VALID,
-            scopes=[],
-        )
+        with assume_test_silo_mode(SiloMode.CONTROL):
+            NotificationSetting.objects.update_settings(
+                ExternalProviders.SLACK,
+                NotificationSettingTypes.WORKFLOW,
+                NotificationSettingOptionValues.ALWAYS,
+                user_id=self.user.id,
+            )
+            NotificationSetting.objects.update_settings(
+                ExternalProviders.SLACK,
+                NotificationSettingTypes.DEPLOY,
+                NotificationSettingOptionValues.ALWAYS,
+                user_id=self.user.id,
+            )
+            NotificationSetting.objects.update_settings(
+                ExternalProviders.SLACK,
+                NotificationSettingTypes.ISSUE_ALERTS,
+                NotificationSettingOptionValues.ALWAYS,
+                user_id=self.user.id,
+            )
+            UserOption.objects.create(user=self.user, key="self_notifications", value="1")
+            self.integration = install_slack(self.organization)
+            self.idp = IdentityProvider.objects.create(
+                type="slack", external_id="TXXXXXXX1", config={}
+            )
+            self.identity = Identity.objects.create(
+                external_id="UXXXXXXX1",
+                idp=self.idp,
+                user=self.user,
+                status=IdentityStatus.VALID,
+                scopes=[],
+            )
         responses.add(
             method=responses.POST,
             url="https://slack.com/api/chat.postMessage",

src/sentry/testutils/factories.py

@@ -108,6 +108,7 @@ from sentry.services.hybrid_cloud.hook import hook_service
 from sentry.signals import project_created
 from sentry.silo import SiloMode
 from sentry.snuba.dataset import Dataset
+from sentry.testutils.outbox import outbox_runner
 from sentry.testutils.silo import assume_test_silo_mode
 from sentry.types.activity import ActivityType
 from sentry.types.integrations import ExternalProviders
@@ -1101,7 +1102,7 @@ class Factories:
         return _kwargs
 
     @staticmethod
-    @assume_test_silo_mode(SiloMode.REGION)
+    @assume_test_silo_mode(SiloMode.CONTROL)
     def create_doc_integration(features=None, has_avatar: bool = False, **kwargs) -> DocIntegration:
         doc = DocIntegration.objects.create(**Factories._doc_integration_kwargs(**kwargs))
         if features:
@@ -1391,7 +1392,8 @@ class Factories:
         **integration_params: Any,
     ) -> Integration:
         integration = Integration.objects.create(external_id=external_id, **integration_params)
-        organization_integration = integration.add_organization(organization)
+        with outbox_runner():
+            organization_integration = integration.add_organization(organization)
         organization_integration.update(**(oi_params or {}))
 
         return integration

src/sentry/testutils/helpers/slack.py

@@ -48,6 +48,7 @@ def install_slack(organization: Organization, workspace_id: str = "TXXXXXXX1") -
     return integration
 
 
+@assume_test_silo_mode(SiloMode.CONTROL)
 def add_identity(
     integration: Integration, user: User, external_id: str = "UXXXXXXX1"
 ) -> IdentityProvider:

src/sentry/testutils/silo.py

@@ -3,6 +3,8 @@ from __future__ import annotations
 import functools
 import inspect
 import re
+import sys
+import unittest.result
 from contextlib import contextmanager
 from typing import (
     Any,
@@ -38,11 +40,11 @@ from sentry.utils.snowflake import SnowflakeIdMixin
 
 TestMethod = Callable[..., None]
 
-region_map = [
+_DEFAULT_TEST_REGIONS = (
     Region("na", 1, "http://na.testserver", RegionCategory.MULTI_TENANT),
     Region("eu", 2, "http://eu.testserver", RegionCategory.MULTI_TENANT),
     Region("acme-single-tenant", 3, "acme.my.sentry.io", RegionCategory.SINGLE_TENANT),
-]
+)
 
 
 def _model_silo_limit(t: type[Model]) -> ModelSiloLimit:
@@ -54,85 +56,130 @@ def _model_silo_limit(t: type[Model]) -> ModelSiloLimit:
     return silo_limit
 
 
-class SiloModeTest:
+class _SiloModeTestCase(TestCase):
+    """A test case that is expected to work in a particular silo mode.
+
+    This class is meant to be extended by test cases tagged with a
+    SiloModeTestDecorator. It should not be declared explicitly as a superclass,
+    but is used to dynamically generate a new test case class (see
+    SiloModeTestDecorator._add_siloed_test_classes_to_module).
+
+    The subclass will apply the silo mode context to the entire test run, including
+    setup.
+    """
+
+    # Expect these class-level attributes to be set when a subclass is
+    # dynamically generated
+    silo_mode: SiloMode
+    regions: Sequence[Region]
+    is_acceptance_test: bool
+
+    def run(
+        self, result: unittest.result.TestResult | None = None
+    ) -> unittest.result.TestResult | None:
+        with override_settings(
+            SILO_MODE=self.silo_mode,
+            SINGLE_SERVER_SILO_MODE=self.is_acceptance_test,
+            SENTRY_SUBNET_SECRET="secret",
+            SENTRY_CONTROL_ADDRESS="http://controlserver/",
+        ):
+            with override_regions(self.regions):
+                if self.silo_mode == SiloMode.REGION:
+                    with override_settings(SENTRY_REGION=self.regions[0].name):
+                        return super().run(result)
+                else:
+                    return super().run(result)
+
+
+class SiloModeTestDecorator:
     """Decorate a test case that is expected to work in a given silo mode.
 
     Tests marked to work in monolith mode are always executed.
-    Tests marked additionally to work in silo or control mode only do so when the test is marked as stable=True
+    Tests marked additionally to work in region or control mode only do so when the test is marked as stable=True
+
+    When testing in a silo mode, if the decorator is on a test case class,
+    an additional class is dynamically generated and added to the module for Pytest
+    to pick up. For example, if you write
+
+    ```
+        @control_silo_test(stable=True)
+        class MyTest(TestCase):
+            def setUp(self):      ...
+            def test_stuff(self): ...
+    ```
+
+    then your result set should include test runs for both `MyTest` (in monolith
+    mode) and `MyTest__InControlMode`.
     """
 
     def __init__(self, *silo_modes: SiloMode) -> None:
-        self.silo_modes = frozenset(silo_modes)
+        self.silo_modes = frozenset(sm for sm in silo_modes if sm != SiloMode.MONOLITH)
 
     @staticmethod
-    def _find_all_test_methods(test_class: type) -> Iterable[TestMethod]:
-        for attr_name in dir(test_class):
-            if attr_name.startswith("test_") or attr_name == "test":
-                attr = getattr(test_class, attr_name)
-                if callable(attr):
-                    yield attr
-
-    def _is_acceptance_test(self, test_class: type) -> bool:
+    def _is_acceptance_test(test_class: type) -> bool:
         from sentry.testutils import AcceptanceTestCase
 
         return issubclass(test_class, AcceptanceTestCase)
 
-    def _create_mode_methods(
-        self, test_class: type, test_method: TestMethod
-    ) -> Iterable[Tuple[str, TestMethod]]:
-        def method_for_mode(mode: SiloMode) -> Iterable[Tuple[str, TestMethod]]:
-            def replacement_test_method(*args: Any, **kwargs: Any) -> None:
-                with override_settings(
-                    SILO_MODE=mode,
-                    SINGLE_SERVER_SILO_MODE=self._is_acceptance_test(test_class),
-                    SENTRY_SUBNET_SECRET="secret",
-                    SENTRY_CONTROL_ADDRESS="http://controlserver/",
-                ):
-                    with override_regions(region_map):
-                        if mode == SiloMode.REGION:
-                            with override_settings(SENTRY_REGION="na"):
-                                test_method(*args, **kwargs)
-                        else:
-                            test_method(*args, **kwargs)
-
-            functools.update_wrapper(replacement_test_method, test_method)
-            modified_name = f"{test_method.__name__}__in_{str(mode).lower()}_silo"
-            replacement_test_method.__name__ = modified_name
-            yield modified_name, replacement_test_method
-
-        for mode in self.silo_modes:
-            # Currently, test classes that are decorated already handle the monolith mode as the default
-            # because the original test method remains -- this is different from the pytest variant
-            # that actually strictly parameterizes the existing test.  This reduces a redundant run of MONOLITH
-            # mode.
-            if mode == SiloMode.MONOLITH:
-                continue
-            yield from method_for_mode(mode)
+    def _add_siloed_test_classes_to_module(
+        self, test_class: type, regions: Sequence[Region] | None
+    ) -> type:
+        is_acceptance_test = self._is_acceptance_test(test_class)
+
+        def create_overriding_test_class(name: str, silo_mode: SiloMode) -> type:
+            return type(
+                name,
+                (test_class, _SiloModeTestCase),
+                {
+                    "silo_mode": silo_mode,
+                    "regions": tuple(regions or _DEFAULT_TEST_REGIONS),
+                    "is_acceptance_test": is_acceptance_test,
+                },
+            )
 
-    def _add_silo_modes_to_methods(self, test_class: type) -> type:
-        for test_method in self._find_all_test_methods(test_class):
-            for (new_method_name, new_test_method) in self._create_mode_methods(
-                test_class, test_method
-            ):
-                setattr(test_class, new_method_name, new_test_method)
-        return test_class
+        for silo_mode in self.silo_modes:
+            silo_mode_name = silo_mode.name[0].upper() + silo_mode.name[1:].lower()
+            siloed_test_class = create_overriding_test_class(
+                f"{test_class.__name__}__In{silo_mode_name}Mode", silo_mode
+            )
 
-    def __call__(self, decorated_obj: Any = None, stable: bool = False) -> Any:
+            module = sys.modules[test_class.__module__]
+            setattr(module, siloed_test_class.__name__, siloed_test_class)
+
+        # Return the value to be wrapped by the original decorator
+        if regions is None:
+            # Pass the original class through, with no modification
+            return test_class
+        else:
+            # Override without changing the original name. We don't need to change
+            # the silo mode, but we do need to override the region config.
+            return create_overriding_test_class(test_class.__name__, SiloMode.MONOLITH)
+
+    def __call__(
+        self,
+        decorated_obj: Any = None,
+        stable: bool = False,
+        regions: Sequence[Region] | None = None,
+    ) -> Any:
         if decorated_obj:
-            return self._call(decorated_obj, stable)
+            return self._call(decorated_obj, stable, regions)
 
         def receive_decorated_obj(f: Any) -> Any:
-            return self._call(f, stable)
+            return self._call(f, stable, regions)
 
         return receive_decorated_obj
 
-    def _mark_parameterized_by_silo_mode(self, test_method: TestMethod) -> TestMethod:
+    def _mark_parameterized_by_silo_mode(
+        self, test_method: TestMethod, regions: Sequence[Region] | None
+    ) -> TestMethod:
+        regions = tuple(regions or _DEFAULT_TEST_REGIONS)
+
         def replacement_test_method(*args: Any, **kwargs: Any) -> None:
             silo_mode = kwargs.pop("silo_mode")
             with override_settings(SILO_MODE=silo_mode):
-                with override_regions(region_map):
+                with override_regions(regions):
                     if silo_mode == SiloMode.REGION:
-                        with override_settings(SENTRY_REGION="na"):
+                        with override_settings(SENTRY_REGION=regions[0].name):
                             test_method(*args, **kwargs)
                     else:
                         test_method(*args, **kwargs)
@@ -149,7 +196,7 @@ class SiloModeTest:
             new_test_method
         )
 
-    def _call(self, decorated_obj: Any, stable: bool) -> Any:
+    def _call(self, decorated_obj: Any, stable: bool, regions: Sequence[Region] | None) -> Any:
         is_test_case_class = isinstance(decorated_obj, type) and issubclass(decorated_obj, TestCase)
         is_function = inspect.isfunction(decorated_obj)
 
@@ -162,15 +209,15 @@ class SiloModeTest:
             return decorated_obj
 
         if is_test_case_class:
-            return self._add_silo_modes_to_methods(decorated_obj)
+            return self._add_siloed_test_classes_to_module(decorated_obj, regions)
 
-        return self._mark_parameterized_by_silo_mode(decorated_obj)
+        return self._mark_parameterized_by_silo_mode(decorated_obj, regions)
 
 
-all_silo_test = SiloModeTest(SiloMode.CONTROL, SiloMode.REGION, SiloMode.MONOLITH)
-no_silo_test = SiloModeTest(SiloMode.MONOLITH)
-control_silo_test = SiloModeTest(SiloMode.CONTROL, SiloMode.MONOLITH)
-region_silo_test = SiloModeTest(SiloMode.REGION, SiloMode.MONOLITH)
+all_silo_test = SiloModeTestDecorator(SiloMode.CONTROL, SiloMode.REGION)
+no_silo_test = SiloModeTestDecorator()
+control_silo_test = SiloModeTestDecorator(SiloMode.CONTROL)
+region_silo_test = SiloModeTestDecorator(SiloMode.REGION)
 
 
 @contextmanager

tests/sentry/api/endpoints/test_accept_organization_invite.py

@@ -1,5 +1,6 @@
 from datetime import timedelta
 
+from django.conf import settings
 from django.db import router
 from django.db.models import F
 from django.test import override_settings
@@ -31,7 +32,8 @@ from sentry.types.region import Region, RegionCategory
 class AcceptInviteTest(TestCase, HybridCloudTestMixin):
     def setUp(self):
         super().setUp()
-        self.organization = self.create_organization(owner=self.create_user("foo@example.com"))
+        with override_settings(SENTRY_REGION=settings.SENTRY_MONOLITH_REGION):
+            self.organization = self.create_organization(owner=self.create_user("foo@example.com"))
         self.user = self.create_user("bar@example.com")
 
     def _get_paths(self, args):

tests/sentry/api/endpoints/test_group_notes_details.py

@@ -4,8 +4,9 @@ from unittest.mock import patch
 import responses
 
 from sentry.models import Activity, ExternalIssue, Group, GroupLink, Integration
+from sentry.silo import SiloMode
 from sentry.testutils import APITestCase
-from sentry.testutils.silo import region_silo_test
+from sentry.testutils.silo import assume_test_silo_mode, region_silo_test
 
 
 @region_silo_test(stable=True)
@@ -14,12 +15,15 @@ class GroupNotesDetailsTest(APITestCase):
         super().setUp()
         self.activity.data["external_id"] = "123"
         self.activity.save()
-        self.integration = Integration.objects.create(
-            provider="example", external_id="example12345", name="Example 12345"
-        )
-        org_integration = self.integration.add_organization(self.organization)
-        org_integration.config = {"sync_comments": True}
-        org_integration.save()
+
+        with assume_test_silo_mode(SiloMode.CONTROL):
+            self.integration = Integration.objects.create(
+                provider="example", external_id="example12345", name="Example 12345"
+            )
+            org_integration = self.integration.add_organization(self.organization)
+            org_integration.config = {"sync_comments": True}
+            org_integration.save()
+
         self.external_issue = ExternalIssue.objects.create(
             organization_id=self.organization.id, integration_id=self.integration.id, key="123"
         )

tests/sentry/api/endpoints/test_organization_artifactbundle_assemble.py

@@ -19,7 +19,8 @@ from sentry.utils.security.orgauthtoken_token import generate_token, hash_token
 class OrganizationArtifactBundleAssembleTest(APITestCase):
     def setUp(self):
         self.organization = self.create_organization(owner=self.user)
-        self.token = ApiToken.objects.create(user=self.user, scope_list=["project:write"])
+        with assume_test_silo_mode(SiloMode.CONTROL):
+            self.token = ApiToken.objects.create(user=self.user, scope_list=["project:write"])
         self.project = self.create_project()
         self.url = reverse(
             "sentry-api-0-organization-artifactbundle-assemble",