chore(grpc-mock): restructure and add mock grpc tests #1579
Conversation
ethanoroshiba
requested review from
a team,
joroshiba,
SuperFluffy and
noot
as code owners
| .await | ||
| .unwrap_err(); | ||
|
|
||
| let rsp = client |
There was a problem hiding this comment.
I'd suggest doing let start = Instant::now(); just before this, and after the .await.unwrap() below, you can assert that start.elapsed() >= const delay.
There was a problem hiding this comment.
Same comment as above, but happy to put it in still if needed
There was a problem hiding this comment.
I'd prefer to not change this file because it's taken from a well known service.
There was a problem hiding this comment.
Reverted in 4b15aa0, but needed a type with more fields to adequately test message_partial_pbjson so I added a new struct MockMessage to test_utils. Let me know if this works for you!
There was a problem hiding this comment.
I didn't mean for you to implement this manually (although I am sure it was a good exercise to learn about prost and manual serde impls).
What if you you copy the echo server over (from here: https://github.com/hyperium/tonic/blob/master/examples/proto/echo/echo.proto) and just generate extra code?
That way we don't have to worry about potential breakage should we ever update prost or tonic.
There was a problem hiding this comment.
I didn't mean for you to implement this manually (although I am sure it was a good exercise to learn about prost and manual serde impls).
What if you you copy the echo server over (from here: https://github.com/hyperium/tonic/blob/master/examples/proto/echo/echo.proto) and just generate extra code?
That way we don't have to worry about potential breakage should we ever update prost or tonic.
| @@ -1,3 +1,4 @@ | |||
| // This file is @generated by prost-build. | |||
There was a problem hiding this comment.
Was this added by you or by prost?
There was a problem hiding this comment.
Added by prost, I think at some point I may have bumped the dep and this is a new thing that the builder does? If I remember correctly, I believe I've noticed it elsewhere as well.
|
@SuperFluffy in RE to the comment ^^above (since I can't reply directly), I'm not sure the echo server would be sufficient as is. To adequately test the |
Summary
Divided GRPC mock tests by component and added a few more.
Background
Some parts of the GRPC mock still weren't tested, and the crate also needed a test for the new delay capabilities.
Changes
mock,response, andmatcher.HealthCheckRequestso that we could more accurately testmatcher::partialpbjson.andmock combinator, matchers not matching incorrect requests,up_to_n_times,DynamicResponse, and delay.Testing
All tests passing.
Related Issues
closes #902
closes #1559