add unit test for DeletePVC #517
Conversation
syamgk
added
do-not-merge/work-in-progress
pkg/occlient/occlient_test.go
Outdated
| if (len(fakeClientSet.Kubernetes.Actions()) != 1) && (tt.wantErr != true) { | ||
| t.Errorf("expected 1 action in DeletePVC got: %v", fakeClientSet.Kubernetes.Actions()) | ||
| } | ||
| }) |
There was a problem hiding this comment.
Just check if fakeClientSet.Kubernetes.Actions()[0].(ktesting.DeleteAction).GetName() returns name
There was a problem hiding this comment.
kadel
added
the
needs-rebase
pkg/occlient/occlient_test.go
Outdated
| tests := []struct { | ||
| name string | ||
| pvcname string | ||
| wantErr bool |
There was a problem hiding this comment.
One more use for failure as well
There was a problem hiding this comment.
Hi @surajnarwade ,
here in this test case, It is only validating whether the correct function is called with the correct parameter, So adding a failure case is not possible
Also according to the Unit test philosophy, we don't need to test the functionality of library function call eg: in this case c.kubeClient.CoreV1().PersistentVolumeClaims(c.namespace).Delete(name, nil)
It's out of our scope of testing because we are in an assumption that the library which we are using is tested by them, and we only need to validate the code which we have written.
This is my understanding so far
syamgk
force-pushed
the
deletePVC-utest
branch
5 times, most recently
from
81f9d1f to
4d97561
Compare
syamgk
removed
do-not-merge/hold
|
hi @syamgk, is this still work in progress? |
syamgk
removed
the
do-not-merge/work-in-progress
|
@kadel its not |
|
This LGTM! No comments regarding the code |
|
@syamgk LGTM |
kadel
added
the
needs-rebase
|
need rebase |
add unit test for DeletePVC
|
rebase done, removing label |
syamgk
removed
the
needs-rebase
add unit test for DeletePVC