Browse Source

test(hybrid-cloud): Adds silo dectorator inheritance checks (#54201)

Adds a validation when decorating test classes that prevents decorating
multiple tests with a silo decorator in the same inheritance tree, which
can cause test failures in unexpected ways.
Gabe Villalobos 1 year ago
parent
commit
f00c91b71a

+ 23 - 0
src/sentry/testutils/silo.py

@@ -58,6 +58,10 @@ def _model_silo_limit(t: type[Model]) -> ModelSiloLimit:
     return silo_limit
 
 
+class AncestorAlreadySiloDecoratedException(Exception):
+    pass
+
+
 class SiloModeTestDecorator:
     """Decorate a test case that is expected to work in a given silo mode.
 
@@ -199,6 +203,11 @@ class SiloModeTestDecorator:
         if not (is_test_case_class or is_function):
             raise ValueError("@SiloModeTest must decorate a function or TestCase class")
 
+        if is_test_case_class:
+            self._validate_that_no_ancestor_is_silo_decorated(decorated_obj)
+            # _silo_modes is used to mark the class as silo decorated in the above validation
+            decorated_obj._silo_modes = self.silo_modes
+
         # Only run non monolith tests when they are marked stable or we are explicitly running for that mode.
         if not (stable or settings.FORCE_SILOED_TESTS):
             # In this case, simply force the current silo mode (monolith)
@@ -209,6 +218,20 @@ class SiloModeTestDecorator:
 
         return self._mark_parameterized_by_silo_mode(decorated_obj, regions)
 
+    def _validate_that_no_ancestor_is_silo_decorated(self, object_to_validate: Any):
+        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__}', which inherits from a silo decorated class"
+                )
+            class_queue.extend(current_class.__bases__)
+
 
 all_silo_test = SiloModeTestDecorator(SiloMode.CONTROL, SiloMode.REGION)
 """

+ 0 - 1
tests/sentry/api/endpoints/test_api_authorizations.py

@@ -3,7 +3,6 @@ from sentry.testutils.cases import APITestCase
 from sentry.testutils.silo import control_silo_test
 
 
-@control_silo_test(stable=True)
 class ApiAuthorizationsTest(APITestCase):
     endpoint = "sentry-api-0-api-authorizations"
 

+ 0 - 1
tests/sentry/api/endpoints/test_doc_integration_avatar.py

@@ -11,7 +11,6 @@ from sentry.testutils.cases import APITestCase
 from sentry.testutils.silo import control_silo_test
 
 
-@control_silo_test(stable=True)
 class DocIntegrationAvatarTest(APITestCase):
     endpoint = "sentry-api-0-doc-integration-avatar"
 

+ 0 - 1
tests/sentry/api/endpoints/test_doc_integration_details.py

@@ -12,7 +12,6 @@ from sentry.testutils.cases import APITestCase
 from sentry.testutils.silo import control_silo_test
 
 
-@control_silo_test(stable=True)
 class DocIntegrationDetailsTest(APITestCase):
     endpoint = "sentry-api-0-doc-integration-details"
 

+ 0 - 1
tests/sentry/api/endpoints/test_doc_integrations.py

@@ -13,7 +13,6 @@ from sentry.testutils.silo import control_silo_test
 from sentry.utils.json import JSONData
 
 
-@control_silo_test(stable=True)
 class DocIntegrationsTest(APITestCase):
     endpoint = "sentry-api-0-doc-integrations"
 

+ 0 - 1
tests/sentry/api/endpoints/test_organization_projects.py

@@ -6,7 +6,6 @@ from sentry.testutils.cases import APITestCase
 from sentry.testutils.silo import assume_test_silo_mode, region_silo_test
 
 
-@region_silo_test(stable=True)
 class OrganizationProjectsTestBase(APITestCase):
     endpoint = "sentry-api-0-organization-projects"
 

+ 0 - 1
tests/sentry/api/endpoints/test_organization_sentry_app_installation_details.py

@@ -12,7 +12,6 @@ from sentry.testutils.cases import APITestCase
 from sentry.testutils.silo import control_silo_test
 
 
-@control_silo_test(stable=True)
 class SentryAppInstallationDetailsTest(APITestCase):
     def setUp(self):
         self.superuser = self.create_user(email="a@example.com", is_superuser=True)

+ 0 - 1
tests/sentry/api/endpoints/test_project_rule_details.py

@@ -68,7 +68,6 @@ def assert_rule_from_payload(rule: Rule, payload: Mapping[str, Any]) -> None:
     assert RuleActivity.objects.filter(rule=rule, type=RuleActivityType.UPDATED.value).exists()
 
 
-@region_silo_test(stable=True)
 class ProjectRuleDetailsBaseTestCase(APITestCase):
     endpoint = "sentry-api-0-project-rule-details"
 

+ 0 - 1
tests/sentry/api/endpoints/test_rule_snooze.py

@@ -13,7 +13,6 @@ from sentry.testutils.outbox import outbox_runner
 from sentry.testutils.silo import region_silo_test
 
 
-@region_silo_test(stable=True)
 class BaseRuleSnoozeTest(APITestCase):
     def setUp(self):
         self.issue_alert_rule = Rule.objects.create(

+ 2 - 2
tests/sentry/api/endpoints/test_user_notification_fine_tuning.py

@@ -5,7 +5,6 @@ from sentry.testutils.silo import control_silo_test
 from sentry.types.integrations import ExternalProviders
 
 
-@control_silo_test(stable=True)
 class UserNotificationFineTuningTestBase(APITestCase):
     endpoint = "sentry-api-0-user-notifications-fine-tuning"
 
@@ -55,7 +54,8 @@ class UserNotificationFineTuningGetTest(UserNotificationFineTuningTestBase):
         assert response.data.get(self.organization.id) == "0"
 
 
-@control_silo_test(stable=True)
+# TODO(hybrid-cloud): Fix underlying logic, which is not silo safe
+@control_silo_test()
 class UserNotificationFineTuningTest(UserNotificationFineTuningTestBase):
     method = "put"
 

Some files were not shown because too many files changed in this diff