marshal: Add an error when UUID has wrong length #890
Merged
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
What === Add explicit expectations to exactly what error is returned in test cases where the marshal and unmarshal results in an error. Why === In cases where an unmarshal fails the presence of an error was expected but not what error was returned. Marshal is not tested for the cases where it fails in the table tests. Both of these are needed to add more tests that test for new errors being returned.
What === Add a descriptive error when a UUID is attempted to be marshaled or unmarshaled and the length of the []byte is not exactly 16 bytes long. Example === ``` can not marshal []byte 6 bytes long into timeuuid, must be exactly 16 bytes long ``` Why === In cases where UUID []byte was not the correct length the generic `can not marshal %T into %s` error was being returned. The error was misleading because it gave the impression the root cause was the type of the data and not it's length. Additionally the `%T` formatter for a `[]byte` prints out `[]uint8` which was even more confusing.
What === Add `Leigh McCulloch` to AUTHORS. Why === As part of his first contribution.
leighmcculloch
commented
Apr 9, 2017
| }, | ||
| { | ||
| NativeType{proto: 2, typ: TypeVarint}, | ||
| []byte(nil), | ||
| nil, | ||
| nil, | ||
| UnmarshalError("can not unmarshal into non-pointer <nil>"), |
There was a problem hiding this comment.
@Zariel: 👋 Aside to the main issue being addressed in this PR it looks like the Marshal and Unmarshal functions are not entire symmetrical. The []byte here can be marshaled but not unmarshaled.
There was a problem hiding this comment.
Actually, I guess the asymmetry here doesn't matter, since you wouldn't Unmarshal into nil.
|
Thanks very much for the contribution! |
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.
What
Add a descriptive error when a UUID is marshaled or unmarshaled and the length of the
[]byteis not exactly 16 bytes long.Example
Why
In cases where UUID
[]bytewas not the correct length the genericcan not marshal %T into %serror was being returned. The error was misleading because it gave the impression the root cause was the type of the data and not it's length. Additionally the%Tformatter for a[]byteprints out[]uint8which was even more confusing.Notes
This PR contains three commits. It's easier to digest the change if you review them separately as the first refactors the tests to support expectations on the error itself.
Ref
Fixes #868