-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add tests for connect error detail debug data #921
Add tests for connect error detail debug data #921
Conversation
Signed-off-by: Timo Stamm <ts@timostamm.de>
Signed-off-by: Timo Stamm <ts@timostamm.de>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe worth also adding cases where the debug
attribute is completely absent?
Signed-off-by: Timo Stamm <ts@timostamm.de>
Good call. Added in f028979. |
@@ -107,6 +107,73 @@ testCases: | |||
error: | |||
code: CODE_OUT_OF_RANGE | |||
message: oops | |||
- request: | |||
# Clients must not depend on data in the "debug" key when deserializing details. | |||
testName: error/disregard-detail-debug |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the test cases are typically named in a way that describe the scenario, vs describing the implementation's expected behavior. How about invalid-details-debug
instead of disregard-detail-debug
? (Sorry I didn't mention this with previous remark.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 There are a couple of existing tests named "allow-unrecognized...". Doesn't seem worth renaming them, but renamed the new ones in 7a9e8c2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thanks for calling those out. I can sort those out in a separate PR.
Signed-off-by: Timo Stamm <ts@timostamm.de>
Connect clients must not depend on data in the "debug" key when deserializing details from an Error or EndStreamResponse.
This PR adds a test where the "debug" key contains data that does not match the error detail, and is not a JSON object (see clarification regarding the property type here here).