Skip to content

Commit d43ac1c

Browse files
authored
test: add transport-wiring regression tests for #459 (#485)
* test: add transport-wiring regression tests for #459 The existing test_client_limits_respect_max_parallel_requests only checks the limits property, which was always computed correctly even before the fix. Add mock-capture tests that patch HTTPTransport and AsyncHTTPTransport constructors, trigger lazy init via completion() / acompletion(), and assert the constructors received the correct limits. These tests fail on the pre-fix code (assert_called_once fails because the old code never explicitly constructed the transport). Made-with: Cursor * test: address review — parametrize over AnthropicClient, use helpers Parametrize all three #459 regression tests over both OpenAICompatibleClient and AnthropicClient (wiring lives in HttpModelClient, so both subclasses need coverage). Use the existing _make_openai_client / _make_anthropic_client helpers with **kwargs instead of constructing clients directly. Move transport patch constants up with the other module-level constants. Made-with: Cursor
1 parent 5265745 commit d43ac1c

File tree

1 file changed

+89
-7
lines changed

1 file changed

+89
-7
lines changed

packages/data-designer-engine/tests/engine/models/clients/test_native_http_clients.py

Lines changed: 89 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,18 @@
2323
_ANTHROPIC_ENDPOINT = "https://api.anthropic.com/v1"
2424
_SYNC_CLIENT_PATCH = "data_designer.engine.models.clients.adapters.http_model_client.lazy.httpx.Client"
2525
_ASYNC_CLIENT_PATCH = "data_designer.engine.models.clients.adapters.http_model_client.lazy.httpx.AsyncClient"
26+
_HTTP_TRANSPORT_PATCH = "data_designer.engine.models.clients.adapters.http_model_client.lazy.httpx.HTTPTransport"
27+
_ASYNC_HTTP_TRANSPORT_PATCH = (
28+
"data_designer.engine.models.clients.adapters.http_model_client.lazy.httpx.AsyncHTTPTransport"
29+
)
2630

2731

2832
def _make_openai_client(
2933
*,
3034
concurrency_mode: ClientConcurrencyMode = ClientConcurrencyMode.SYNC,
3135
sync_client: MagicMock | None = None,
3236
async_client: MagicMock | None = None,
37+
**kwargs: Any,
3338
) -> OpenAICompatibleClient:
3439
return OpenAICompatibleClient(
3540
provider_name=_OPENAI_PROVIDER,
@@ -38,6 +43,7 @@ def _make_openai_client(
3843
concurrency_mode=concurrency_mode,
3944
sync_client=sync_client,
4045
async_client=async_client,
46+
**kwargs,
4147
)
4248

4349

@@ -46,6 +52,7 @@ def _make_anthropic_client(
4652
concurrency_mode: ClientConcurrencyMode = ClientConcurrencyMode.SYNC,
4753
sync_client: MagicMock | None = None,
4854
async_client: MagicMock | None = None,
55+
**kwargs: Any,
4956
) -> AnthropicClient:
5057
return AnthropicClient(
5158
provider_name=_ANTHROPIC_PROVIDER,
@@ -54,6 +61,7 @@ def _make_anthropic_client(
5461
concurrency_mode=concurrency_mode,
5562
sync_client=sync_client,
5663
async_client=async_client,
64+
**kwargs,
5765
)
5866

5967

@@ -295,17 +303,91 @@ async def test_acompletion_lazy_initializes_async_client(
295303
# Connection pool size regression tests (issue #459)
296304
# ---------------------------------------------------------------------------
297305

306+
_POOL_LIMITS_CASES = [
307+
pytest.param(_make_openai_client, id="openai"),
308+
pytest.param(_make_anthropic_client, id="anthropic"),
309+
]
310+
311+
_SYNC_TRANSPORT_WIRING_CASES = [
312+
pytest.param(_make_openai_client, _OPENAI_MODEL, _make_openai_chat_response(), id="openai"),
313+
pytest.param(_make_anthropic_client, _ANTHROPIC_MODEL, _make_anthropic_chat_response(), id="anthropic"),
314+
]
315+
316+
_ASYNC_TRANSPORT_WIRING_CASES = [
317+
pytest.param(_make_openai_client, _OPENAI_MODEL, _make_openai_chat_response(), id="openai"),
318+
pytest.param(_make_anthropic_client, _ANTHROPIC_MODEL, _make_anthropic_chat_response(), id="anthropic"),
319+
]
320+
298321

299-
def test_client_limits_respect_max_parallel_requests() -> None:
300-
"""Connection pool limits must reflect max_parallel_requests (regression for issue #459).
322+
@pytest.mark.parametrize("client_factory", _POOL_LIMITS_CASES)
323+
def test_client_limits_respect_max_parallel_requests(client_factory: Callable[..., Any]) -> None:
324+
"""Connection pool limits must reflect max_parallel_requests.
301325
302326
pool_max = max(32, 2 * max_parallel_requests) = max(32, 600) = 600
303327
"""
304-
client = OpenAICompatibleClient(
305-
provider_name=_OPENAI_PROVIDER,
306-
endpoint=_OPENAI_ENDPOINT,
307-
api_key="sk-test",
308-
max_parallel_requests=300,
328+
client = client_factory(
309329
concurrency_mode=ClientConcurrencyMode.SYNC,
330+
max_parallel_requests=300,
310331
)
311332
assert client.limits.max_connections == 600
333+
334+
335+
@pytest.mark.parametrize(("client_factory", "model_name", "response_json"), _SYNC_TRANSPORT_WIRING_CASES)
336+
@patch(_HTTP_TRANSPORT_PATCH)
337+
@patch(_SYNC_CLIENT_PATCH)
338+
def test_sync_pool_limits_forwarded_to_transport(
339+
mock_client_cls: MagicMock,
340+
mock_transport_cls: MagicMock,
341+
client_factory: Callable[..., Any],
342+
model_name: str,
343+
response_json: dict[str, Any],
344+
) -> None:
345+
"""Regression for #459: limits must reach HTTPTransport, not just httpx.Client.
346+
347+
The pre-fix code passed limits= to httpx.Client which silently ignores it
348+
when a custom transport= is provided. The fix constructs HTTPTransport
349+
with the correct limits before wrapping it in RetryTransport. This test
350+
fails on the pre-fix code because HTTPTransport was never constructed
351+
explicitly (assert_called_once fails).
352+
"""
353+
mock_client_cls.return_value = MagicMock(post=MagicMock(return_value=mock_httpx_response(response_json)))
354+
client = client_factory(
355+
concurrency_mode=ClientConcurrencyMode.SYNC,
356+
max_parallel_requests=300,
357+
)
358+
client.completion(_make_chat_request(model_name))
359+
360+
mock_transport_cls.assert_called_once()
361+
limits = mock_transport_cls.call_args.kwargs["limits"]
362+
assert limits.max_connections == 600
363+
assert limits.max_keepalive_connections == 300
364+
365+
366+
@pytest.mark.parametrize(("client_factory", "model_name", "response_json"), _ASYNC_TRANSPORT_WIRING_CASES)
367+
@patch(_ASYNC_HTTP_TRANSPORT_PATCH)
368+
@patch(_ASYNC_CLIENT_PATCH)
369+
@pytest.mark.asyncio
370+
async def test_async_pool_limits_forwarded_to_transport(
371+
mock_client_cls: MagicMock,
372+
mock_transport_cls: MagicMock,
373+
client_factory: Callable[..., Any],
374+
model_name: str,
375+
response_json: dict[str, Any],
376+
) -> None:
377+
"""Regression for #459: limits must reach AsyncHTTPTransport for async clients.
378+
379+
Same issue as the sync path — the pre-fix code never explicitly constructed
380+
AsyncHTTPTransport, so RetryTransport created a default pool with 100
381+
connections regardless of max_parallel_requests.
382+
"""
383+
mock_client_cls.return_value = MagicMock(post=AsyncMock(return_value=mock_httpx_response(response_json)))
384+
client = client_factory(
385+
concurrency_mode=ClientConcurrencyMode.ASYNC,
386+
max_parallel_requests=300,
387+
)
388+
await client.acompletion(_make_chat_request(model_name))
389+
390+
mock_transport_cls.assert_called_once()
391+
limits = mock_transport_cls.call_args.kwargs["limits"]
392+
assert limits.max_connections == 600
393+
assert limits.max_keepalive_connections == 300

0 commit comments

Comments
 (0)