Skip to content

Commit 68f4c60

Browse files
test(retry): isolate retry tests from shared telemetry executor
The e2e retry tests in retry_test_mixins.py assert exact urllib3 call counts (e.g. `mock_validate_conn.call_count == 6`) while mocking urllib3 globally via `_validate_conn` / `_get_conn`. The shared `TelemetryClientFactory` executor — populated by *prior* tests in the same worker — keeps firing background telemetry pushes that land on the same mocked urllib3 layer, inflating the counts and tripping the assertions. Drain the factory and route any new `initialize_telemetry_client` call to `NoopTelemetryClient` for the duration of the three tests that assert exact counts: - test_oserror_retries - test_retry_max_count_not_exceeded - test_retry_exponential_backoff Factory state is fully restored on exit so subsequent tests see the real telemetry pipeline. Co-authored-by: Isaac
1 parent 63c4c78 commit 68f4c60

1 file changed

Lines changed: 57 additions & 3 deletions

File tree

tests/e2e/common/retry_test_mixins.py

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,58 @@
1515
SessionAlreadyClosedError,
1616
UnsafeToRetryError,
1717
)
18+
from databricks.sql.telemetry.telemetry_client import (
19+
NoopTelemetryClient,
20+
TelemetryClientFactory,
21+
_TelemetryClientHolder,
22+
)
23+
24+
25+
@contextmanager
26+
def _isolated_from_telemetry():
27+
# Tests that mock urllib3 globally (via _get_conn / _validate_conn) also
28+
# intercept background telemetry pushes from the shared
29+
# TelemetryClientFactory executor — inflating mock.call_count and breaking
30+
# assertions like `call_count == 6`. Drain the factory and force any new
31+
# connection to use NoopTelemetryClient for the duration of the test.
32+
with TelemetryClientFactory._lock:
33+
saved_clients = TelemetryClientFactory._clients
34+
saved_executor = TelemetryClientFactory._executor
35+
saved_initialized = TelemetryClientFactory._initialized
36+
for holder in list(saved_clients.values()):
37+
try:
38+
holder.client.close()
39+
except Exception:
40+
pass
41+
if saved_executor is not None:
42+
try:
43+
TelemetryClientFactory._stop_flush_thread()
44+
saved_executor.shutdown(wait=True)
45+
except Exception:
46+
pass
47+
TelemetryClientFactory._clients = {}
48+
TelemetryClientFactory._executor = None
49+
TelemetryClientFactory._initialized = False
50+
51+
def _install_noop(*args, host_url=None, **kwargs):
52+
host_url = TelemetryClientFactory.getHostUrlSafely(host_url)
53+
with TelemetryClientFactory._lock:
54+
TelemetryClientFactory._clients[host_url] = _TelemetryClientHolder(
55+
NoopTelemetryClient()
56+
)
57+
58+
try:
59+
with patch.object(
60+
TelemetryClientFactory,
61+
"initialize_telemetry_client",
62+
staticmethod(_install_noop),
63+
):
64+
yield
65+
finally:
66+
with TelemetryClientFactory._lock:
67+
TelemetryClientFactory._clients = saved_clients
68+
TelemetryClientFactory._executor = saved_executor
69+
TelemetryClientFactory._initialized = saved_initialized
1870

1971

2072
class Client429ResponseMixin:
@@ -253,7 +305,7 @@ def test_retry_urllib3_settings_are_honored(
253305
@patch("databricks.sql.telemetry.telemetry_client.TelemetryClient._send_telemetry")
254306
def test_oserror_retries(self, mock_send_telemetry, extra_params):
255307
"""If a network error occurs during make_request, the request is retried according to policy"""
256-
with patch(
308+
with _isolated_from_telemetry(), patch(
257309
"urllib3.connectionpool.HTTPSConnectionPool._validate_conn",
258310
) as mock_validate_conn:
259311
mock_validate_conn.side_effect = OSError("Some arbitrary network error")
@@ -278,7 +330,9 @@ def test_retry_max_count_not_exceeded(self, mock_send_telemetry, extra_params):
278330
THEN the connector issues six request (original plus five retries)
279331
before raising an exception
280332
"""
281-
with mocked_server_response(status=429, headers={"Retry-After": "0"}) as mock_obj:
333+
with _isolated_from_telemetry(), mocked_server_response(
334+
status=429, headers={"Retry-After": "0"}
335+
) as mock_obj:
282336
with pytest.raises(MaxRetryError) as cm:
283337
extra_params = {**extra_params, **self._retry_policy}
284338
with self.connection(extra_params=extra_params) as conn:
@@ -302,7 +356,7 @@ def test_retry_exponential_backoff(self, mock_send_telemetry, extra_params):
302356
retry_policy["_retry_delay_min"] = 1
303357

304358
time_start = time.time()
305-
with mocked_server_response(
359+
with _isolated_from_telemetry(), mocked_server_response(
306360
status=429, headers={"Retry-After": "8"}
307361
) as mock_obj:
308362
with pytest.raises(RequestError) as cm:

0 commit comments

Comments
 (0)