-
Notifications
You must be signed in to change notification settings - Fork 21
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
fix(condition): handling of missing source and/or target keys #277
Conversation
condition/number_equal_to_test.go
Outdated
@@ -134,6 +134,70 @@ var numberEqualToTests = []struct { | |||
[]byte(`{"foo": 100, "bar": 200}`), | |||
false, | |||
}, | |||
{ | |||
"pass", |
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.
Any tests that return false should be labeled as "fail" ("pass" is for tests that return true).
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.
gotcha, thanks for pointing that out! fixed test cases to reflect expected 'fail' outcomes.
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 will need to be more work on this prior to the next major update. I think we can do this in more places, but not everywhere without introducing a breaking change.
Description
Adds additional handling of missing source and/or target keys in condition package number equal to and string equal to.
Motivation and Context
Fix for issue:Condition Package Zero Value Matches Can Be Inaccurate #263.
Current behaviour of condition package does not contain source key or target key's object path existence check. This caused unexpected behaviour:
When only src key is present but the path doesn't exist, condition package inaccurately matches on zero value and empty string.
When both source key and target key are present but both paths do not exist, the number equal to and string equal to return true.
This adds checks thus
How Has This Been Tested?
Types of changes
Checklist: