test(hc): Check for inheritance in silo-decorated test classes (#64733)

Fix failure modes when inheriting from a test class with a silo mode
decorator on it. Enforce that subclasses must have their own silo mode
decorators. Add new decorators to test case subclasses where required.

Delete the unused `_validate_that_no_ancestor_is_silo_decorated` method.
It's unlikely to be used again, as the thing it was prohibiting is now
mandatory.

src/sentry/testutils/silo.py

@@ -111,7 +111,7 @@ def _model_silo_limit(t: type[Model]) -> ModelSiloLimit:
     return silo_limit
 
 
-class AncestorAlreadySiloDecoratedException(Exception):
+class SubclassNotSiloDecoratedException(Exception):
     pass
 
 
@@ -192,20 +192,47 @@ class _SiloModeTestModification:
     def _create_overriding_test_class(
         self, test_class: type[TestCase], silo_mode: SiloMode, name_suffix: str = ""
     ) -> type[TestCase]:
-        def override_method(method_name: str) -> Callable[..., Any]:
-            context = self.test_config(silo_mode)
-            method: Callable[..., Any] = getattr(test_class, method_name)
-            return context(method)
+        silo_mode_attr = "__silo_mode_override"
+
+        @contextmanager
+        def create_context(obj: TestCase) -> Generator[None, None, None]:
+            tagged_class, tagged_mode = getattr(obj, silo_mode_attr)
+
+            if type(obj) is not tagged_class:
+                # This condition indicates that the test case inherits the silo mode
+                # attribute from a superclass. Although we could just test in that
+                # mode, doing so would silently skip other modes if the superclass is
+                # supposed to be tested in more than one mode. So, enforce a general
+                # rule that test case subclasses must have decorators of their own.
+                sup = tagged_class.__name__
+                sub = type(obj).__name__
+                raise SubclassNotSiloDecoratedException(
+                    f"A test class ({sub}) extends a silo-decorated test class ({sup}) "
+                    f"without a silo decorator of its own. Add a decorator to {sub}. "
+                    f"(You probably want to copy and paste the decorator from {sup}. "
+                    f"If you don't want to run {sub} in a silo mode at all, use "
+                    f"`@no_silo_test`.)"
+                )
+
+            with self.test_config(tagged_mode):
+                yield
 
         # Unfortunately, due to the way DjangoTestCase setup and app manipulation works, `override_settings` in a
         # run method produces unusual, broken results.  We're forced to wrap the hidden methods that invoke setup
         # test method in order to use override_settings correctly in django test cases.
-        new_methods = {
-            method_name: override_method(method_name)
-            for method_name in ("_callSetUp", "_callTestMethod")
-        }
+
+        def _callSetUp(obj: TestCase) -> Any:
+            with create_context(obj):
+                return TestCase._callSetUp(obj)  # type: ignore[attr-defined]
+
+        def _callTestMethod(obj: TestCase, method: Any) -> Any:
+            with create_context(obj):
+                return TestCase._callTestMethod(obj, method)  # type: ignore[attr-defined]
+
+        new_methods = {"_callSetUp": _callSetUp, "_callTestMethod": _callTestMethod}
         name = test_class.__name__ + name_suffix
         new_class = type(name, (test_class,), new_methods)
+        setattr(new_class, silo_mode_attr, (new_class, silo_mode))
         return cast(type[TestCase], new_class)
 
     def _arrange_silo_modes(self) -> tuple[SiloMode, Collection[SiloMode]]:
@@ -274,32 +301,6 @@ class _SiloModeTestModification:
 
         return self._mark_parameterized_by_silo_mode(decorated_obj)
 
-    def _validate_that_no_ancestor_is_silo_decorated(self, object_to_validate: Any):
-        # Deprecated? Silo decorators at multiple places in the inheritance tree may
-        # be necessary if a base class needs to be run in a non-default mode,
-        # especially when the default is no longer region mode. The previous
-        # rationale may have been limited to problems around swapping the local
-        # region, which may now be resolved.
-        #
-        # TODO(RyanSkonnord): Delete this after ascertaining that it's safe to have
-        #  silo decorators on test case class ancestors
-
-        class_queue = [object_to_validate]
-
-        # Do a breadth-first traversal of all base classes to ensure that the
-        #  object does not inherit from a class which has already been decorated,
-        #  even in multi-inheritance scenarios.
-        while len(class_queue) > 0:
-            current_class = class_queue.pop(0)
-            if getattr(current_class, "_silo_modes", None):
-                raise AncestorAlreadySiloDecoratedException(
-                    f"Cannot decorate class '{object_to_validate.__name__}', "
-                    f"which inherits from a silo decorated class ({current_class.__name__})"
-                )
-            class_queue.extend(current_class.__bases__)
-
-        object_to_validate._silo_modes = self.silo_modes
-
 
 all_silo_test = SiloModeTestDecorator(*SiloMode)
 """

tests/sentry/api/endpoints/test_organization_member_team_details.py

@@ -299,6 +299,7 @@ class CreateWithOpenMembershipTest(OrganizationMemberTeamTestBase):
         ).exists()
 
 
+@region_silo_test
 class CreateWithClosedMembershipTest(CreateOrganizationMemberTeamTest):
     @cached_property
     def org(self):

tests/sentry/api/endpoints/test_userroles_details.py

@@ -31,6 +31,7 @@ class UserRolesDetailsTest(APITestCase):
         assert resp.status_code == 403
 
 
+@control_silo_test
 class UserRolesDetailsGetTest(UserRolesDetailsTest):
     def test_simple(self):
         self.create_user_role(name="test-role")
@@ -40,6 +41,7 @@ class UserRolesDetailsGetTest(UserRolesDetailsTest):
         assert resp.data["name"] == "test-role"
 
 
+@control_silo_test
 class UserRolesDetailsPutTest(UserRolesDetailsTest):
     method = "PUT"
 
@@ -55,6 +57,7 @@ class UserRolesDetailsPutTest(UserRolesDetailsTest):
         assert role2.permissions == ["users.edit"]
 
 
+@control_silo_test
 class UserRolesDetailsDeleteTest(UserRolesDetailsTest):
     method = "DELETE"
 

tests/sentry/integrations/github/test_client.py

@@ -867,6 +867,7 @@ class GitHubClientFileBlameBase(TestCase):
         )
 
 
+@region_silo_test
 class GitHubClientFileBlameQueryBuilderTest(GitHubClientFileBlameBase):
     """
     Tests that get_blame_for_files builds the correct GraphQL query
@@ -1209,6 +1210,7 @@ class GitHubClientFileBlameQueryBuilderTest(GitHubClientFileBlameBase):
         assert json.loads(responses.calls[1].request.body)["query"] == query
 
 
+@region_silo_test
 class GitHubClientFileBlameResponseTest(GitHubClientFileBlameBase):
     """
     Tests that get_blame_for_files handles the GraphQL response correctly
@@ -1623,6 +1625,7 @@ class GitHubClientFileBlameResponseTest(GitHubClientFileBlameBase):
         )
 
 
+@region_silo_test
 class GitHubClientFileBlameRateLimitTest(GitHubClientFileBlameBase):
     """
     Tests that rate limits are handled correctly

tests/sentry/integrations/github_enterprise/test_search.py

@@ -1,8 +1,11 @@
 from datetime import datetime, timedelta
 
+from sentry.testutils.silo import control_silo_test
+
 from ..github import test_search
 
 
+@control_silo_test
 class GithubEnterpriseSearchTest(test_search.GithubSearchTest):
     # Inherit test methods/scenarios from GithubSearchTest
     # and fill out the slots that customize it to use github:enterprise

tests/sentry/integrations/jira/test_sentry_installation.py

@@ -24,6 +24,7 @@ class JiraSentryInstallationViewTestCase(APITestCase):
         self.integration = self.create_provider_integration(provider="jira", name="Example Jira")
 
 
+@control_silo_test
 class JiraSentryInstallationViewErrorsTest(JiraSentryInstallationViewTestCase):
     @patch(
         "sentry.integrations.jira.views.sentry_installation.get_integration_from_request",
@@ -44,6 +45,7 @@ class JiraSentryInstallationViewErrorsTest(JiraSentryInstallationViewTestCase):
         assert UNABLE_TO_VERIFY_INSTALLATION.encode() in response.content
 
 
+@control_silo_test
 class JiraSentryInstallationViewTest(JiraSentryInstallationViewTestCase):
     def setUp(self):
         super().setUp()

tests/sentry/integrations/vercel/test_webhook.py

@@ -304,6 +304,7 @@ class VercelReleasesTest(APITestCase):
         assert "Could not determine repository" == response.data["detail"]
 
 
+@control_silo_test
 class VercelReleasesNewTest(VercelReleasesTest):
     webhook_url = "/extensions/vercel/delete/"
 

tests/snuba/api/endpoints/test_organization_events_mep.py

@@ -3126,6 +3126,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTest(MetricsEnhancedPe
         assert not meta["isMetricsData"]
 
 
+@region_silo_test
 class OrganizationEventsMetricsEnhancedPerformanceEndpointTestWithOnDemandMetrics(
     MetricsEnhancedPerformanceTestCase
 ):
@@ -3205,6 +3206,7 @@ class OrganizationEventsMetricsEnhancedPerformanceEndpointTestWithOnDemandMetric
         }
 
 
+@region_silo_test
 class OrganizationEventsMetricsEnhancedPerformanceEndpointTestWithMetricLayer(
     OrganizationEventsMetricsEnhancedPerformanceEndpointTest
 ):

tests/snuba/api/endpoints/test_organization_events_stats_mep.py

@@ -766,6 +766,7 @@ class OrganizationEventsStatsMetricsEnhancedPerformanceEndpointTest(
         assert data["order"] == 0
 
 
+@region_silo_test
 class OrganizationEventsStatsMetricsEnhancedPerformanceEndpointTestWithMetricLayer(
     OrganizationEventsStatsMetricsEnhancedPerformanceEndpointTest
 ):

tests/snuba/api/endpoints/test_organization_events_stats_span_metrics.py

@@ -273,6 +273,7 @@ class OrganizationEventsStatsSpansMetricsEndpointTest(MetricsEnhancedPerformance
         assert data[1][1][0]["count"] == 4.0
 
 
+@region_silo_test
 class OrganizationEventsStatsSpansMetricsEndpointTestWithMetricLayer(
     OrganizationEventsStatsSpansMetricsEndpointTest
 ):