test(hc): Remove stable parameter from SiloModeTestDecorator (#60498)

src/sentry/testutils/silo.py

@@ -85,22 +85,25 @@ def strip_silo_mode_test_suffix(name: str) -> str:
 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 region or control mode only do so when the test is marked as stable=True
+    A test marked with a single silo mode runs only in that mode by default. An
+    `include_monolith_run=True` will add a secondary run in monolith mode.
 
-    When testing in a silo mode, if the decorator is on a test case class,
+    If a test is marked with both control and region modes, then the primary run will
+    be in monolith mode and a secondary run will be generated in each silo mode.
+
+    When testing on more than one 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
+        @control_silo_test(include_monolith_run=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`.
+    then your result set should include test runs for both `MyTest` (in control mode)
+    and `MyTest__InMonolithMode`.
     """
 
     def __init__(self, *silo_modes: SiloMode) -> None:
@@ -109,14 +112,12 @@ class SiloModeTestDecorator:
     def __call__(
         self,
         decorated_obj: Any = None,
-        stable: bool = True,
         regions: Sequence[Region] = (),
         include_monolith_run: bool = False,
     ) -> Any:
         mod = _SiloModeTestModification(
             silo_modes=self.silo_modes,
             regions=tuple(regions or _DEFAULT_TEST_REGIONS),
-            stable=stable,
             include_monolith_run=include_monolith_run,
         )
 
@@ -129,7 +130,6 @@ class _SiloModeTestModification:
 
     silo_modes: frozenset[SiloMode]
     regions: tuple[Region, ...]
-    stable: bool
     include_monolith_run: bool
 
     run_original_class_in_silo_mode: bool = True
@@ -234,10 +234,9 @@ class _SiloModeTestModification:
             # _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 SENTRY_USE_MONOLITH_DBS or not (self.stable or settings.FORCE_SILOED_TESTS):
-            # In this case, simply force the current silo mode (monolith)
+        if SENTRY_USE_MONOLITH_DBS:
+            # In this case, skip modifying the object and let it run in the default
+            # silo mode (monolith)
             return decorated_obj
 
         if is_test_case_class:
@@ -276,18 +275,12 @@ control_silo_test = SiloModeTestDecorator(SiloMode.CONTROL)
 """
 Apply to test functions/classes to indicate that tests are
 expected to pass with the current silo mode set to CONTROL.
-
-When the stable=True parameter is provided tests will be
-run twice as both CONTROL and MONOLITH modes.
 """
 
 region_silo_test = SiloModeTestDecorator(SiloMode.REGION)
 """
 Apply to test functions/classes to indicate that tests are
 expected to pass with the current silo mode set to REGION.
-
-When the stable=True parameter is provided tests will be
-run twice as both REGION and MONOLITH modes.
 """
 
 

tests/acceptance/test_accept_organization_invite.py

@@ -6,7 +6,7 @@ from sentry.testutils.cases import AcceptanceTestCase
 from sentry.testutils.silo import no_silo_test
 
 
-# When we want to set this stable=True, we'll need to configure regions in order for invites to work.
+# When we want to set this @region_silo_test, we'll need to configure regions in order for invites to work.
 # See the accept_organization_invite.py#get_invite_state logic
 @no_silo_test
 class AcceptOrganizationInviteTest(AcceptanceTestCase):

tests/sentry/api/endpoints/test_organization_stats.py

@@ -10,7 +10,7 @@ from sentry.testutils.silo import region_silo_test
 from sentry.utils.outcomes import Outcome
 
 
-@region_silo_test  # TODO(hybrid-cloud): stable=True blocked on org members
+@region_silo_test
 @freeze_time(before_now(days=1).replace(hour=1, minute=10))
 class OrganizationStatsTest(APITestCase, OutcomesSnubaTest):
     def test_simple(self):

tests/sentry/api/endpoints/test_organizations_ddm_meta.py

@@ -17,8 +17,8 @@ from sentry.utils import json
 pytestmark = pytest.mark.sentry_metrics
 
 
+@region_silo_test
 @freeze_time("2023-11-21T10:30:30.000Z")
-@region_silo_test(stable=True)
 class OrganizationDDMMetaEndpointTest(MetricsAPIBaseTestCase):
     endpoint = "sentry-api-0-organization-ddm-meta"
 

tests/sentry/api/endpoints/test_project_rule_actions.py

@@ -12,7 +12,7 @@ from sentry.testutils.skips import requires_snuba
 pytestmark = [requires_snuba]
 
 
-@region_silo_test(stable=True)
+@region_silo_test
 class ProjectRuleActionsEndpointTest(APITestCase):
     endpoint = "sentry-api-0-project-rule-actions"
     method = "POST"

tests/sentry/api/endpoints/test_source_map_debug.py

@@ -13,7 +13,7 @@ from sentry.testutils.skips import requires_kafka, requires_snuba
 pytestmark = [requires_snuba, requires_kafka]
 
 
-@region_silo_test  # TODO(hybrid-cloud): stable=True blocked on actors
+@region_silo_test
 class SourceMapDebugEndpointTestCase(APITestCase):
     endpoint = "sentry-api-0-event-source-map-debug"
 

tests/sentry/api/endpoints/test_source_map_debug_blue_thunder_edition.py

@@ -73,7 +73,7 @@ def create_event(
     return event
 
 
-@region_silo_test  # TODO(hybrid-cloud): stable=True blocked on actors
+@region_silo_test
 class SourceMapDebugBlueThunderEditionEndpointTestCase(APITestCase):
     endpoint = "sentry-api-0-event-source-map-debug-blue-thunder-edition"
 

tests/sentry/api/endpoints/test_user_avatar.py

@@ -2,7 +2,6 @@ from base64 import b64encode
 from io import BytesIO
 from unittest import mock
 
-import pytest
 from django.test import override_settings
 from django.urls import reverse
 
@@ -11,7 +10,6 @@ from sentry.models.avatars.user_avatar import UserAvatar, UserAvatarType
 from sentry.models.files import ControlFile, File
 from sentry.silo.base import SiloMode
 from sentry.testutils.cases import APITestCase
-from sentry.testutils.hybrid_cloud import use_split_dbs
 from sentry.testutils.silo import assume_test_silo_mode, control_silo_test
 
 
@@ -164,53 +162,6 @@ class UserAvatarTest(APITestCase):
         assert isinstance(avatar.get_file(), ControlFile)
         assert ControlFile.objects.filter(id=avatar.control_file_id).exists()
 
-    @pytest.mark.skipif(use_split_dbs(), reason="requires single/monolith db mode")
-    @mock.patch("sentry.tasks.files.copy_file_to_control_and_update_model.apply_async")
-    def test_get_upload_use_control_during_update(self, mock_task):
-        user = self.create_user(email="a@example.com")
-
-        with assume_test_silo_mode(SiloMode.REGION):
-            photo = File.objects.create(name="test.png", type="avatar.file")
-            photo.putfile(BytesIO(b"test"))
-        avatar = UserAvatar.objects.create(
-            user=user, file_id=photo.id, avatar_type=UserAvatarType.UPLOAD
-        )
-        assert avatar
-        assert avatar.get_file_id()
-        assert avatar.file_id
-        assert not avatar.control_file_id
-        with assume_test_silo_mode(SiloMode.REGION):
-            assert isinstance(avatar.get_file(), File)
-
-        self.login_as(user=user)
-
-        url = reverse("sentry-api-0-user-avatar", kwargs={"user_id": "me"})
-
-        mock_task.reset_mock()
-        # Run in monolith as this transitional operation can only occur there.
-        with assume_test_silo_mode(SiloMode.MONOLITH):
-            response = self.client.put(
-                url,
-                data={
-                    "avatar_type": "upload",
-                    "avatar_photo": b64encode(self.load_fixture("avatar.jpg")),
-                },
-                format="json",
-            )
-
-        assert response.status_code == 200, response.content
-        assert response.data["id"] == str(user.id)
-        assert response.data["avatar"]["avatarType"] == "upload"
-        assert response.data["avatar"]["avatarUuid"]
-        assert mock_task.call_count == 1
-
-        avatar = UserAvatar.objects.get(user=user)
-        assert avatar
-        assert avatar.get_file_id()
-        assert avatar.file_id is None, "non-control file relation be removed."
-        assert avatar.control_file_id
-        assert isinstance(avatar.get_file(), ControlFile)
-
     @mock.patch("sentry.tasks.files.copy_file_to_control_and_update_model.apply_async")
     def test_do_not_copy_file_to_control(self, mock_task):
         user = self.create_user(email="a@example.com")

tests/sentry/tasks/integrations/github/test_open_pr_comment.py

@@ -459,7 +459,7 @@ You modified these files in this pull request and we noticed these issues associ
         )
 
 
-@region_silo_test(stable=True)
+@region_silo_test
 class TestOpenPRCommentWorkflow(GithubCommentTestCase):
     def setUp(self):
         super().setUp()

tests/sentry/web/frontend/test_newest_performance_issue.py

@@ -29,7 +29,7 @@ nplus_one_no_timestamp = {**get_event("n-plus-one-in-django-index-view")}
 del nplus_one_no_timestamp["timestamp"]
 
 
-@region_silo_test(stable=True)
+@region_silo_test
 class NewestIssueViewTest(TestCase, PerformanceIssueTestCase):
     @cached_property
     def path(self):