-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add Unit Tests for Input Parsing #433
Conversation
Does the PR have any schema changes?Looking good! No breaking changes found. |
provider/image.go
Outdated
return reg | ||
} | ||
return reg | ||
} | ||
|
||
func setArgs(a resource.PropertyValue) map[string]*string { |
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.
Curious why these are called setX? They're pure functions that are doing some marshaling? The set prefix makes me expect they set something.
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.
replacing with marshalX
- thank you that is a good point!
provider/image_test.go
Outdated
input := resource.PropertyValue{ | ||
resource.NewPropertyMapFromMap(map[string]interface{}{}), | ||
} | ||
actual := marshalBuild(input) |
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.
Clearly from this test it looks like marshalBuild applies default values. This is a bit unexpected given the name "marshalBuild" the "marshal" part makes me expect something that just does 'boring' conversions between formats. Perhaps "marshalBuildAndApplyDefaults"?
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
This PR cleans up the input verification for Image and adds unit tests.
There are a few minor code changes mostly as a result of adding test cases.
Fixes #420