add some tests for util/k8sutil/erros.go #32
Conversation
pkg/util/k8sutil/errors_test.go
Outdated
| "k8s.io/apimachinery/pkg/util/validation/field" | ||
| ) | ||
|
|
||
| var conflict = apierrors.NewConflict(schema.GroupResource{"fist", "second"}, "something", os.ErrInvalid) |
There was a problem hiding this comment.
Group variable in single var block:
var(
conflict = ...
other = ...
)
| "k8s.io/apimachinery/pkg/runtime/schema" | ||
| "k8s.io/apimachinery/pkg/util/validation/field" | ||
| ) | ||
|
|
There was a problem hiding this comment.
Add tests for masked errors. (assert.X(t, maskAny(...)))
pkg/util/k8sutil/test/events_test.go
Outdated
| @@ -0,0 +1,64 @@ | |||
| package k8sutil_test | |||
There was a problem hiding this comment.
Is this package name ok?
There was a problem hiding this comment.
Where should I state the reason (cyclic import) that led to package creation?
There was a problem hiding this comment.
Package name should be test since the folder is named that.
There was a problem hiding this comment.
Add a doc.go with only a header, package test followed by a comment describing about the cycle import.
|
|
||
| func TestPodGoneEvent(t *testing.T) { | ||
| event := k8sutil.NewPodGoneEvent("name", "role", &apiObjectForTest) | ||
| assert.Equal(t, event.Reason, "Pod Of Role Gone") |
There was a problem hiding this comment.
role is uppercase in reason but not in message
There was a problem hiding this comment.
The use of strings.Title seems to be a bit inconsistent.
In the following snipped it is used in the middle of the string in one case and in the other case it is not:
62 // NewPodGoneEvent creates an event indicating that a pod is missing
63 func NewPodGoneEvent(podName, role string, apiObject APIObject) *v1.Event {
64 event := newDeploymentEvent(apiObject)
65 event.Type = v1.EventTypeNormal
66 event.Reason = fmt.Sprintf("Pod Of %s Gone", strings.Title(role))
67 event.Message = fmt.Sprintf("Pod %s of member %s is gone", podName, role)
There was a problem hiding this comment.
I'm following the etcd-operator on this
pkg/util/k8sutil/errors_test.go
Outdated
|
|
||
| func TestIsAlreadyExists(t *testing.T) { | ||
| assert.False(t, IsAlreadyExists(conflictError)) | ||
| assert.False(t, IsConflict(maskAny(invalidError))) |
There was a problem hiding this comment.
IsConflict -> IsAlreadyExists
pkg/util/k8sutil/errors_test.go
Outdated
|
|
||
| func TestIsNotFound(t *testing.T) { | ||
| assert.False(t, IsNotFound(invalidError)) | ||
| assert.False(t, IsConflict(maskAny(invalidError))) |
first ridiculous commits