Skip to content

Conversation

@smaye81
Copy link
Member

@smaye81 smaye81 commented Sep 12, 2023

This adds some improvements and updates to the Connect Conformance Tests. Namely:

  • Use the proto definitions in the Conformance repo rather than house local files.
  • Add an additional test that validates errors after response. The conformance implementation for failServerStreaming changed here to inspect response parameters and return that amount of responses before failing so that users could test trailers-only responses. This fixes the current test so that it doesn't send any response parameters and expects an error straightaway. In addition, it adds another test to test that some messages are received before the error.

Note that this updates Connect-Swift to use commit 6ce5b0, but this is not the current commit. This is because all commits afterwards cause two additional tests to fail (failUnary and failServerStreaming) due to issues with parsing ErrorDetails. Those will be fixed in a follow-up PR.

Steve Ayers added 2 commits September 12, 2023 11:20
try await self.executeTestWithClients { client in
let expectedErrorDetail = Grpc_Testing_ErrorDetail.with { proto in
proto.reason = "soirée 🎉"
proto.domain = "connect-crosstest"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is yeah. This didn't change until the repo was moved. This updates to the commit right before that (follow-up PR will address this).

try self.executeTestWithClients { client in
let expectedErrorDetail = Grpc_Testing_ErrorDetail.with { proto in
proto.reason = "soirée 🎉"
proto.domain = "connect-crosstest"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, see above.

smaye81 and others added 2 commits September 12, 2023 14:53
…ce.swift

Co-authored-by: Michael Rebello <me@michaelrebello.com>
@smaye81 smaye81 merged commit 92b3ce9 into main Sep 12, 2023
@smaye81 smaye81 deleted the sayers/fix_streaming_error_after_response branch September 12, 2023 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants