Provide 'fake' responses for generated test clients #855
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
glbrntt
added
kind/enhancement
| /// | ||
| /// `sendError` may be used to terminate an RPC without providing a response. As for `sendMessage`, | ||
| /// the `trailingMetadata` defaults to being empty. | ||
| public class FakeUnaryResponse<Request: GRPCPayload, Response: GRPCPayload>: FakeResponseStream<Request, Response> { |
There was a problem hiding this comment.
Making this type public means that this can be used in prod frameworks/apps depending on gRPC as well, not only their test targets. Is that the intended behaviour?
Do correct me if I'm wrong, but I would expect the test-only types to be internal and hence only accessible from (unit) test targets with @testable import GRPC.
Collaborator
Author
There was a problem hiding this comment.
I don't think it's sensible for consumers of a framework to use @testable imports: it blurs the line between what is and is not acceptable for the consumer to use. Changing internal code only requires a SemVer patch change which could lead the consumer being broken in unexpected ways between updates. Making this public provides the right SemVer guarantees.
Lukasa
requested changes
Jun 24, 2020
Collaborator
Lukasa
left a comment
Lukasa
left a comment
There was a problem hiding this comment.
I have a few minor notes but this looks really good.
| /// - response: The message to send. | ||
| /// - initialMetadata: The initial metadata to send. By default the metadata will be empty. | ||
| /// - trailingMetadata: The trailing metadata to send. By default the metadata will be empty. | ||
| /// - status: The status to send. By default this have status code '.ok'. |
Collaborator
There was a problem hiding this comment.
Nit
Suggested change
| /// - status: The status to send. By default this have status code '.ok'. | |
| /// - status: The status to send. By default this is status code '.ok'. |
MrMage
requested changes
Jun 25, 2020
Collaborator
MrMage
left a comment
MrMage
left a comment
There was a problem hiding this comment.
Looks great overall!
Reviewed 1 of 3 files at r1.
Reviewable status: 1 of 3 files reviewed, 6 unresolved discussions (waiting on @davidpasztor, @glbrntt, @Lukasa, and @MrMage)
Sources/GRPC/FakeResponseStream.swift, line 231 at r1 (raw file):
/// - trailingMetadata: The trailing metadata to send. By default the metadata will be empty. /// - status: The status to send. By default this have status code '.ok'. public func sendMessage(
Love this "convenience method"! I wonder whether we should even provide a convenience initializer with this? I.e.
FakeUnaryResponse(responseMessage)
Sources/GRPC/FakeResponseStream.swift, line 281 at r1 (raw file):
/// Like `sendEnd`, `trailingMetadata` is empty by default. public class FakeStreamingResponse<Request: GRPCPayload, Response: GRPCPayload>: FakeResponseStream<Request, Response> { public override init() {
Same here — how about a convenience initializer for the "happy case" of just returning a bunch of messages?
Motivation: When consuming gRPC it is often helpful to be able to write tests that ensure the client is integrated correctly. At the moment this is only possible by running a local gRPC server with a custom service handler to return the responses you would like to test. Modifications: This pull request is the first of a series to introduce generated test clients. It adds 'fake response streams', which allow the user to act as the server for a single RPC. The stream comes in unary and streaming varieties and is backed by an EmbeddedChannel. Later PRs will connect these streaming objects to call objects and provide code generation so that users can more readily make fake streams for their test clients. Result: We're on the road to generated test clients.
glbrntt
commented
Jun 25, 2020
Collaborator
Author
glbrntt
left a comment
glbrntt
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 3 files reviewed, 6 unresolved discussions (waiting on @davidpasztor, @glbrntt, @Lukasa, and @MrMage)
Sources/GRPC/FakeResponseStream.swift, line 231 at r1 (raw file):
Previously, MrMage (Daniel Alm) wrote…
Love this "convenience method"! I wonder whether we should even provide a convenience initializer with this? I.e.
FakeUnaryResponse(responseMessage)
Possibly, but I think we should address this when we add the generated code (as that will be how users will create these the majority of the time) since we'll need to generate a function for each convenience init as well.
glbrntt
force-pushed
the
gb-test-client-fake-responses
branch
from
b213865 to
efb1851
Compare
MrMage
approved these changes
Jun 25, 2020
Lukasa
approved these changes
Jun 26, 2020
glbrntt
added a commit
to glbrntt/grpc-swift
that referenced
this pull request
Jun 29, 2020
Motivation: When consuming gRPC it is often helpful to be able to write tests that ensure the client is integrated correctly. At the moment this is only possible by running a local gRPC server with a custom service handler to return the responses you would like to test. Modifications: This is a continuation of the work started in grpc#855. - This change addes a 'FakeChannel' this is the glue that binds the Call objects with the 'fake responses' added in the aforementioned pull request. - Also adds a 'write capturing' handler which forwards request parts to a handler provided on the fake response. - Appropriate internal initializers on each of the call types. Result: - Users can manually create 'test' RPCs.
Merged
glbrntt
added a commit
that referenced
this pull request
Jun 30, 2020
Motivation: When consuming gRPC it is often helpful to be able to write tests that ensure the client is integrated correctly. At the moment this is only possible by running a local gRPC server with a custom service handler to return the responses you would like to test. Modifications: This is a continuation of the work started in #855. - This change addes a 'FakeChannel' this is the glue that binds the Call objects with the 'fake responses' added in the aforementioned pull request. - Also adds a 'write capturing' handler which forwards request parts to a handler provided on the fake response. - Appropriate internal initializers on each of the call types. Result: - Users can manually create 'test' RPCs.
glbrntt
added a commit
to glbrntt/grpc-swift
that referenced
this pull request
Jul 1, 2020
Motivation: When consuming gRPC it is often helpful to be able to write tests that ensure the client is integrated correctly. At the moment this is only possible by running a local gRPC server with a custom service handler to return the responses you would like to test. Modifications: This builds on work in grpc#855, grpc#864, and grpc#865. This pull request introduces code generation for the test clients. It provides type-safe wrappers and convenience methods on top of `FakeChannel` and a code-gen option to enable 'TestClient' generation. It also removes an `init` requirement on the `GRPCClient` protocol. Result: Users can generate test clients.
glbrntt
added a commit
to glbrntt/grpc-swift
that referenced
this pull request
Jul 1, 2020
Motivation: When consuming gRPC it is often helpful to be able to write tests that ensure the client is integrated correctly. At the moment this is only possible by running a local gRPC server with a custom service handler to return the responses you would like to test. Modifications: This builds on work in grpc#855, grpc#864, and grpc#865. This pull request introduces code generation for the test clients. It provides type-safe wrappers and convenience methods on top of `FakeChannel` and a code-gen option to enable 'TestClient' generation. It also removes an `init` requirement on the `GRPCClient` protocol. Result: Users can generate test clients.
glbrntt
added a commit
to glbrntt/grpc-swift
that referenced
this pull request
Jul 2, 2020
Motivation: When consuming gRPC it is often helpful to be able to write tests that ensure the client is integrated correctly. At the moment this is only possible by running a local gRPC server with a custom service handler to return the responses you would like to test. Modifications: This builds on work in grpc#855, grpc#864, and grpc#865. This pull request introduces code generation for the test clients. It provides type-safe wrappers and convenience methods on top of `FakeChannel` and a code-gen option to enable 'TestClient' generation. It also removes an `init` requirement on the `GRPCClient` protocol. Result: Users can generate test clients.
MrMage
pushed a commit
that referenced
this pull request
Jul 3, 2020
* Provide the codegen with an option to generate test clients Motivation: When consuming gRPC it is often helpful to be able to write tests that ensure the client is integrated correctly. At the moment this is only possible by running a local gRPC server with a custom service handler to return the responses you would like to test. Modifications: This builds on work in #855, #864, and #865. This pull request introduces code generation for the test clients. It provides type-safe wrappers and convenience methods on top of `FakeChannel` and a code-gen option to enable 'TestClient' generation. It also removes an `init` requirement on the `GRPCClient` protocol. Result: Users can generate test clients. * Regenerate * fix type, add assertion * Add "Remaining"
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
When consuming gRPC it is often helpful to be able to write tests that
ensure the client is integrated correctly. At the moment this is only
possible by running a local gRPC server with a custom service handler to
return the responses you would like to test.
Modifications:
This pull request is the first of a series to introduce generated test
clients. It adds 'fake response streams', which allow the user to act as
the server for a single RPC. The stream comes in unary and streaming
varieties and is backed by an EmbeddedChannel.
Later PRs will connect these streaming objects to call objects and
provide code generation so that users can more readily make fake streams
for their test clients.
Result:
We're on the road to generated test clients.
This change is