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

Fixing failing unit test causing CI pipeline to fail #2494

Merged
merged 3 commits into from
Feb 14, 2024

Conversation

Adam724
Copy link
Contributor

@Adam724 Adam724 commented Feb 14, 2024

This PR introduced a new option to allow optional struct fields to be set to omitempty. However, it does not properly evaluate the boolean condition, and always behaves as if the option is set to true. This is causing some tests to fail unexpectedly when fields are set to null. The generated code contains the omitempty flag erroneously and thus ignores the null field when generating JSON output, causing the input/output JSON not to match. This PR fixes #2462.

@Adam724 Adam724 changed the title Fixing failing unit test causing CI pipline to fail Fixing failing unit test causing CI pipeline to fail Feb 14, 2024
@dvdsgl
Copy link
Member

dvdsgl commented Feb 14, 2024

THANK YOU!

@dvdsgl dvdsgl merged commit fa751ab into glideapps:master Feb 14, 2024
24 checks passed
@Adam724
Copy link
Contributor Author

Adam724 commented Feb 14, 2024

Glad it is working! If you could take a look at this PR when you get a chance, that'd be great. Realized while testing a bit more today that the logic for omitEmpty is not right. It needs to still check whether the field is optional first before adding the tag. Ideally there would be a test case to catch this, but doesn't look like the current capabilities support passing CLI options to tests. I am looking into adding this, but will include it in a different PR since it's more involved.

@Adam724 Adam724 deleted the fix-golang-unit-tests branch February 14, 2024 22:23
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.

golang test failure on test/inputs/json/misc/734ad.json
2 participants