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
Relaxes assertions on query parameters #771
Conversation
errs = append(errs, checkError(expected.Error, actual.Error)...) | ||
errs = append(errs, checkPayloads(expected.Payloads, actual.Payloads)...) | ||
|
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.
This was a slight UX improvement -- often, the reason a test case fails is because an unexpected error occurs. Previously when that happened, the test case would complain about response headers and trailers before getting to the "meat" of the problem: the error. So this reorders the checks so it is more likely the most relevant errors are reported first.
- No need to assert precise encoding of message parameter. - Also moves error and payload assertions first since they are usually most relevant (when present) to a test case failure.
582224a
to
c8322ad
Compare
cc @rebello95, this should allow you to remove the "known failing" file in connect-swift. This will be in an upcoming v1.0.0-rc3 of the conformance tests (along with some new test cases). |
Thanks @jhump! |
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.
We could probably remove MarshalStable
from codec.go now since all the usages were removed here.
Previously, computing the expected response would add an expected query parameter for the `message` param, with an encoded value. But the problem is that not all clients would necessarily encode the message in exactly the same way, and that's okay. Some will choose to base64-encode (which is usually more efficient with binary payloads). Also, there's no _canonical_ protobuf binary format, so there could be subtle differences in the byte output, even for equivalent messages. Finally, there's no _canonical_ encoding for compressed payloads -- different default compression settings or other subtle differences in implementation can lead to more than one valid compressed encoding of the same data.
Previously, this would add an expected query parameter for the
message
param, with an encoded value. But the problem is that not all clients would necessarily encode the message in exactly the same way, and that's fine. Some will choose to base64-encode (which is usually more efficient with binary payloads). Also, there's no canonical protobuf binary format, so there could be subtle differences in the byte output, even for equivalent messages. Finally, there's no canonical encoding for compressed payloads -- different default compression settings or other subtle differences in implementation can lead to more than one valid compressed encoding of the same data.