From 75979916445565852615e235bf7808cdeea82038 Mon Sep 17 00:00:00 2001 From: Raj Joshi Date: Tue, 10 Dec 2024 13:02:38 -0800 Subject: [PATCH] :wrench: chore: cleaning up tests & using a util method --- .../integrations/slack/utils/notifications.py | 14 +----- tests/sentry/identity/test_oauth2.py | 12 ++---- .../discord/webhooks/test_command.py | 27 +++--------- .../integrations/msteams/test_webhook.py | 18 +++----- .../slack/webhooks/commands/test_link_team.py | 43 +++++-------------- .../slack/webhooks/commands/test_link_user.py | 18 +++----- .../slack/webhooks/events/test_message_im.py | 6 +-- .../test_commit_context_slo.py | 30 +++---------- 8 files changed, 41 insertions(+), 127 deletions(-) diff --git a/src/sentry/integrations/slack/utils/notifications.py b/src/sentry/integrations/slack/utils/notifications.py index 32a2edebc34058..3923bd21c19632 100644 --- a/src/sentry/integrations/slack/utils/notifications.py +++ b/src/sentry/integrations/slack/utils/notifications.py @@ -31,13 +31,10 @@ SLACK_LINK_IDENTITY_MSG_SUCCESS_DATADOG_METRIC, SLACK_METRIC_ALERT_FAILURE_DATADOG_METRIC, SLACK_METRIC_ALERT_SUCCESS_DATADOG_METRIC, + record_lifecycle_termination_level, ) from sentry.integrations.slack.sdk_client import SlackSdkClient from sentry.integrations.slack.spec import SlackMessagingSpec -from sentry.integrations.slack.utils.errors import ( - SLACK_SDK_HALT_ERROR_CATEGORIES, - unpack_slack_api_error, -) from sentry.models.options.organization_option import OrganizationOption from sentry.utils import metrics @@ -176,14 +173,7 @@ def send_incident_alert_notification( lifecycle.add_extras(log_params) # If the error is a channel not found or archived, we can halt the flow # This means that the channel was deleted or archived after the alert rule was created - if ( - (reason := unpack_slack_api_error(e)) - and reason is not None - and reason in SLACK_SDK_HALT_ERROR_CATEGORIES - ): - lifecycle.record_halt(reason.message) - else: - lifecycle.record_failure(e) + record_lifecycle_termination_level(lifecycle, e) else: success = True diff --git a/tests/sentry/identity/test_oauth2.py b/tests/sentry/identity/test_oauth2.py index 86beb3fa06e4a4..0b6b69243d8588 100644 --- a/tests/sentry/identity/test_oauth2.py +++ b/tests/sentry/identity/test_oauth2.py @@ -12,7 +12,7 @@ from sentry.identity.pipeline import IdentityProviderPipeline from sentry.identity.providers.dummy import DummyProvider from sentry.integrations.types import EventLifecycleOutcome -from sentry.testutils.asserts import assert_failure_metric +from sentry.testutils.asserts import assert_failure_metric, assert_slo_metric from sentry.testutils.cases import TestCase from sentry.testutils.silo import control_silo_test @@ -68,10 +68,7 @@ def test_exchange_token_success( "redirect_uri": "http://testserver/extensions/default/setup/", } - assert len(mock_record.mock_calls) == 2 - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @responses.activate def test_exchange_token_success_customer_domains(self, mock_record, mock_integration_const): @@ -96,10 +93,7 @@ def test_exchange_token_success_customer_domains(self, mock_record, mock_integra "redirect_uri": "http://testserver/extensions/default/setup/", } - assert len(mock_record.mock_calls) == 2 - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @responses.activate def test_exchange_token_ssl_error(self, mock_record, mock_integration_const): diff --git a/tests/sentry/integrations/discord/webhooks/test_command.py b/tests/sentry/integrations/discord/webhooks/test_command.py index 8388f06b0a4d54..e909531f020b84 100644 --- a/tests/sentry/integrations/discord/webhooks/test_command.py +++ b/tests/sentry/integrations/discord/webhooks/test_command.py @@ -6,7 +6,7 @@ from sentry.integrations.discord.webhooks.types import DiscordResponseTypes from sentry.integrations.messaging.metrics import MessageCommandFailureReason from sentry.integrations.types import EventLifecycleOutcome -from sentry.testutils.asserts import assert_failure_metric +from sentry.testutils.asserts import assert_failure_metric, assert_slo_metric from sentry.testutils.cases import APITestCase WEBHOOK_URL = "/extensions/discord/interactions/" @@ -118,9 +118,7 @@ def test_link_guild(self, mock_record): assert data["data"]["flags"] == EPHEMERAL_FLAG assert response.status_code == 200 - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") def test_link_dm(self, mock_record): @@ -156,9 +154,7 @@ def test_link_dm(self, mock_record): assert data["data"]["flags"] == EPHEMERAL_FLAG assert response.status_code == 200 - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") def test_link_already_linked(self, mock_record): @@ -205,9 +201,7 @@ def test_link_already_linked(self, mock_record): assert data["data"]["flags"] == EPHEMERAL_FLAG assert response.status_code == 200 - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") def test_unlink_no_identity(self, mock_record): @@ -232,9 +226,7 @@ def test_unlink_no_identity(self, mock_record): assert data["data"]["flags"] == EPHEMERAL_FLAG assert response.status_code == 200 - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") def test_unlink(self, mock_record): @@ -282,9 +274,7 @@ def test_unlink(self, mock_record): assert data["data"]["flags"] == EPHEMERAL_FLAG assert response.status_code == 200 - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") def test_help(self, mock_record): @@ -309,7 +299,4 @@ def test_help(self, mock_record): assert data["data"]["flags"] == EPHEMERAL_FLAG assert response.status_code == 200 - assert len(mock_record.mock_calls) == 2 - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) diff --git a/tests/sentry/integrations/msteams/test_webhook.py b/tests/sentry/integrations/msteams/test_webhook.py index af9034180411b1..981ae7c790cec3 100644 --- a/tests/sentry/integrations/msteams/test_webhook.py +++ b/tests/sentry/integrations/msteams/test_webhook.py @@ -12,6 +12,7 @@ from sentry.integrations.msteams.utils import ACTION_TYPE from sentry.integrations.types import EventLifecycleOutcome from sentry.silo.base import SiloMode +from sentry.testutils.asserts import assert_slo_metric from sentry.testutils.cases import APITestCase from sentry.testutils.silo import assume_test_silo_mode from sentry.users.models.identity import Identity @@ -397,9 +398,7 @@ def test_unlink_user(self, mock_time, mock_decode, mock_record): ) assert "Bearer my_token" in responses.calls[3].request.headers["Authorization"] - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @responses.activate @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @@ -435,10 +434,7 @@ def test_help_command(self, mock_time, mock_decode, mock_record): ].request.body.decode("utf-8") assert "Bearer my_token" in responses.calls[3].request.headers["Authorization"] - assert len(mock_record.mock_calls) == 2 - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @responses.activate @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @@ -500,9 +496,7 @@ def test_link_command(self, mock_time, mock_decode, mock_record): ] assert self.metrics.incr.mock_calls == calls - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @responses.activate @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @@ -544,9 +538,7 @@ def test_link_command_already_linked(self, mock_time, mock_decode, mock_record): ) assert "Bearer my_token" in responses.calls[3].request.headers["Authorization"] - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @responses.activate @mock.patch("sentry.utils.jwt.decode") diff --git a/tests/sentry/integrations/slack/webhooks/commands/test_link_team.py b/tests/sentry/integrations/slack/webhooks/commands/test_link_team.py index 0dfabb61c78776..b452311e66772c 100644 --- a/tests/sentry/integrations/slack/webhooks/commands/test_link_team.py +++ b/tests/sentry/integrations/slack/webhooks/commands/test_link_team.py @@ -13,6 +13,7 @@ ) from sentry.integrations.types import EventLifecycleOutcome from sentry.silo.base import SiloMode +from sentry.testutils.asserts import assert_slo_metric from sentry.testutils.helpers import get_response_text, link_user from sentry.testutils.helpers.features import with_feature from sentry.testutils.silo import assume_test_silo_mode @@ -64,10 +65,7 @@ def test_link_another_team_to_channel(self, mock_record): data = orjson.loads(response.content) assert CHANNEL_ALREADY_LINKED_MESSAGE in get_response_text(data) - assert len(mock_record.mock_calls) == 2 - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @with_feature("organizations:slack-multiple-team-single-channel-linking") @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @@ -91,10 +89,7 @@ def test_link_another_team_to_channel_with_flag(self, mock_record): data = orjson.loads(response.content) assert "Link your Sentry team to this Slack channel!" in get_response_text(data) - assert len(mock_record.mock_calls) == 2 - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @responses.activate @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @@ -114,9 +109,7 @@ def test_link_team_from_dm(self, mock_record): data = orjson.loads(response.content) assert LINK_FROM_CHANNEL_MESSAGE in get_response_text(data) - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @responses.activate @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @@ -130,9 +123,7 @@ def test_link_team_identity_does_not_exist(self, mock_record): data = self.send_slack_message("link team", user_id=OTHER_SLACK_ID) assert LINK_USER_FIRST_MESSAGE in get_response_text(data) - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @responses.activate @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @@ -151,9 +142,7 @@ def test_link_team_insufficient_role(self, mock_record): data = self.send_slack_message("link team", user_id=OTHER_SLACK_ID) assert INSUFFICIENT_ROLE_MESSAGE in get_response_text(data) - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @responses.activate @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @@ -167,9 +156,7 @@ def test_link_team_as_team_admin(self, mock_record): data = self.send_slack_message("link team", user_id=OTHER_SLACK_ID) assert "Link your Sentry team to this Slack channel!" in get_response_text(data) - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) class SlackCommandsUnlinkTeamTest(SlackCommandsLinkTeamTestBase): @@ -187,9 +174,7 @@ def test_unlink_team(self, mock_record): ) assert "Click here to unlink your team from this channel" in get_response_text(data) - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @responses.activate @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @@ -207,9 +192,7 @@ def test_unlink_team_as_team_admin(self, mock_record): ) assert "Click here to unlink your team from this channel" in get_response_text(data) - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @responses.activate @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @@ -225,9 +208,7 @@ def test_unlink_no_team(self, mock_record): ) assert TEAM_NOT_LINKED_MESSAGE in get_response_text(data) - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @responses.activate @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @@ -248,6 +229,4 @@ def test_unlink_multiple_orgs(self, mock_record): ) assert "Click here to unlink your team from this channel" in get_response_text(data) - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) diff --git a/tests/sentry/integrations/slack/webhooks/commands/test_link_user.py b/tests/sentry/integrations/slack/webhooks/commands/test_link_user.py index b2b989eb4ca282..c8559fb9665518 100644 --- a/tests/sentry/integrations/slack/webhooks/commands/test_link_user.py +++ b/tests/sentry/integrations/slack/webhooks/commands/test_link_user.py @@ -8,6 +8,7 @@ ) from sentry.integrations.slack.webhooks.base import NOT_LINKED_MESSAGE from sentry.integrations.types import EventLifecycleOutcome +from sentry.testutils.asserts import assert_slo_metric from sentry.testutils.helpers import get_response_text from sentry.testutils.silo import control_silo_test from sentry.users.models.identity import Identity @@ -39,9 +40,7 @@ def test_link_command(self, mock_record): data = self.send_slack_message("link") assert "Link your Slack identity" in get_response_text(data) - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") def test_link_command_already_linked(self, mock_record): @@ -49,9 +48,7 @@ def test_link_command_already_linked(self, mock_record): data = self.send_slack_message("link") assert "You are already linked as" in get_response_text(data) - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @control_silo_test @@ -121,16 +118,11 @@ def test_unlink_command(self, mock_record): data = self.send_slack_message("unlink") assert "to unlink your identity" in get_response_text(data) - assert len(mock_record.mock_calls) == 2 - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") def test_unlink_command_already_unlinked(self, mock_record): data = self.send_slack_message("unlink") assert NOT_LINKED_MESSAGE in get_response_text(data) - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) diff --git a/tests/sentry/integrations/slack/webhooks/events/test_message_im.py b/tests/sentry/integrations/slack/webhooks/events/test_message_im.py index cd1a8a143b6346..19c1ed2d6ab502 100644 --- a/tests/sentry/integrations/slack/webhooks/events/test_message_im.py +++ b/tests/sentry/integrations/slack/webhooks/events/test_message_im.py @@ -6,6 +6,7 @@ from sentry.integrations.types import EventLifecycleOutcome from sentry.silo.base import SiloMode +from sentry.testutils.asserts import assert_slo_metric from sentry.testutils.cases import IntegratedApiTestCase from sentry.testutils.helpers import get_response_text from sentry.testutils.silo import assume_test_silo_mode @@ -107,10 +108,7 @@ def test_user_message_link(self, mock_record): data = self.mock_post.call_args[1] assert "Link your Slack identity" in get_response_text(data) - assert len(mock_record.mock_calls) == 2 - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) def test_user_message_already_linked_sdk(self): """ diff --git a/tests/sentry/integrations/source_code_management/test_commit_context_slo.py b/tests/sentry/integrations/source_code_management/test_commit_context_slo.py index c1a20d02bfbb0e..0a582a0e5f5b4d 100644 --- a/tests/sentry/integrations/source_code_management/test_commit_context_slo.py +++ b/tests/sentry/integrations/source_code_management/test_commit_context_slo.py @@ -6,7 +6,7 @@ ) from sentry.integrations.types import EventLifecycleOutcome from sentry.models.repository import Repository -from sentry.testutils.asserts import assert_failure_metric, assert_halt_metric +from sentry.testutils.asserts import assert_failure_metric, assert_halt_metric, assert_slo_metric from sentry.testutils.cases import TestCase from sentry.users.models.identity import Identity @@ -43,11 +43,7 @@ def test_get_blame_for_files_success(self, mock_record): result = self.integration.get_blame_for_files([self.source_line], {}) assert result == [] - assert len(mock_record.mock_calls) == 2 - - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS) @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") def test_get_blame_for_files_missing_identity(self, mock_record): @@ -58,10 +54,7 @@ def test_get_blame_for_files_missing_identity(self, mock_record): assert result == [] assert len(mock_record.mock_calls) == 2 - - start, failure = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert failure.args[0] == EventLifecycleOutcome.FAILURE + assert_slo_metric(mock_record, EventLifecycleOutcome.FAILURE) assert_failure_metric(mock_record, Identity.DoesNotExist()) @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @@ -74,11 +67,7 @@ def test_get_blame_for_files_invalid_identity(self, mock_record): result = self.integration.get_blame_for_files([self.source_line], {}) assert result == [] - assert len(mock_record.mock_calls) == 2 - - start, failure = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert failure.args[0] == EventLifecycleOutcome.FAILURE + assert_slo_metric(mock_record, EventLifecycleOutcome.FAILURE) assert_failure_metric(mock_record, IdentityNotValid()) @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @@ -93,11 +82,7 @@ def test_get_blame_for_files_rate_limited(self, mock_record): result = self.integration.get_blame_for_files([self.source_line], {}) assert result == [] - assert len(mock_record.mock_calls) == 2 - - start, halt = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert halt.args[0] == EventLifecycleOutcome.HALTED + assert_slo_metric(mock_record, EventLifecycleOutcome.HALTED) assert_halt_metric(mock_record, ApiRateLimitedError(text="Rate limited")) @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event") @@ -109,7 +94,4 @@ def test_get_commit_context_all_frames(self, mock_record): assert result == [] assert len(mock_record.mock_calls) == 2 - - start, success = mock_record.mock_calls - assert start.args[0] == EventLifecycleOutcome.STARTED - assert success.args[0] == EventLifecycleOutcome.SUCCESS + assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS)