Add tests to watch.go #151
Conversation
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
added
sig/api-machinery
This commit adds unit tests for watch.go As of now part of tests are implemented, I'll add additional tests.
atoato88
force-pushed
the
add-unit-test-watch-go
branch
from
7814ba1 to
27795a3
Compare
k8s-ci-robot
removed
the
needs-rebase
|
/retitle Add tests to watch.go I'd like to add more tests, but it's difficult on unit test. It may need k8s cluster and it's not unit test, I think. |
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
|
/assign @justinsb |
|
/assign @pwittrock |
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| actualSchema := uniqueGroupVersionKind(tt.inputObjects) | ||
| assert.Equal(t, tt.expectedSchema, actualSchema) |
There was a problem hiding this comment.
FYI, the google go style guide says to avoid assertion libraries, and to use e.g.
if diff := cmp.Diff(got, want); diff != "" {
t.Errorf("unexpected result from watch; diff %s", diff)
}
(or something along those lines)
|
Thanks @atoato88 . A suggestion to avoid assertion libraries, but that's more based on my code style guide from work, and I don't know that kubernetes has made the same statement. /approve |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: atoato88, justinsb The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
/retest Test failure was unrelated, should have been fixed by #155 |
This PR adds unit tests for
watch.goAs of now part of tests are implemented, I'll add additional tests.
related to: #150