Update test cases for consistency, backfill missing cancel tests #874
Conversation
This file contains 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
…x up issues; some other minor improvements Signed-off-by: Josh Humphries <2035234+jhump@users.noreply.github.com>
Signed-off-by: Josh Humphries <2035234+jhump@users.noreply.github.com>
…known-failing in grpcwebclient Signed-off-by: Josh Humphries <2035234+jhump@users.noreply.github.com>
jhump
force-pushed
the
jh/tweaking-test-cases
branch
from
6c48499
to
b9f8a89
Compare
Signed-off-by: Josh Humphries <2035234+jhump@users.noreply.github.com>
jhump
force-pushed
the
jh/tweaking-test-cases
branch
from
26abd1c
to
bd780b4
Compare
smaye81
approved these changes
May 23, 2024
…enceclient and grpcclient with the pseudo-code in docs Signed-off-by: Josh Humphries <2035234+jhump@users.noreply.github.com>
jhump
force-pushed
the
jh/tweaking-test-cases
branch
from
36c5085
to
20737b7
Compare
Signed-off-by: Josh Humphries <2035234+jhump@users.noreply.github.com>
jhump
force-pushed
the
jh/tweaking-test-cases
branch
from
1310eea
to
92fbfcd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
After noticing some naming inconsistencies, I added a test that verifies that the embedded test files all conform to the naming conventions described in the docs.
That found a few issues, so this PR fixes them all.
I noticed and changed a few other things while doing this other clean-up in the tests:
cancel-after-zero-responsestest cases are not actually different than thecancel-after-close-send. While the contents of the test case are slightly different, the behavior they induce in the client is the same, which is to cancel the RPC right after the request stream is closed. It seemed odd that we'd have a test for cancelling "after" receiving zero responses. That's really like a cancel before the first response is received, and that felt awkward to implement since ideally you simply check that number after receiving a response (per the name of the field). So I opted to remove those and keep thecancel-after-close-sendversion.cancel-after-zero-responses, and nocancel-after-close-sendversion. So I "fixed" that -- to usecancel-after-sendfor consistency with the others -- only to realize that the reference clients did not actually implement it. That's when I also realized that there was no test for cancelling unary RPCs. (The reference clients also did not implement cancellation in the unary RPC flow.)The biggest changes in here were to get all of the client implementations to handle the
cancel.after_close_send_msfield in the input.