Skip to content

Commit e727caa

Browse files
committed
assert snuba deletion calls in test instead of actual deletions
This sacrifices the tests simulation in favor of predictibale results and less flakiness
1 parent 7d1f085 commit e727caa

File tree

1 file changed

+35
-8
lines changed

1 file changed

+35
-8
lines changed

tests/sentry/deletions/test_group.py

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,8 @@ def select_rows(
383383
def tenant_ids(self) -> dict[str, str]:
384384
return {"referrer": self.referrer, "organization_id": self.organization.id}
385385

386-
def test_simple_issue_platform(self) -> None:
386+
@mock.patch("sentry.deletions.tasks.nodestore.bulk_snuba_queries")
387+
def test_simple_issue_platform(self, mock_bulk_snuba_queries: mock.Mock) -> None:
387388
# Adding this query here to make sure that the cache is not being used
388389
assert self.select_error_events(self.project.id) is None
389390
assert self.select_issue_platform_events(self.project.id) is None
@@ -433,10 +434,36 @@ def test_simple_issue_platform(self) -> None:
433434
assert nodestore.backend.get(event_node_id)
434435
assert self.select_error_events(self.project.id) == expected_error
435436

436-
# The Issue Platform group and occurrence have been deleted
437+
# The Issue Platform group and occurrence have been deleted from Postgres
437438
assert not Group.objects.filter(id=issue_platform_group.id).exists()
438439
# assert not nodestore.backend.get(occurrence_node_id)
439-
assert self.select_issue_platform_events(self.project.id) is None
440+
441+
# Verify that deletion was sent to Snuba
442+
# We mock bulk_snuba_queries to avoid eventual consistency issues with ClickHouse light deletes
443+
assert mock_bulk_snuba_queries.called
444+
delete_calls = [
445+
call for call in mock_bulk_snuba_queries.call_args_list if call[0]
446+
] # Get calls with args
447+
assert (
448+
len(delete_calls) > 0
449+
), "Expected at least one call to bulk_snuba_queries for deletion"
450+
451+
# Verify a DeleteQuery was sent for the issue platform group
452+
from snuba_sdk import DeleteQuery
453+
454+
found_delete = False
455+
for call in delete_calls:
456+
requests = call[0][0] # First positional arg is the list of requests
457+
for req in requests:
458+
if isinstance(req.query, DeleteQuery):
459+
# Verify it's deleting the correct group
460+
if issue_platform_group.id in req.query.column_conditions.get("group_id", []):
461+
found_delete = True
462+
break
463+
if found_delete:
464+
break
465+
466+
assert found_delete, f"No DeleteQuery found for group {issue_platform_group.id}"
440467

441468
@mock.patch("sentry.deletions.tasks.nodestore.bulk_snuba_queries")
442469
def test_issue_platform_batching(self, mock_bulk_snuba_queries: mock.Mock) -> None:
@@ -451,11 +478,11 @@ def test_issue_platform_batching(self, mock_bulk_snuba_queries: mock.Mock) -> No
451478
group3 = self.create_group(project=self.project)
452479
group4 = self.create_group(project=self.project)
453480

454-
# Set times_seen for each group
455-
Group.objects.filter(id=group1.id).update(times_seen=3, type=GroupCategory.FEEDBACK)
456-
Group.objects.filter(id=group2.id).update(times_seen=1, type=GroupCategory.FEEDBACK)
457-
Group.objects.filter(id=group3.id).update(times_seen=3, type=GroupCategory.FEEDBACK)
458-
Group.objects.filter(id=group4.id).update(times_seen=3, type=GroupCategory.FEEDBACK)
481+
# Set times_seen and type for each group
482+
Group.objects.filter(id=group1.id).update(times_seen=3, type=FeedbackGroup.type_id)
483+
Group.objects.filter(id=group2.id).update(times_seen=1, type=FeedbackGroup.type_id)
484+
Group.objects.filter(id=group3.id).update(times_seen=3, type=FeedbackGroup.type_id)
485+
Group.objects.filter(id=group4.id).update(times_seen=3, type=FeedbackGroup.type_id)
459486

460487
# This will delete the group and the events from the node store and Snuba
461488
with self.tasks():

0 commit comments

Comments
 (0)