-
Notifications
You must be signed in to change notification settings - Fork 1k
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 use_generic_streaming_requests
option to make testing server impls easier
#2115
Conversation
NVM |
Adds an option for server trait generation that causes client-streaming methods to receive a generic `Request<impl Stream>` param instead of a concrete `Request<Streaming<...>>` param. This allows for testing trait implementions with any type implementing `Stream`, and not having to go through the trouble of encoding a `Streaming` object. Fixes hyperium#462
@@ -257,92 +263,61 @@ fn generate_trait_methods<T: Service>( | |||
quote!(&self) | |||
}; | |||
|
|||
let method = match ( |
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.
I'm not sure if the benefits of adding this feature are worth the increased complexity that comes with adding more features. However, the refactoring of this part that was done to add this feature seems to improve the readability and maintainability of the code, so if it be separated into a separate pull request, I think we can proceed with the discussion separately.
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.
Sure, the refactoring is in a separate commit so I'll put it in its own PR. Thanks!
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.
Thanks for working on this; currently, it is incredibly painful! |
Motivation
Currently, testing the server trait implementations of client-streaming methods (i.e. requests that receive a stream) is quite difficult, and requires jumping through hoops in order to construct a
Streaming
object (see #462 for one way of doing it).Solution
Adds a codegen option that makes the server trait go from this:
to this:
Since
Request<Streaming<Message>>
implsIntoStreamingRequest<Message = Result<Message, Status>>
, the grpc server can continue invoking these handlers in the same way, but it also allows any object implementingStream<Item = Result<Message, Status>>
to be used.This makes testing much easier.