Provide the codegen with an option to generate test clients

[glbrntt]

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.

---

This change is 

[MrMage]

Looks great overall!

Reviewed 11 of 11 files at r1.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @glbrntt and @Lukasa)

---

Sources/Examples/Echo/Model/echo.grpc.swift, line 185 at r1 (raw file):

  }

  /// Immediately returns an echo of a request.

I wonder whether we should implement these on the protocol instead, and only there?

---

Sources/protoc-gen-grpc-swift/Generator-Client.swift, line 292 at r1 (raw file):

extension Generator {
  fileprivate func printFakeResponseStreams() {

Should we extract this into a separate file? (Might require increasing the visibility of some of the other methods, but that's acceptable for tooling code.)

---

Sources/protoc-gen-grpc-swift/options.swift, line 88 at r1 (raw file):

      case "TestClient":
        if let value = Bool(pair.value) {
          self.generateTestClient = value

super-nit: inconsistent use of self.

---

Tests/GRPCTests/TestClientExample.swift, line 38 at r1 (raw file):

    // Send the response; this defaults to sending back an '.ok' status.
    XCTAssertNoThrow(try getResponseStream.sendMessage(.with { $0.text = "Bar!" }))

How about adding a convenience initializer to makeGetResponseStream() with the desired response?

---

Tests/GRPCTests/TestClientExample.swift, line 42 at r1 (raw file):

    // Check the response values:
    XCTAssertEqual(try get.response.wait(), .with { $0.text = "Bar!" })
    XCTAssertTrue(try get.status.map { $0.isOk }.wait())

What would happen if we called client.get a second time now?

Also, is it possible to inspect the sent request? (If so, could you demo it?)

Can we assert that all calls have been "consumed", i.e. all expected calls were sent?

---

Tests/GRPCTests/TestClientExample.swift, line 104 at r1 (raw file):

    }

    // Send some requests.

Could we also test this in an "interleaved" fashion?

---

Tests/GRPCTests/TestClientExample.swift, line 121 at r1 (raw file):

}

// These tests demonstrate using the generated protocol for an RPC, rather than using a generated

Sorry, what exactly is the difference to the above? Also, why e.g. the sayHello method?

[glbrntt]

Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @Lukasa and @MrMage)

---

Sources/Examples/Echo/Model/echo.grpc.swift, line 185 at r1 (raw file):

Previously, MrMage (Daniel Alm) wrote…

I wonder whether we should implement these on the protocol instead, and only there?

Hmm, good point. It'd definitely reduce the amount of generated code. Let's do it.

---

Sources/protoc-gen-grpc-swift/Generator-Client.swift, line 292 at r1 (raw file):

Previously, MrMage (Daniel Alm) wrote…

Should we extract this into a separate file? (Might require increasing the visibility of some of the other methods, but that's acceptable for tooling code.)

I'm okay with where it currently is. Happy to move it if you feel strongly.

---

Sources/protoc-gen-grpc-swift/options.swift, line 88 at r1 (raw file):

Previously, MrMage (Daniel Alm) wrote…

super-nit: inconsistent use of self.

Unfortunately that's the case for all the generated code. I'm adding self explicitly when I modify things.

---

Tests/GRPCTests/TestClientExample.swift, line 38 at r1 (raw file):

Previously, MrMage (Daniel Alm) wrote…

How about adding a convenience initializer to makeGetResponseStream() with the desired response?

Thanks for the reminder! Yeah, let's do that.

I think it probably makes sense to just do the happy path for unary and streaming, i.e.:

• Unary: makeResponseStream(withResponse response: Response, requestHandler: (...) -> ()), and
• Streaming: makeResponseStream(withResponses responses: [Response], requestHandler: (...) -> ())

---

Tests/GRPCTests/TestClientExample.swift, line 42 at r1 (raw file):

What would happen if we called client.get a second time now?

A 'failed' response stream gets created; all promises on the call will be failed. (There's an example test for this.)

Also, is it possible to inspect the sent request? (If so, could you demo it?)

Yes: the response stream takes a request handler, there are tests demonstrating this as well.

Can we assert that all calls have been "consumed", i.e. all expected calls were sent?

Do you mean exposed as a feature to the user? If so then not at the moment. Do you think there's value in that?

---

Tests/GRPCTests/TestClientExample.swift, line 104 at r1 (raw file):

Previously, MrMage (Daniel Alm) wrote…

Could we also test this in an "interleaved" fashion?

This is done in some of the tests later on.

---

Tests/GRPCTests/TestClientExample.swift, line 121 at r1 (raw file):

Previously, MrMage (Daniel Alm) wrote…

Sorry, what exactly is the difference to the above? Also, why e.g. the sayHello method?

To demonstrate coding against the Echo_EchoClientProtocol this wasn't possible before #865

[Lukasa]

Nice, looks great.

[MrMage]

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @glbrntt and @MrMage)

---

Tests/GRPCTests/TestClientExample.swift, line 38 at r1 (raw file):

Previously, glbrntt (George Barnett) wrote…

Thanks for the reminder! Yeah, let's do that.

I think it probably makes sense to just do the happy path for unary and streaming, i.e.:

• Unary: makeResponseStream(withResponse response: Response, requestHandler: (...) -> ()), and
• Streaming: makeResponseStream(withResponses responses: [Response], requestHandler: (...) -> ())

Agreed. Users can still add extensions for error cases if they really need them.

---

Tests/GRPCTests/TestClientExample.swift, line 42 at r1 (raw file):

Do you mean exposed as a feature to the user? If so then not at the moment. Do you think there's value in that?

I think it might have sense to have that, to ensure that the client doesn't send more than it should.

---

Tests/GRPCTests/TestClientExample.swift, line 104 at r1 (raw file):

Previously, glbrntt (George Barnett) wrote…

This is done in some of the tests later on.

Sorry, in which one? I don't see any tests alternating between update.sendMessage and updateResponseStream.sendMessage.

---

Tests/GRPCTests/TestClientExample.swift, line 121 at r1 (raw file):

Previously, glbrntt (George Barnett) wrote…

To demonstrate coding against the Echo_EchoClientProtocol this wasn't possible before #865

Sorry, I still don't quite understand this. Is this to demonstrate the case of "Let's say we have an object XYZ that communicates with the EchoService via gRPC. We want to show how object XYZ could be tested, with its gRPC client mocked out."? If so, I would implement a client object or method independently of the test itself, because a "production code" client would not call XCTAssertEqual, for example.

Also, shouldn't it be in its own test class?

I might still be completely misunderstanding this; please let me know. We can also discuss this via Slack.

[glbrntt]

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @glbrntt and @MrMage)

---

Tests/GRPCTests/TestClientExample.swift, line 104 at r1 (raw file):

Previously, MrMage (Daniel Alm) wrote…

Sorry, in which one? I don't see any tests alternating between update.sendMessage and updateResponseStream.sendMessage.

Ah, you're right, I was thinking of interleaving the start of the RPC with sending responses

[glbrntt]

@MrMage this is the relevant commit to look at (the other is regenerating code): 50464e0

[MrMage]

Reviewed 15 of 15 files at r2.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @glbrntt and @MrMage)

---

Sources/Examples/Echo/Model/echo.grpc.swift, line 256 at r2 (raw file):

  /// Returns true if there are response streams enqueued for 'Update'
  public var hasUpdateResponses: Bool {

I wonder if we should add something like "Outstanding" to this name?

---

Tests/GRPCTests/TestClientExample.swift, line 306 at r2 (raw file):

    XCTAssertNoThrow(try updateResponseStream.sendMessage(.with { $0.text = "3" }))

    // And interleave with requests with responses.

nit: extra "with"

---

Tests/GRPCTests/TestClientExample.swift, line 309 at r2 (raw file):

    XCTAssertNoThrow(try update.sendMessage(.with { $0.text = "a" }).wait())
    XCTAssertNoThrow(try update.sendMessage(.with { $0.text = "b" }).wait())
    XCTAssertNoThrow(try update.sendMessage(.with { $0.text = "c" }).wait())

Could we check above this line that the response count is already = 2?

[MrMage]

Reviewed 3 of 3 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved