-
Notifications
You must be signed in to change notification settings - Fork 613
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
Instance attributes #908
Instance attributes #908
Conversation
agent/utils/utils_test.go
Outdated
if result != specifiedValue { | ||
t.Error("Expected " + specifiedValue + ", got " + result) | ||
} | ||
assert.Equal(t, result, specifiedValue, "Expected "+specifiedValue+", got "+result) |
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.
You don't need the "Expected "+specifiedValue+", got "+result
, since assert.Equal
will do that for you.
Also, it should be assert.Equal(t, expected, actual, "message")
, so you'll want to swap the order.
agent/utils/utils_test.go
Outdated
|
||
for _, tc := range testCases { | ||
t.Run(tc.name, func(t *testing.T) { | ||
assert.Equal(t, ZeroOrNil(tc.param), tc.expected) |
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.
assert.Equal(t, tc.expected, ZeroOrNil(tc.param), tc.name)
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.
tc.name will be included in the subtest output -- still want it in the message too?
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.
Doesn't hurt to be there. In the subtest output it'll replace some characters (
will become _
, for example), but the message can have the actual string.
fdd425d
to
ace3d5d
Compare
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.
lgtm. tables all the way!
agent/utils/utils.go
Outdated
@@ -44,7 +44,8 @@ func ZeroOrNil(obj interface{}) bool { | |||
if obj == nil { | |||
return true | |||
} | |||
if value.Kind() == reflect.Slice || value.Kind() == reflect.Array { | |||
switch value.Kind() { | |||
case reflect.Slice, reflect.Array, reflect.Map: |
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.
this is neat. =]
agent/utils/utils_test.go
Outdated
{map[string]string{"foo": "bar"}, false, "map[string]string{foo:bar} is not zero or nil"}, | ||
} | ||
|
||
for _, tc := range testCases { |
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.
ohhh i see what's going on here, this is pretty cool. i need more tables in my life.
ace3d5d
to
4a6cda3
Compare
Summary
Refactored / cleaned up tests. This builds on top of #893
Implementation details
Testing
make release
)go build -out amazon-ecs-agent.exe ./agent
)make test
) passgo test -timeout=25s ./agent/...
) passmake run-integ-tests
) pass.\scripts\run-integ-tests.ps1
) passmake run-functional-tests
) pass.\scripts\run-functional-tests.ps1
) passThis contribution is under the terms of the Apache 2.0 License: yes