Skip to content

Commit 35b7459

Browse files
committed
fixed broken test, removed tests on log assertions
Signed-off-by: Nikhil Suri <nikhil.suri@databricks.com>
1 parent dbd915f commit 35b7459

File tree

1 file changed

+22
-46
lines changed

1 file changed

+22
-46
lines changed

tests/unit/test_telemetry_request_error_handling.py

Lines changed: 22 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"""
55

66
import pytest
7-
from unittest.mock import Mock, patch
7+
from unittest.mock import Mock
88

99
from databricks.sql.telemetry.telemetry_push_client import (
1010
CircuitBreakerTelemetryPushClient,
@@ -106,40 +106,25 @@ def test_request_error_missing_context_attribute(self, client, mock_delegate):
106106
with pytest.raises(RequestError, match="HTTP request failed"):
107107
client.request(HttpMethod.POST, "https://test.com", {})
108108

109-
def test_request_error_with_http_code_429_logs_warning(self, client, mock_delegate):
110-
"""Test that rate limit errors log at warning level and raise exception."""
111-
with patch(
112-
"databricks.sql.telemetry.telemetry_push_client.logger"
113-
) as mock_logger:
114-
request_error = RequestError(
115-
"HTTP request failed", context={"http-code": 429}
116-
)
117-
mock_delegate.request.side_effect = request_error
118-
119-
with pytest.raises(TelemetryRateLimitError):
120-
client.request(HttpMethod.POST, "https://test.com", {})
121-
122-
# Should log warning for rate limiting
123-
mock_logger.warning.assert_called()
124-
warning_args = mock_logger.warning.call_args[0]
125-
assert "429" in str(warning_args)
126-
assert "circuit breaker" in warning_args[0].lower()
127-
128-
def test_request_error_with_http_code_500_logs_debug(self, client, mock_delegate):
129-
"""Test that non-rate-limit errors log at debug level and raise original error."""
130-
with patch(
131-
"databricks.sql.telemetry.telemetry_push_client.logger"
132-
) as mock_logger:
133-
request_error = RequestError(
134-
"HTTP request failed", context={"http-code": 500}
135-
)
136-
mock_delegate.request.side_effect = request_error
137-
138-
with pytest.raises(RequestError):
139-
client.request(HttpMethod.POST, "https://test.com", {})
140-
141-
# Should log debug for wrapping/unwrapping
142-
assert mock_logger.debug.call_count >= 1
109+
def test_request_error_with_http_code_429_raises_rate_limit_error(self, client, mock_delegate):
110+
"""Test that rate limit errors raise TelemetryRateLimitError."""
111+
request_error = RequestError(
112+
"HTTP request failed", context={"http-code": 429}
113+
)
114+
mock_delegate.request.side_effect = request_error
115+
116+
with pytest.raises(TelemetryRateLimitError):
117+
client.request(HttpMethod.POST, "https://test.com", {})
118+
119+
def test_request_error_with_http_code_500_raises_original_request_error(self, client, mock_delegate):
120+
"""Test that non-rate-limit errors raise original RequestError."""
121+
request_error = RequestError(
122+
"HTTP request failed", context={"http-code": 500}
123+
)
124+
mock_delegate.request.side_effect = request_error
125+
126+
with pytest.raises(RequestError):
127+
client.request(HttpMethod.POST, "https://test.com", {})
143128

144129
def test_request_error_with_string_http_code(self, client, mock_delegate):
145130
"""Test RequestError with http-code as string (edge case)."""
@@ -161,17 +146,8 @@ def test_http_code_extraction_prioritization(self, client, mock_delegate):
161146
)
162147
mock_delegate.request.side_effect = request_error
163148

164-
with patch(
165-
"databricks.sql.telemetry.telemetry_push_client.logger"
166-
) as mock_logger:
167-
with pytest.raises(TelemetryRateLimitError):
168-
client.request(HttpMethod.POST, "https://test.com", {})
169-
170-
# Verify warning logged with correct status code
171-
mock_logger.warning.assert_called()
172-
warning_call = mock_logger.warning.call_args[0]
173-
assert "503" in str(warning_call)
174-
assert "retries exhausted" in warning_call[0].lower()
149+
with pytest.raises(TelemetryRateLimitError):
150+
client.request(HttpMethod.POST, "https://test.com", {})
175151

176152
def test_non_request_error_exceptions_raised(self, client, mock_delegate):
177153
"""Test that non-RequestError exceptions are wrapped then unwrapped."""

0 commit comments

Comments
 (0)