Skip to content

Commit 2597189

Browse files
joshuarlijoshuarli
committed
Merge remote-tracking branches 'origin/fix/flaky-detector-serializer-trigger-ordering', 'origin/fix/flaky-merge-groups-ordering', 'origin/fix/flaky-permissions-test-user-id-collision', 'origin/fix/flaky-relocate-recovery-call-ordering', 'origin/fix/flaky-tagkey-values-duration', 'origin/fix/flaky-update-alert-rule-project-ordering' and 'origin/fix/flaky-uptime-alert-index-ordering' into shuffle-tests-across-shards
8 parents b6f537f + 4d890b5 + 1ee00cf + 691b841 + 43a9543 + c64e364 + ad4c190 + f9da077 commit 2597189

7 files changed

Lines changed: 127 additions & 96 deletions

File tree

‎tests/sentry/api/helpers/test_group_index.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ def test_multiple_groups_same_project(self, mock_handle_merge: MagicMock) -> Non
512512

513513
assert len(call_args) == 3
514514
# Have to convert to ids because first argument is a queryset
515-
assert [group.id for group in call_args[0]] == group_ids
515+
assert sorted(group.id for group in call_args[0]) == sorted(group_ids)
516516
assert call_args[1] == {proj1.id: proj1}
517517
assert call_args[2] == self.user
518518

@@ -535,7 +535,7 @@ def test_no_project_ids_passed(self, mock_handle_merge: MagicMock) -> None:
535535

536536
assert len(call_args) == 3
537537
# Have to convert to ids because first argument is a queryset
538-
assert [group.id for group in call_args[0]] == group_ids
538+
assert sorted(group.id for group in call_args[0]) == sorted(group_ids)
539539
assert call_args[1] == {project.id: project}
540540
assert call_args[2] == self.user
541541

‎tests/sentry/api/test_permissions.py‎

Lines changed: 99 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -51,35 +51,39 @@ class IsAuthenticatedPermissionsTest(DRFPermissionTestCase):
5151

5252
def setUp(self) -> None:
5353
super().setUp()
54-
self.normal_user = self.create_user(id=1)
55-
self.readonly_user = self.create_user(id=2)
54+
self.normal_user = self.create_user()
55+
self.readonly_user = self.create_user()
5656

57-
@override_options({"demo-mode.enabled": True, "demo-mode.users": [2]})
5857
def test_has_permission(self) -> None:
59-
assert self.user_permission.has_permission(self.make_request(self.normal_user), APIView())
60-
assert not self.user_permission.has_permission(
61-
self.make_request(self.readonly_user), APIView()
62-
)
58+
with override_options(
59+
{"demo-mode.enabled": True, "demo-mode.users": [self.readonly_user.id]}
60+
):
61+
assert self.user_permission.has_permission(
62+
self.make_request(self.normal_user), APIView()
63+
)
64+
assert not self.user_permission.has_permission(
65+
self.make_request(self.readonly_user), APIView()
66+
)
6367

64-
@override_options({"demo-mode.enabled": True, "demo-mode.users": [2]})
6568
def test_has_object_permission(self) -> None:
66-
assert self.user_permission.has_object_permission(
67-
self.make_request(self.normal_user), APIView(), None
68-
)
69-
assert not self.user_permission.has_object_permission(
70-
self.make_request(self.readonly_user), APIView(), None
71-
)
69+
with override_options(
70+
{"demo-mode.enabled": True, "demo-mode.users": [self.readonly_user.id]}
71+
):
72+
assert self.user_permission.has_object_permission(
73+
self.make_request(self.normal_user), APIView(), None
74+
)
75+
assert not self.user_permission.has_object_permission(
76+
self.make_request(self.readonly_user), APIView(), None
77+
)
7278

7379

7480
class DemoSafePermissionsTest(DRFPermissionTestCase):
7581
user_permission = DemoSafePermission()
7682

7783
def setUp(self) -> None:
7884
super().setUp()
79-
self.normal_user = self.create_user(
80-
id=1,
81-
)
82-
self.readonly_user = self.create_user(id=2)
85+
self.normal_user = self.create_user()
86+
self.readonly_user = self.create_user()
8387
self.organization = self.create_organization(owner=self.normal_user)
8488
self.org_member_scopes = self.create_member(
8589
organization_id=self.organization.id, user_id=self.readonly_user.id
@@ -93,90 +97,102 @@ def _get_rpc_context(self, user):
9397
assert rpc_context
9498
return rpc_context
9599

96-
@override_options({"demo-mode.enabled": True, "demo-mode.users": [2]})
97100
def test_safe_methods(self) -> None:
98-
for method in ("GET", "HEAD", "OPTIONS"):
99-
assert self.user_permission.has_permission(
100-
self.make_request(self.readonly_user, method=method), APIView()
101-
)
102-
assert self.user_permission.has_permission(
103-
self.make_request(self.normal_user, method=method), APIView()
104-
)
101+
with override_options(
102+
{"demo-mode.enabled": True, "demo-mode.users": [self.readonly_user.id]}
103+
):
104+
for method in ("GET", "HEAD", "OPTIONS"):
105+
assert self.user_permission.has_permission(
106+
self.make_request(self.readonly_user, method=method), APIView()
107+
)
108+
assert self.user_permission.has_permission(
109+
self.make_request(self.normal_user, method=method), APIView()
110+
)
105111

106-
@override_options({"demo-mode.enabled": True, "demo-mode.users": [2]})
107112
def test_unsafe_methods(self) -> None:
108-
for method in ("POST", "PUT", "PATCH", "DELETE"):
109-
assert not self.user_permission.has_permission(
110-
self.make_request(self.readonly_user, method=method), APIView()
111-
)
112-
assert self.user_permission.has_permission(
113-
self.make_request(self.normal_user, method=method), APIView()
114-
)
113+
with override_options(
114+
{"demo-mode.enabled": True, "demo-mode.users": [self.readonly_user.id]}
115+
):
116+
for method in ("POST", "PUT", "PATCH", "DELETE"):
117+
assert not self.user_permission.has_permission(
118+
self.make_request(self.readonly_user, method=method), APIView()
119+
)
120+
assert self.user_permission.has_permission(
121+
self.make_request(self.normal_user, method=method), APIView()
122+
)
115123

116-
@override_options({"demo-mode.enabled": False, "demo-mode.users": [2]})
117124
def test_safe_method_demo_mode_disabled(self) -> None:
118-
for method in ("GET", "HEAD", "OPTIONS"):
119-
assert not self.user_permission.has_permission(
120-
self.make_request(self.readonly_user, method=method), APIView()
121-
)
122-
assert self.user_permission.has_permission(
123-
self.make_request(self.normal_user, method=method), APIView()
124-
)
125+
with override_options(
126+
{"demo-mode.enabled": False, "demo-mode.users": [self.readonly_user.id]}
127+
):
128+
for method in ("GET", "HEAD", "OPTIONS"):
129+
assert not self.user_permission.has_permission(
130+
self.make_request(self.readonly_user, method=method), APIView()
131+
)
132+
assert self.user_permission.has_permission(
133+
self.make_request(self.normal_user, method=method), APIView()
134+
)
125135

126-
@override_options({"demo-mode.enabled": False, "demo-mode.users": [2]})
127136
def test_unsafe_methods_demo_mode_disabled(self) -> None:
128-
for method in ("POST", "PUT", "PATCH", "DELETE"):
129-
assert not self.user_permission.has_permission(
130-
self.make_request(self.readonly_user, method=method), APIView()
131-
)
132-
assert self.user_permission.has_permission(
133-
self.make_request(self.normal_user, method=method), APIView()
134-
)
137+
with override_options(
138+
{"demo-mode.enabled": False, "demo-mode.users": [self.readonly_user.id]}
139+
):
140+
for method in ("POST", "PUT", "PATCH", "DELETE"):
141+
assert not self.user_permission.has_permission(
142+
self.make_request(self.readonly_user, method=method), APIView()
143+
)
144+
assert self.user_permission.has_permission(
145+
self.make_request(self.normal_user, method=method), APIView()
146+
)
135147

136-
@override_options({"demo-mode.enabled": False, "demo-mode.users": [2]})
137148
def test_determine_access_disabled(self) -> None:
138-
self.user_permission.determine_access(
139-
request=self.make_request(self.normal_user),
140-
organization=self._get_rpc_context(self.normal_user),
141-
)
149+
with override_options(
150+
{"demo-mode.enabled": False, "demo-mode.users": [self.readonly_user.id]}
151+
):
152+
self.user_permission.determine_access(
153+
request=self.make_request(self.normal_user),
154+
organization=self._get_rpc_context(self.normal_user),
155+
)
142156

143-
readonly_rpc_context = self._get_rpc_context(self.readonly_user)
157+
readonly_rpc_context = self._get_rpc_context(self.readonly_user)
144158

145-
self.user_permission.determine_access(
146-
request=self.make_request(self.readonly_user),
147-
organization=readonly_rpc_context,
148-
)
159+
self.user_permission.determine_access(
160+
request=self.make_request(self.readonly_user),
161+
organization=readonly_rpc_context,
162+
)
149163

150-
assert readonly_rpc_context.member.scopes == list(self.org_member_scopes)
164+
assert readonly_rpc_context.member.scopes == list(self.org_member_scopes)
151165

152-
@override_options({"demo-mode.enabled": True, "demo-mode.users": [2]})
153166
def test_determine_access(self) -> None:
154-
self.user_permission.determine_access(
155-
request=self.make_request(self.normal_user),
156-
organization=self._get_rpc_context(self.normal_user),
157-
)
167+
with override_options(
168+
{"demo-mode.enabled": True, "demo-mode.users": [self.readonly_user.id]}
169+
):
170+
self.user_permission.determine_access(
171+
request=self.make_request(self.normal_user),
172+
organization=self._get_rpc_context(self.normal_user),
173+
)
158174

159-
readonly_rpc_context = self._get_rpc_context(self.readonly_user)
175+
readonly_rpc_context = self._get_rpc_context(self.readonly_user)
160176

161-
self.user_permission.determine_access(
162-
request=self.make_request(self.readonly_user),
163-
organization=readonly_rpc_context,
164-
)
177+
self.user_permission.determine_access(
178+
request=self.make_request(self.readonly_user),
179+
organization=readonly_rpc_context,
180+
)
165181

166-
assert readonly_rpc_context.member.scopes == sorted(READONLY_SCOPES)
182+
assert readonly_rpc_context.member.scopes == sorted(READONLY_SCOPES)
167183

168-
@override_options({"demo-mode.enabled": False, "demo-mode.users": []})
169184
def test_determine_access_no_demo_users(self) -> None:
170-
self.user_permission.determine_access(
171-
request=self.make_request(self.normal_user),
172-
organization=self._get_rpc_context(self.normal_user),
173-
)
185+
with override_options({"demo-mode.enabled": False, "demo-mode.users": []}):
186+
self.user_permission.determine_access(
187+
request=self.make_request(self.normal_user),
188+
organization=self._get_rpc_context(self.normal_user),
189+
)
174190

175-
readonly_rpc_context = self._get_rpc_context(self.readonly_user)
191+
readonly_rpc_context = self._get_rpc_context(self.readonly_user)
176192

177-
self.user_permission.determine_access(
178-
request=self.make_request(self.readonly_user),
179-
organization=readonly_rpc_context,
180-
)
193+
self.user_permission.determine_access(
194+
request=self.make_request(self.readonly_user),
195+
organization=readonly_rpc_context,
196+
)
181197

182-
assert readonly_rpc_context.member.scopes == list(self.org_member_scopes)
198+
assert readonly_rpc_context.member.scopes == list(self.org_member_scopes)

‎tests/sentry/incidents/serializers/test_workflow_engine_detector.py‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,19 @@ class TestDetectorSerializer(TestWorkflowEngineSerializer):
2828
def setUp(self) -> None:
2929
super().setUp()
3030

31+
@staticmethod
32+
def _sort_triggers(data: dict[str, Any]) -> dict[str, Any]:
33+
"""Sort triggers by id so ordering doesn't affect comparison."""
34+
result = data.copy()
35+
result["triggers"] = sorted(result.get("triggers", []), key=lambda t: t["id"])
36+
return result
37+
3138
def test_simple(self) -> None:
3239
self.add_warning_trigger()
3340
serialized_detector = serialize(
3441
self.detector, self.user, WorkflowEngineDetectorSerializer()
3542
)
36-
assert serialized_detector == self.expected
43+
assert self._sort_triggers(serialized_detector) == self._sort_triggers(self.expected)
3744

3845
def test_latest_incident(self) -> None:
3946
self.add_warning_trigger()
@@ -181,7 +188,7 @@ def test_sentry_app(self, mock_sentry_app_components_preparer: Any) -> None:
181188
self.user,
182189
WorkflowEngineDetectorSerializer(prepare_component_fields=True),
183190
)
184-
assert serialized_detector == sentry_app_expected
191+
assert self._sort_triggers(serialized_detector) == self._sort_triggers(sentry_app_expected)
185192

186193
def test_new_models_only(self) -> None:
187194
# test that we can still serialize if objects do not have lookup table entries

‎tests/sentry/incidents/test_logic.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -870,8 +870,7 @@ def test(self) -> None:
870870
assert set(self.alert_rule.snuba_query.event_types) == set(event_types)
871871
assert self.alert_rule.threshold_type == threshold_type.value
872872
assert self.alert_rule.threshold_period == threshold_period
873-
assert self.alert_rule.projects.all().count() == 2
874-
assert self.alert_rule.projects.all()[0] == updated_projects[0]
873+
assert set(self.alert_rule.projects.all()) == set(updated_projects)
875874

876875
def test_update_subscription(self) -> None:
877876
old_subscription_id = self.alert_rule.snuba_query.subscriptions.get().subscription_id

‎tests/sentry/uptime/endpoints/test_organization_uptime_alert_index.py‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,15 @@ class OrganizationUptimeAlertIndexEndpointTest(OrganizationUptimeAlertIndexBaseE
1212
method = "get"
1313

1414
def check_valid_response(self, response, expected_detectors):
15-
assert [
16-
serialize(uptime_alert, serializer=UptimeDetectorSerializer())
17-
for uptime_alert in expected_detectors
18-
] == response.data
15+
expected = sorted(
16+
[
17+
serialize(uptime_alert, serializer=UptimeDetectorSerializer())
18+
for uptime_alert in expected_detectors
19+
],
20+
key=lambda x: x["id"],
21+
)
22+
actual = sorted(response.data, key=lambda x: x["id"])
23+
assert expected == actual
1924

2025
def test(self) -> None:
2126
alert_1 = self.create_uptime_detector(name="test1")

‎tests/sentry/users/web/test_accounts.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,8 @@ def test_relocate_recovery_post_multiple_orgs(
212212
ip_address="127.0.0.1",
213213
sender=recover_confirm,
214214
),
215-
]
215+
],
216+
any_order=True,
216217
)
217218

218219
assert UserEmail.objects.get(email=user.email).is_verified

‎tests/snuba/api/endpoints/test_organization_tagkey_values.py‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -502,11 +502,14 @@ def setUp(self) -> None:
502502
)
503503
self.store_event(data, project_id=self.project.id)
504504
self.transaction = data.copy()
505+
# Compute both timestamps from the same base to get an exact 5000ms duration
506+
end_time = before_now(seconds=30)
507+
start_time = end_time - timedelta(seconds=5)
505508
self.transaction.update(
506509
{
507510
"transaction": "/city_by_code/",
508-
"timestamp": before_now(seconds=30).isoformat(),
509-
"start_timestamp": before_now(seconds=35).isoformat(),
511+
"timestamp": end_time.isoformat(),
512+
"start_timestamp": start_time.isoformat(),
510513
}
511514
)
512515
self.transaction["contexts"]["trace"].update(

0 commit comments

Comments
 (0)