Skip to content
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

chore(tests): Add json.Unmarshal test with empty value cases #116

Merged
merged 4 commits into from
Aug 21, 2023

Conversation

wawan93
Copy link
Contributor

@wawan93 wawan93 commented Apr 24, 2023

I've added some test cases in JSON for documentation purposes.
Only "00000000-0000-0000-0000-000000000000" and null values are parsed without errors. Empty strings (even with omitempty tag) are parsed with the error invalid UUID length.

@google-cla
Copy link

google-cla bot commented Apr 24, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@wawan93 wawan93 changed the title Add json.Unmarshal test with empty cases Add json.Unmarshal test with empty value cases Apr 24, 2023
@noahdietz noahdietz requested a review from a team as a code owner August 18, 2023 02:11
@noahdietz noahdietz changed the title Add json.Unmarshal test with empty value cases chore(tests): Add json.Unmarshal test with empty value cases Aug 21, 2023
@noahdietz
Copy link
Collaborator

noahdietz commented Aug 21, 2023

Empty strings (even with omitempty tag) are parsed with the error invalid UUID length.

I'm new to this project but @bormanp is this ideal? My gut says that deserializing a JSON blob that omits a UUID field into a struct that states omitempty for that same UUID field shouldn't produce an error and instead should just be Nil. If we fix this, we fix it in a separate PR of course. Even without omitempty I don't think it should be an error, just Nil.

Edit: I guess omitempty is for serialization not deserialization, so it's not a distinction for Unmarshal.

json_test.go Outdated Show resolved Hide resolved
@noahdietz noahdietz added the automerge Summons the Merge-on-Green bot label Aug 21, 2023
@noahdietz noahdietz merged commit 06716f6 into google:master Aug 21, 2023
6 checks passed
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Summons the Merge-on-Green bot label Aug 21, 2023
tkarrass pushed a commit to tkarrass/uuid that referenced this pull request Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants