Skip to content

Commit

Permalink
update tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@lobsterkatie
lobsterkatie committed Jul 23, 2024
1 parent 36b6321 commit f7ac804
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 33 deletions.
18 changes: 0 additions & 18 deletions tests/sentry/event_manager/grouping/test_seer_grouping.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
from sentry.testutils.helpers.eventprocessing import save_new_event
from sentry.testutils.helpers.features import with_feature
from sentry.testutils.pytest.mocking import capture_results
from sentry.utils.circuit_breaker import with_circuit_breaker
from sentry.utils.types import NonNone


Expand Down Expand Up @@ -152,23 +151,6 @@ def test_obeys_seer_similarity_flags(self):
assert get_seer_similar_issues_return_values[0][1] == existing_event.group
assert new_event.group_id == existing_event.group_id

@patch("sentry.event_manager.should_call_seer_for_grouping", return_value=True)
@patch("sentry.event_manager.with_circuit_breaker", wraps=with_circuit_breaker)
@patch("sentry.event_manager.get_seer_similar_issues", return_value=({}, None))
def test_obeys_circult_breaker(
self, mock_get_seer_similar_issues: MagicMock, mock_with_circuit_breaker: MagicMock, _
):
with patch("sentry.utils.circuit_breaker._should_call_callback", return_value=True):
save_new_event({"message": "Dogs are great!"}, self.project)
assert mock_with_circuit_breaker.call_count == 1
assert mock_get_seer_similar_issues.call_count == 1

with patch("sentry.utils.circuit_breaker._should_call_callback", return_value=False):
save_new_event({"message": "Adopt don't shop"}, self.project)

assert mock_with_circuit_breaker.call_count == 2 # increased
assert mock_get_seer_similar_issues.call_count == 1 # didn't increase

@patch("sentry.event_manager.should_call_seer_for_grouping", return_value=True)
@patch("sentry.event_manager.get_seer_similar_issues", return_value=({}, None))
def test_calls_seer_if_no_group_found(self, mock_get_seer_similar_issues: MagicMock, _):
Expand Down
12 changes: 12 additions & 0 deletions tests/sentry/grouping/ingest/test_seer.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,18 @@ def test_obeys_project_ratelimit(self):
is expected_result
)

@with_feature("projects:similarity-embeddings-grouping")
def test_obeys_circuit_breaker(self):
for request_allowed, expected_result in [(True, True), (False, False)]:
with patch(
"sentry.grouping.ingest.seer.seer_similarity_circuit_breaker.should_allow_request",
return_value=request_allowed,
):
assert (
should_call_seer_for_grouping(self.event, self.primary_hashes)
is expected_result
)

@with_feature("projects:similarity-embeddings-grouping")
def test_obeys_customized_fingerprint_check(self):
default_fingerprint_event = Event(
Expand Down
73 changes: 58 additions & 15 deletions tests/sentry/seer/similarity/test_similar_issues.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,17 @@ def test_no_groups_found(self, mock_seer_request: MagicMock, mock_metrics_incr:
tags={"response_status": 200, "outcome": "no_similar_groups"},
)

@mock.patch(
"sentry.seer.similarity.similar_issues.seer_similarity_circuit_breaker.record_error"
)
@mock.patch("sentry.seer.similarity.similar_issues.metrics.incr")
@mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
def test_bad_response_data(self, mock_seer_request: MagicMock, mock_metrics_incr: MagicMock):
def test_bad_response_data(
self,
mock_seer_request: MagicMock,
mock_metrics_incr: MagicMock,
mock_record_circuit_breaker_error: MagicMock,
):
cases: list[tuple[Any, str]] = [
(None, "AttributeError"),
([], "AttributeError"),
Expand Down Expand Up @@ -139,14 +147,22 @@ def test_bad_response_data(self, mock_seer_request: MagicMock, mock_metrics_incr
sample_rate=SIMILARITY_REQUEST_METRIC_SAMPLE_RATE,
tags={"response_status": 200, "outcome": "error", "error": expected_error},
)
assert mock_record_circuit_breaker_error.call_count == 0

mock_metrics_incr.reset_mock()

@mock.patch(
"sentry.seer.similarity.similar_issues.seer_similarity_circuit_breaker.record_error"
)
@mock.patch("sentry.seer.similarity.similar_issues.metrics.incr")
@mock.patch("sentry.seer.similarity.similar_issues.logger")
@mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
def test_redirect(
self, mock_seer_request: MagicMock, mock_logger: MagicMock, mock_metrics_incr: MagicMock
self,
mock_seer_request: MagicMock,
mock_logger: MagicMock,
mock_metrics_incr: MagicMock,
mock_record_circuit_breaker_error: MagicMock,
):
mock_seer_request.return_value = HTTPResponse(
status=308, headers={"location": "/new/and/improved/endpoint/"}
Expand All @@ -161,12 +177,20 @@ def test_redirect(
sample_rate=SIMILARITY_REQUEST_METRIC_SAMPLE_RATE,
tags={"response_status": 308, "outcome": "error", "error": "Redirect"},
)
assert mock_record_circuit_breaker_error.call_count == 0

@mock.patch(
"sentry.seer.similarity.similar_issues.seer_similarity_circuit_breaker.record_error"
)
@mock.patch("sentry.seer.similarity.similar_issues.metrics.incr")
@mock.patch("sentry.seer.similarity.similar_issues.logger")
@mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
def test_request_error(
self, mock_seer_request: MagicMock, mock_logger: MagicMock, mock_metrics_incr: MagicMock
self,
mock_seer_request: MagicMock,
mock_logger: MagicMock,
mock_metrics_incr: MagicMock,
mock_record_circuit_breaker_error: MagicMock,
):
for request_error, expected_error_tag in [
(TimeoutError, "TimeoutError"),
Expand All @@ -192,25 +216,44 @@ def test_request_error(
sample_rate=SIMILARITY_REQUEST_METRIC_SAMPLE_RATE,
tags={"outcome": "error", "error": expected_error_tag},
)
assert mock_record_circuit_breaker_error.call_count == 1

mock_logger.warning.reset_mock()
mock_metrics_incr.reset_mock()
mock_record_circuit_breaker_error.reset_mock()

@mock.patch(
"sentry.seer.similarity.similar_issues.seer_similarity_circuit_breaker.record_error"
)
@mock.patch("sentry.seer.similarity.similar_issues.metrics.incr")
@mock.patch("sentry.seer.similarity.similar_issues.logger")
@mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
def test_error_status(
self, mock_seer_request: MagicMock, mock_logger: MagicMock, mock_metrics_incr: MagicMock
self,
mock_seer_request: MagicMock,
mock_logger: MagicMock,
mock_metrics_incr: MagicMock,
mock_record_circuit_breaker_error: MagicMock,
):
mock_seer_request.return_value = HTTPResponse("No soup for you", status=403)
for response, status, counts_for_circuit_breaker in [
("No soup for you", 403, False),
("No soup, period", 500, True),
]:
mock_seer_request.return_value = HTTPResponse(response, status=status)

assert get_similarity_data_from_seer(self.request_params) == []
mock_logger.error.assert_called_with(
f"Received 403 when calling Seer endpoint {SEER_SIMILAR_ISSUES_URL}.",
extra={"response_data": "No soup for you"},
)
mock_metrics_incr.assert_any_call(
"seer.similar_issues_request",
sample_rate=SIMILARITY_REQUEST_METRIC_SAMPLE_RATE,
tags={"response_status": 403, "outcome": "error", "error": "RequestError"},
)
assert get_similarity_data_from_seer(self.request_params) == []
mock_logger.error.assert_called_with(
f"Received {status} when calling Seer endpoint {SEER_SIMILAR_ISSUES_URL}.",
extra={"response_data": response},
)
mock_metrics_incr.assert_any_call(
"seer.similar_issues_request",
sample_rate=SIMILARITY_REQUEST_METRIC_SAMPLE_RATE,
tags={"response_status": status, "outcome": "error", "error": "RequestError"},
)
assert mock_record_circuit_breaker_error.call_count == (
1 if counts_for_circuit_breaker else 0
)

@mock.patch("sentry.seer.similarity.similar_issues.seer_grouping_connection_pool.urlopen")
def test_returns_sorted_results(self, mock_seer_request: MagicMock):
Expand Down

0 comments on commit f7ac804

Please sign in to comment.