Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/sentry/tempest/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from sentry.taskworker.config import TaskworkerConfig
from sentry.taskworker.namespaces import tempest_tasks
from sentry.tempest.models import MessageType, TempestCredentials
from sentry.tempest.utils import has_tempest_access

logger = logging.getLogger(__name__)

Expand All @@ -27,7 +28,12 @@
)
def poll_tempest(**kwargs):
# FIXME: Once we have more traffic this needs to be done smarter.
for credentials in TempestCredentials.objects.all():
for credentials in TempestCredentials.objects.select_related("project__organization").all():
# Bruno wants to keeps the customers credentials around so need to
# skip if the credentials are associated with an org that no longer has tempest access.
if not has_tempest_access(credentials.project.organization):
continue

if credentials.latest_fetched_item_id is None:
fetch_latest_item_id.apply_async(
kwargs={"credentials_id": credentials.id},
Expand Down
41 changes: 39 additions & 2 deletions tests/sentry/tempest/test_tempest.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,13 @@ def test_poll_tempest_crashes_error(self, mock_fetch: MagicMock) -> None:
mock_fetch.assert_called_once()
assert "Fetching the crashes failed." in cm.output[0]

@patch("sentry.tempest.tasks.has_tempest_access")
@patch("sentry.tempest.tasks.fetch_latest_item_id")
@patch("sentry.tempest.tasks.poll_tempest_crashes")
def test_poll_tempest_no_latest_id(
self, mock_poll_crashes: MagicMock, mock_fetch_latest: MagicMock
self, mock_poll_crashes: MagicMock, mock_fetch_latest: MagicMock, mock_has_access: MagicMock
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Mock Parameter Order Mismatch

The parameter order in test methods using multiple @patch decorators is reversed. Python applies patch decorators from bottom to top, so the outermost decorator's mock object (e.g., mock_has_access) should be the first parameter. This mismatch causes incorrect mock assignments and test configurations.

Additional Locations (2)

Fix in Cursor Fix in Web

) -> None:
mock_has_access.return_value = True
# Ensure latest_fetched_item_id is None
self.credentials.latest_fetched_item_id = None
self.credentials.save()
Expand All @@ -164,11 +166,13 @@ def test_poll_tempest_no_latest_id(
)
mock_poll_crashes.apply_async.assert_not_called()

@patch("sentry.tempest.tasks.has_tempest_access")
@patch("sentry.tempest.tasks.fetch_latest_item_id")
@patch("sentry.tempest.tasks.poll_tempest_crashes")
def test_poll_tempest_with_latest_id(
self, mock_poll_crashes: MagicMock, mock_fetch_latest: MagicMock
self, mock_poll_crashes: MagicMock, mock_fetch_latest: MagicMock, mock_has_access: MagicMock
) -> None:
mock_has_access.return_value = True
# Set an existing ID
self.credentials.latest_fetched_item_id = "42"
self.credentials.save()
Expand Down Expand Up @@ -238,3 +242,36 @@ def test_poll_tempest_crashes_invalidates_config(
# Second call -> should reuse existing ProjectKey and thus not invalidate config
poll_tempest_crashes(self.credentials.id)
mock_invalidate.assert_not_called()

@patch("sentry.tempest.tasks.has_tempest_access")
@patch("sentry.tempest.tasks.fetch_latest_item_id")
@patch("sentry.tempest.tasks.poll_tempest_crashes")
def test_poll_tempest_skips_credentials_without_access(
self, mock_poll_crashes: MagicMock, mock_fetch_latest: MagicMock, mock_has_access: MagicMock
) -> None:
"""Test that poll_tempest skips credentials when organization doesn't have tempest access"""
org_with_access = self.create_organization()
project_with_access = self.create_project(organization=org_with_access)
credentials_with_access = self.create_tempest_credentials(project_with_access)
credentials_with_access.latest_fetched_item_id = "42"
credentials_with_access.save()

org_without_access = self.create_organization()
project_without_access = self.create_project(organization=org_without_access)
credentials_without_access = self.create_tempest_credentials(project_without_access)
credentials_without_access.latest_fetched_item_id = "42"
credentials_without_access.save()

def mock_access_check(organization):
return organization.id == org_with_access.id

mock_has_access.side_effect = mock_access_check

poll_tempest()

assert mock_has_access.call_count == 3
mock_poll_crashes.apply_async.assert_called_once_with(
kwargs={"credentials_id": credentials_with_access.id},
headers={"sentry-propagate-traces": False},
)
mock_fetch_latest.apply_async.assert_not_called()
Loading