Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests to validation behaviour of graphql-transport-ws subscriptions #3671

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3445d24
Add tests for blocking operation validation
kristjanvalur Mar 8, 2023
2be1bf4
Move validation into the task
kristjanvalur Apr 8, 2023
02772ee
Add some error/validation test cases
kristjanvalur Apr 9, 2023
7687c4e
Use duck typing to detect an ExecutionResult/GraphQLExeucitonResult
kristjanvalur Apr 9, 2023
6198007
add an async context getter for tests which is easily patchable.
kristjanvalur Apr 9, 2023
9ef3bf0
Add tests to ensure context_getter does not block connection
kristjanvalur Apr 9, 2023
7cf537e
Move context getter and root getter into worker task
kristjanvalur Apr 9, 2023
e41ffe9
Catch top level errors
kristjanvalur Jun 8, 2023
c3d6447
Add a test for the task error handler
kristjanvalur Jun 9, 2023
a9589e4
add release.md
kristjanvalur May 9, 2023
ca229a7
Remove dead code, fix coverage
kristjanvalur Jun 30, 2023
1a62b1a
remove special case for AsyncMock
kristjanvalur Aug 3, 2023
76716d0
Add "no cover" to schema code which is designed to not be hit.
kristjanvalur Nov 8, 2023
1ad929b
Update tests for litestar
kristjanvalur Mar 31, 2024
8ced4e4
Litestar integration must be excluded from long test, like Starlite.
kristjanvalur Mar 31, 2024
35a8e68
coverage
kristjanvalur Mar 31, 2024
4084639
Mark some test schema methods as no cover since they are not always used
kristjanvalur Apr 2, 2024
d17a4d4
Mypy support for SubscriptionExecutionResult
kristjanvalur Sep 7, 2024
a1d0695
ruff
kristjanvalur Sep 7, 2024
e43aca8
Remove unused method for coverage
kristjanvalur Sep 8, 2024
3d97deb
Merge branch 'main' into kristjan/validate-in-task
kristjanvalur Oct 13, 2024
1be5a06
revert the handler to original state
kristjanvalur Oct 13, 2024
c074e09
Remove tests for long contexts
kristjanvalur Oct 13, 2024
373400f
Revert "add an async context getter for tests which is easily patchab…
kristjanvalur Oct 13, 2024
c4d0b05
cleanup
kristjanvalur Oct 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions RELEASE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Release type: patch

Fix error handling for query operations over graphql-transport-ws
25 changes: 25 additions & 0 deletions tests/views/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,19 @@
return False


class ConditionalFailPermission(BasePermission):
@property
def message(self):
return f"failed after sleep {self.sleep}"

Check warning on line 26 in tests/views/schema.py

View check run for this annotation

Codecov / codecov/patch

tests/views/schema.py#L26

Added line #L26 was not covered by tests

async def has_permission(self, source, info, **kwargs: Any) -> bool:
self.sleep = kwargs.get("sleep", None)
self.fail = kwargs.get("fail", True)

Check warning on line 30 in tests/views/schema.py

View check run for this annotation

Codecov / codecov/patch

tests/views/schema.py#L29-L30

Added lines #L29 - L30 were not covered by tests
if self.sleep is not None:
await asyncio.sleep(kwargs["sleep"])
return not self.fail

Check warning on line 33 in tests/views/schema.py

View check run for this annotation

Codecov / codecov/patch

tests/views/schema.py#L32-L33

Added lines #L32 - L33 were not covered by tests


class MyExtension(SchemaExtension):
def get_results(self) -> Dict[str, str]:
return {"example": "example"}
Expand Down Expand Up @@ -80,6 +93,12 @@
def always_fail(self) -> Optional[str]:
return "Hey"

@strawberry.field(permission_classes=[ConditionalFailPermission])
def conditional_fail(
self, sleep: Optional[float] = None, fail: bool = False
) -> str:
return "Hey"

Check warning on line 100 in tests/views/schema.py

View check run for this annotation

Codecov / codecov/patch

tests/views/schema.py#L100

Added line #L100 was not covered by tests

@strawberry.field
async def error(self, message: str) -> AsyncGenerator[str, None]:
yield GraphQLError(message) # type: ignore
Expand Down Expand Up @@ -262,6 +281,12 @@
finally:
await asyncio.sleep(delay)

@strawberry.subscription(permission_classes=[ConditionalFailPermission])
async def conditional_fail(
self, sleep: Optional[float] = None, fail: bool = False
) -> AsyncGenerator[str, None]:
yield "Hey" # pragma: no cover


class Schema(strawberry.Schema):
def process_errors(
Expand Down
159 changes: 159 additions & 0 deletions tests/websockets/test_graphql_transport_ws.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
from pytest_mock import MockerFixture

from strawberry.subscriptions import GRAPHQL_TRANSPORT_WS_PROTOCOL
from strawberry.subscriptions.protocols.graphql_transport_ws.handlers import (

Check warning on line 16 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L16

Added line #L16 was not covered by tests
BaseGraphQLTransportWSHandler,
)
from strawberry.subscriptions.protocols.graphql_transport_ws.types import (
CompleteMessage,
ConnectionAckMessage,
Expand Down Expand Up @@ -437,6 +440,28 @@
process_errors.assert_called_once()


async def test_query_field_errors(ws: WebSocketClient):
await ws.send_json(

Check warning on line 444 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L443-L444

Added lines #L443 - L444 were not covered by tests
SubscribeMessage(
id="sub1",
payload=SubscribeMessagePayload(
query="query { notASubscriptionField }",
),
).as_dict()
)

response = await ws.receive_json()
assert response["type"] == ErrorMessage.type
assert response["id"] == "sub1"
assert len(response["payload"]) == 1
assert response["payload"][0].get("path") is None
assert response["payload"][0]["locations"] == [{"line": 1, "column": 9}]
assert (

Check warning on line 459 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L453-L459

Added lines #L453 - L459 were not covered by tests
response["payload"][0]["message"]
== "Cannot query field 'notASubscriptionField' on type 'Query'."
)


async def test_subscription_cancellation(ws: WebSocketClient):
await ws.send_json(
SubscribeMessage(
Expand Down Expand Up @@ -963,3 +988,137 @@
mock.assert_called_once()
assert_next(response, "sub1", {"echo": "Hi"})
assert "extensions" not in response["payload"]


async def test_validation_query(ws: WebSocketClient):

Check warning on line 993 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L993

Added line #L993 was not covered by tests
"""
Test validation for query
"""
await ws.send_json(

Check warning on line 997 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L997

Added line #L997 was not covered by tests
SubscribeMessage(
id="sub1",
payload=SubscribeMessagePayload(
query="query { conditionalFail(fail:true) }"
),
Comment on lines +993 to +1002
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (testing): Inconsistency in error handling between queries and subscriptions

As noted in the PR description, there's a discrepancy in how validation errors are handled for queries vs. subscriptions. This test is failing because it expects an ErrorMessage, but receives a NextMessage instead. Consider updating the implementation to handle validation errors consistently across queries and subscriptions.

).as_dict()
)

# We expect an error message directly
response = await ws.receive_json()
assert response["type"] == ErrorMessage.type
assert response["id"] == "sub1"
assert len(response["payload"]) == 1
assert response["payload"][0].get("path") == ["conditionalFail"]
assert response["payload"][0]["message"] == "failed after sleep None"

Check warning on line 1012 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1007-L1012

Added lines #L1007 - L1012 were not covered by tests


async def test_validation_subscription(ws: WebSocketClient):

Check warning on line 1015 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1015

Added line #L1015 was not covered by tests
"""
Test validation for subscription
"""
await ws.send_json(

Check warning on line 1019 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1019

Added line #L1019 was not covered by tests
SubscribeMessage(
id="sub1",
payload=SubscribeMessagePayload(
query="subscription { conditionalFail(fail:true) }"
),
).as_dict()
)

# We expect an error message directly
response = await ws.receive_json()
assert response["type"] == ErrorMessage.type
assert response["id"] == "sub1"
assert len(response["payload"]) == 1
assert response["payload"][0].get("path") == ["conditionalFail"]
assert response["payload"][0]["message"] == "failed after sleep None"

Check warning on line 1034 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1029-L1034

Added lines #L1029 - L1034 were not covered by tests


async def test_long_validation_concurrent_query(ws: WebSocketClient):

Check warning on line 1037 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1037

Added line #L1037 was not covered by tests
"""
Test that the websocket is not blocked while validating a
single-result-operation
"""
await ws.send_json(

Check warning on line 1042 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1042

Added line #L1042 was not covered by tests
SubscribeMessage(
id="sub1",
payload=SubscribeMessagePayload(
query="query { conditionalFail(sleep:0.1) }"
),
).as_dict()
)
await ws.send_json(

Check warning on line 1050 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1050

Added line #L1050 was not covered by tests
SubscribeMessage(
id="sub2",
payload=SubscribeMessagePayload(
query="query { conditionalFail(fail:false) }"
),
).as_dict()
)

# we expect the second query to arrive first, because the
# first query is stuck in validation
response = await ws.receive_json()
assert_next(response, "sub2", {"conditionalFail": "Hey"})

Check warning on line 1062 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1061-L1062

Added lines #L1061 - L1062 were not covered by tests


async def test_long_validation_concurrent_subscription(ws: WebSocketClient):

Check warning on line 1065 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1065

Added line #L1065 was not covered by tests
"""
Test that the websocket is not blocked while validating a
subscription
"""
await ws.send_json(

Check warning on line 1070 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1070

Added line #L1070 was not covered by tests
SubscribeMessage(
id="sub1",
payload=SubscribeMessagePayload(
query="subscription { conditionalFail(sleep:0.1) }"
),
).as_dict()
)
await ws.send_json(

Check warning on line 1078 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1078

Added line #L1078 was not covered by tests
SubscribeMessage(
id="sub2",
payload=SubscribeMessagePayload(
query="query { conditionalFail(fail:false) }"
),
).as_dict()
)

# we expect the second query to arrive first, because the
# first operation is stuck in validation
response = await ws.receive_json()
assert_next(response, "sub2", {"conditionalFail": "Hey"})

Check warning on line 1090 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1089-L1090

Added lines #L1089 - L1090 were not covered by tests


async def test_task_error_handler(ws: WebSocketClient):

Check warning on line 1093 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1093

Added line #L1093 was not covered by tests
"""
Test that error handling works
"""
# can't use a simple Event here, because the handler may run
# on a different thread
wakeup = False

Check warning on line 1099 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1099

Added line #L1099 was not covered by tests

# a replacement method which causes an error in th eTask
async def op(*args: Any, **kwargs: Any):

Check warning on line 1102 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1102

Added line #L1102 was not covered by tests
nonlocal wakeup
wakeup = True
raise ZeroDivisionError("test")

Check warning on line 1105 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1104-L1105

Added lines #L1104 - L1105 were not covered by tests

with patch.object(BaseGraphQLTransportWSHandler, "task_logger") as logger:
with patch.object(BaseGraphQLTransportWSHandler, "handle_operation", op):

Check warning on line 1108 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1107-L1108

Added lines #L1107 - L1108 were not covered by tests
# send any old subscription request. It will raise an error
await ws.send_json(

Check warning on line 1110 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1110

Added line #L1110 was not covered by tests
SubscribeMessage(
id="sub1",
payload=SubscribeMessagePayload(
query="subscription { conditionalFail(sleep:0) }"
),
).as_dict()
)

# wait for the error to be logged. Must use timed loop and not event.
while not wakeup: # noqa: ASYNC110
await asyncio.sleep(0.01)

Check warning on line 1121 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1121

Added line #L1121 was not covered by tests
# and another little bit, for the thread to finish
await asyncio.sleep(0.01)
assert logger.exception.called

Check warning on line 1124 in tests/websockets/test_graphql_transport_ws.py

View check run for this annotation

Codecov / codecov/patch

tests/websockets/test_graphql_transport_ws.py#L1123-L1124

Added lines #L1123 - L1124 were not covered by tests
Loading