fix(manifest): render http health check path correctly #1686
Conversation
| @@ -154,6 +154,7 @@ func TestNewLoadBalancedWebService_UnmarshalYaml(t *testing.T) { | |||
| Interval: durationp(78 * time.Second), | |||
| Timeout: durationp(9 * time.Second), | |||
| }, | |||
| HealthCheckPath: nil, | |||
There was a problem hiding this comment.
can we update the name of this test case to reflect this change? for example:
"should use custom healthcheck configuration when provided and set default path to nil"
| @@ -149,7 +149,8 @@ func (b *BuildArgsOrString) UnmarshalYAML(unmarshal func(interface{}) error) err | |||
| } | |||
|
|
|||
| if !b.BuildArgs.isEmpty() { | |||
| // Unmarshaled successfully to b.BuildArgs, return. | |||
| // Unmarshaled successfully to b.BuildArgs, unset b.BuildString, and return. | |||
| b.BuildString = nil | |||
There was a problem hiding this comment.
does this UnmarshalYAML have unit tests? can we do something similar to above to make sure that the buildstring gets reset to nil if a custom build field is provided
There was a problem hiding this comment.
it is tested here https://github.com/aws/copilot-cli/pull/1686/files#diff-c4a6f6a13adfd37f7bd3fbc2d6d56fe5d1f2a6e752f6d795628adca336bf97e3R157. since newDefaultLoadBalancedWebService has default to be "/" and it is set back to nil
There was a problem hiding this comment.
wut am I misunderstanding something? that one is for UnmarshalYAML for healthchecks whereas this change is for the image.build field
There was a problem hiding this comment.
oooh sorry i misread it.
iamhopaul123
force-pushed
the
quickfix
branch
from
40eaa73
to
79b778d
Compare
fixes #1685
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.