BED-4908: Add unit tests #901
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
| endpoint := "/api/v2/collectors/%s" | ||
|
|
||
| t.Run("sharphound", func(t *testing.T) { | ||
| req, err := http.NewRequestWithContext(context.Background(), "GET", fmt.Sprintf(endpoint, "sharphound"), nil) |
There was a problem hiding this comment.
Nit: we can replace the "GET" with http.MethodGet
cmd/api/src/api/v2/version_test.go
Outdated
|
|
||
| endpoint := "/api/version" | ||
| req, err := http.NewRequestWithContext(context.Background(), "GET", endpoint, nil) | ||
| require.Nil(t, err) |
There was a problem hiding this comment.
This can be require.NoError
cmd/api/src/api/utils_test.go
Outdated
| } | ||
|
|
||
| func filter(testStruct TestStruct) bool { | ||
| return testStruct.val < 2 |
There was a problem hiding this comment.
This is clean, but I'd maybe move these definitions to within the test itself.
Not that it looks like we'll be adding more tests here, but to me it makes sense to have this defined in the relative test since it hard defines a requirement (testStruct.val < 2)
type testStruct struct {
val int
}
result := FilterStructSlice(structs, func(s testStruct) bool {
return s.val < 2
})
cmd/api/src/api/utils_test.go
Outdated
| func TestParseOptionalBoolMisspelledValue(t *testing.T) { | ||
| result, err := ParseOptionalBool("trueee", false) | ||
| require.Equal(t, false, result) | ||
| require.Error(t, err) |
There was a problem hiding this comment.
Convention would be to check the error first and then assert that the values are correct.
require.Error(t, err)
assert.Equal(t, false, result)It doesn't make that much of a difference with this test, but it comes in handy with things such as:
resultSlice, err := SortTest()
require.NoError(t, err)
require.Len(t, resultSlice 2)
assert.Equal(t, "hello", resultSlice[0])
assert.Equal(t, "world", resultSlice[1])In this scenario, we require err to be nil and require the length of resultSlice to be 2 in order to then assert the individual values of the slice.
This would allow us to run the unit test and get all the potential problems from within the array in one go.
If the test cannot run on a failure condition: require
If the test can run with the failure condition: assert
| require.Equal(t, http.StatusOK, response.Code) | ||
| }) | ||
|
|
||
| t.Run("invalid", func(t *testing.T) { |
There was a problem hiding this comment.
We should also test the 500 status error.
You can do this by defining a new resources object in a new test
var (
resources = v2.Resources{
CollectorManifests: map[string]config.CollectorManifest{},
}
)|
I have read the CLA Document and I hereby sign the CLA |
Add test case to collectors test
Description
Add a few missing unit tests
Motivation and Context
BED-4908
Why is this change required? What problem does it solve?
How Has This Been Tested?
New units tests run successfully
Screenshots (optional):
Types of changes
Checklist: