Implement unimplemented_method test #846
Conversation
|
Fix #706 |
| try { | ||
| stub.unimplementedCall(Empty.getDefaultInstance()); | ||
| } catch (Throwable t) { | ||
| assertEquals(Status.UNIMPLEMENTED, Status.fromThrowable(t)); |
There was a problem hiding this comment.
Per the spec, we should check the code and message.
assertEquals(Status.UNIMPLEMENTED.getCode(), Status.fromThrowable(t).getCode());
assertNull(Status.fromThrowable(t).getDescription());
I think the second check will fail with the current implementation, so we should either:
- update the server side code to not append the description, or
- remove this overstrict limit from the Spec and don't do such check.
@ejona86, what do you think?
There was a problem hiding this comment.
We could maybe do both. It does seem the test case is overly-strict. I know if we get a 404 from a server the client will say it came from an HTTP 404. We can talk to @a11r to determine why it requires the description to be null.
There was a problem hiding this comment.
Talked to @a11r, he would like to stick with no description if it does not add additional information.
Removed server side description.
|
PTAL |
|
@zsurocking, LGTM |
|
|
||
| /** Sends an rpc to an unimplemented method on the server. **/ | ||
| @Test(timeout = 10000) | ||
| /** Sends an rpc to an unimplemented method on the server. **/ |
|
@zsurocking LGTM |
|
Thank you for your pull request. Before we can look at your contribution, we need to ensure all contributors are covered by a Contributor License Agreement. After the following items are addressed, please respond with a new comment here, and the automated system will re-verify.
Regards, |
No description provided.