test(BA-5583): add unit tests for graphql-transport-ws module#10773
test(BA-5583): add unit tests for graphql-transport-ws module#10773
Conversation
Cover all classes in the graphql_ws package: - types.py: ClientMessage discriminated union, server message serialization - connection.py: WSReceiver, WSSender, GraphQLWSConnection - subscriptions.py: SubscriptionExecutor lifecycle - handler.py: GraphQLTransportWSHandler message dispatch Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a dedicated unit-test suite for the graphql-transport-ws implementation under src/ai/backend/manager/api/gql/graphql_ws/, aiming to cover protocol DTOs, the aiohttp WS connection wrapper, subscription execution, and the top-level handler loop.
Changes:
- Introduces unit tests for message DTO parsing/serialization (
types.py) and sender/receiver behavior (connection.py). - Adds subscription lifecycle tests (start/duplicate, cancellation, streaming, errors) for
SubscriptionExecutor. - Adds handler-level dispatch tests for subscribe/complete/ping and exception handling, plus a Pants
python_teststarget.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/manager/api/gql/graphql_ws/test_types.py | Tests Pydantic DTO validation/serialization for graphql-transport-ws messages. |
| tests/unit/manager/api/gql/graphql_ws/test_connection.py | Tests WSReceiver/WSSender behaviors and connection init handshake flow. |
| tests/unit/manager/api/gql/graphql_ws/test_subscriptions.py | Tests per-connection subscription task management and streaming behavior. |
| tests/unit/manager/api/gql/graphql_ws/test_handler.py | Tests handler message-loop dispatch and cleanup behavior. |
| tests/unit/manager/api/gql/graphql_ws/BUILD | Adds Pants target to run the new unit tests. |
| tests/unit/manager/api/gql/graphql_ws/init.py | Marks the directory as a Python package for test discovery/imports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def mock_subs(self) -> AsyncMock: | ||
| return AsyncMock() | ||
|
|
||
| @pytest.fixture(autouse=True) | ||
| def _patch_handler(self, mock_conn: AsyncMock, mock_subs: AsyncMock) -> Generator[None]: | ||
| with ( | ||
| patch( | ||
| "ai.backend.manager.api.gql.graphql_ws.handler.GraphQLWSConnection.open", | ||
| new_callable=AsyncMock, | ||
| return_value=mock_conn, | ||
| ), | ||
| patch( | ||
| "ai.backend.manager.api.gql.graphql_ws.handler.SubscriptionExecutor", | ||
| return_value=mock_subs, | ||
| ), |
There was a problem hiding this comment.
SubscriptionExecutor.cancel() is synchronous in production (subs.cancel(message) is not awaited in GraphQLTransportWSHandler.handle()), but the test patches SubscriptionExecutor to return an AsyncMock. That makes mock_subs.cancel awaitable and can create un-awaited coroutine warnings / inaccurate behavior. Prefer a MagicMock (optionally with a spec) for mock_subs, and make only async methods like start/cancel_all into AsyncMocks.
SubscriptionExecutor.cancel() is synchronous, so mock_subs should be a MagicMock with only start/cancel_all as AsyncMock. This avoids un-awaited coroutine issues from AsyncMock on sync methods. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Will continue after #10780 is merged |
Summary
graphql_wspackage (src/ai/backend/manager/api/gql/graphql_ws/) introduced in BA-5564types.py), connection wrapper (connection.py), subscription executor (subscriptions.py), and WS handler (handler.py)Test plan
pants lintpassespants checkpasses (no mypy errors in test files)pants test tests/unit/manager/api/gql/graphql_ws/— all 48 tests passResolves BA-5583