Skip to content
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

Update test protos for PingStream method #140

Merged
merged 3 commits into from
Nov 1, 2023
Merged

Update test protos for PingStream method #140

merged 3 commits into from
Nov 1, 2023

Conversation

emcfarlane
Copy link
Contributor

Test protos were using CumSum as a Ping method that returned twice and all other methods were unused. Regenerate the PingService with a PingStream methods that returns a full duplex method of Ping. Size is removed from the constant 2 to be calculated with proto.Size() (response sizes are the same as request, so response isn't calculated).

Testing changes:

  • CumSum == PingStream but always returns a response for a request, not two responses for each request
  • failPing returns an error, not a message than an error.

No behaviour should be changed.

@emcfarlane emcfarlane self-assigned this Oct 26, 2023
Copy link
Member

@akshayjshah akshayjshah left a comment

Choose a reason for hiding this comment

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

Left some questions and suggestions, but they're optional. You're welcome to address the comments by pushing code here and then merging it (no more review needed), in follow-up PRs, or by sending my thoughts directly to /dev/null :)

bench_test.go Outdated Show resolved Hide resolved
interceptor_test.go Show resolved Hide resolved
interceptor_test.go Show resolved Hide resolved
interceptor_test.go Show resolved Hide resolved
pingserver_test.go Show resolved Hide resolved
Base automatically changed from ed/bench to main November 1, 2023 19:13
@akshayjshah akshayjshah merged commit 61d17dd into main Nov 1, 2023
5 checks passed
@akshayjshah akshayjshah deleted the ed/protos branch November 1, 2023 20:27
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.

None yet

2 participants