-
Notifications
You must be signed in to change notification settings - Fork 39
feat: basic websocket support #65
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
base: master
Are you sure you want to change the base?
Conversation
@h2non the PR is missing a lot of tests, documentation and a bit of a polish, but let me know what you think so far. |
f8984a8
to
7d84960
Compare
7d84960
to
b66e1b6
Compare
req.url = url | ||
|
||
# TODO does this work multithreaded?!? | ||
self._mock = self.engine.match(req) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn m 8000 ore.
It will not work. Mutex synchronization is required to make it safe. However, I feel that's another feature that has to be implemented in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! This looks promising and ambitious!
Adding some extra tests would be great to cover more complex scenarios.
It would be great it can support as well this client, which works on top of asyncio
:
https://github.com/aaugustin/websockets
At user API design level, I wonder about how the mock DSL can be improved to mock up exchanged frames with arbitrary responses provided by a routine.
I guess one obvious solution would be allowing the user to pass a function with a signature like:
def framer(frame, request) -> MockFrame:
// Do magic things
return mock
At a user definition API level, I'm inclined to overload the body()
method to support this case:
def frame_mock(frame, request) -> MockFrame:
// Do magic things
return mock
(pook.get('ws://some-non-existing.org')
.reply(204)
.body(frame_mock)
Alternatively, implement a custom method for frames specific case:
(pook.get('ws://some-non-existing.org')
.reply(204)
.frames(frames_mock)
Opinions?
Any update on this? |
body() could be abused if used múltiple times, being each message a body, if not It would have the full conversation, that's impractical. |
The same can be done for Server-Send Events, being body() used for each event. |
@piranna can you clarify what you mean? Please feel free to open a PR targeting the latest master if you have ideas for how to implement this and I will review it gladly. |
WebSockets (and Server-Send Events) don't have a single "body", but instead they send multiple messages, each one can be considered as a "body", and in fact they can come as as response to an incoming "request" message.
Maybe I'll do, that would depend if I can convince my boss to start doing testing... |
Yes, understood. Calling In your original comments, were you referring to a specific issue with the implementation of this PR or just general feedback? |
General feedback, I think being able to send multiple messages would be a needed feature. Any idea how this could be implemented? In SSE is not so critical since it's unidirectional and on the wire response looks like a single resource, just only very slowly written, but for WebSockets it would be needed. |
The implementation in this PR already supports sending multiple responses. Each call to receive a frame continues the iteration through the list of frames passed to @h2non's point was that the frames cannot be created in response to individual messages, so the suggestion of using a callback that can interpret the sent frame and create the response is meant to solve that. Is that what you're getting at? A complication is that I'm not sure how to handle the fact that websocket connections are not all transactions, there isn't always a single response frame for every sent frame, and the timing of those responses may be significant. The frame generation callback could handle that nicely, if it's necessary to mock multiple frames sent after receiving a specific message or at the start of the connection, for example. It's quite complicated. SSE is not so complicated because of its unidirectionality, but should still be a separate feature and implementation. |
That's the point I'm trying to get attention into.
Agree, just only since both can generate multiple messages back, I think solution could be common to both. |
This PR adds a very very basic (and highly likely buggy) support for mocking websockets.