From 22183e5e6dd149bdc0e7e6a2b6f6dfb8d1404974 Mon Sep 17 00:00:00 2001 From: Roman Dodin Date: Wed, 3 Aug 2022 17:04:22 +0200 Subject: [PATCH 1/2] dummy1 --- api/clientset/v1alpha1/srlinux_test.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/api/clientset/v1alpha1/srlinux_test.go b/api/clientset/v1alpha1/srlinux_test.go index 4805720..2efa275 100644 --- a/api/clientset/v1alpha1/srlinux_test.go +++ b/api/clientset/v1alpha1/srlinux_test.go @@ -10,6 +10,7 @@ import ( "reflect" "testing" + "github.com/google/go-cmp/cmp" "github.com/h-fam/errdiff" "github.com/kr/pretty" srlinuxv1 "github.com/srl-labs/srl-controller/api/types/v1alpha1" @@ -83,6 +84,7 @@ func setUp(t *testing.T) (*Clientset, *restfake.RESTClient) { objs := []runtime.Object{obj1, obj2} cs.restClient = fakeClient + f := dynamicfake.NewSimpleDynamicClient(scheme.Scheme, objs...) f.PrependReactor("get", "*", func(action ktest.Action) (bool, runtime.Object, error) { gAction := action.(ktest.GetAction) @@ -156,16 +158,19 @@ func TestCreate(t *testing.T) { t.Run(tt.desc, func(t *testing.T) { tc := cs.Srlinux("foo") got, err := tc.Create(context.Background(), tt.want) + if s := errdiff.Substring(err, tt.wantErr); s != "" { t.Fatalf("unexpected error: %s", s) } + if tt.wantErr != "" { return } - want := tt.want.DeepCopy() - want.TypeMeta = metav1.TypeMeta{} - if !reflect.DeepEqual(got, want) { - t.Fatalf("Create(%+v) failed: diff\n%s", tt.want, pretty.Diff(got, want)) + + // want := tt.want.DeepCopy() + // want.TypeMeta = metav1.TypeMeta{} + if !cmp.Equal(got, tt.want) { + t.Fatalf("Create (got %+v) failed (want %+v): diff\n%s", got, tt.want, pretty.Diff(got, tt.want)) } }) } From db8c3f8e2bd629f04516d0d2d5e4e5c85335690e Mon Sep 17 00:00:00 2001 From: Roman Dodin Date: Thu, 4 Aug 2022 14:34:54 +0200 Subject: [PATCH 2/2] added comments and use cmp.Diff --- api/clientset/v1alpha1/srlinux_test.go | 58 +++++++++++++++----------- go.mod | 2 - go.sum | 1 - 3 files changed, 33 insertions(+), 28 deletions(-) diff --git a/api/clientset/v1alpha1/srlinux_test.go b/api/clientset/v1alpha1/srlinux_test.go index 2efa275..adfce38 100644 --- a/api/clientset/v1alpha1/srlinux_test.go +++ b/api/clientset/v1alpha1/srlinux_test.go @@ -7,12 +7,11 @@ import ( "fmt" "io" "net/http" - "reflect" "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/h-fam/errdiff" - "github.com/kr/pretty" srlinuxv1 "github.com/srl-labs/srl-controller/api/types/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -64,8 +63,14 @@ var ( Version: "2", }, } + + // ignoreTypeMetaOpt is cmpopt option that is used to discard TypeMeta field + // when comparing Srlinux structs. This is required since fakeRest server will never populate + // those fields, and those fields may be present in the test's want object. + ignoreTypeMetaOpt = cmpopts.IgnoreFields(srlinuxv1.Srlinux{}, "TypeMeta") ) +// setUp creates a Srlinux clientset and patches its rest and dynamic clients. func setUp(t *testing.T) (*Clientset, *restfake.RESTClient) { t.Helper() @@ -82,6 +87,8 @@ func setUp(t *testing.T) (*Clientset, *restfake.RESTClient) { t.Fatalf("NewForConfig() failed: %v", err) } + // objects will be added to the object tracker when the fake dynamic interface is created, + // this allows to unit test methods that require processing of unstructured data. objs := []runtime.Object{obj1, obj2} cs.restClient = fakeClient @@ -143,13 +150,18 @@ func TestCreate(t *testing.T) { }} for _, tt := range tests { - fakeClient.Err = nil + fakeClient.Err = nil // set Err to nil on each case start + // if we expect an error to be returned from the rest server + // we set the fake rest client error to it + // thus it will be returned for every rest call to that server. if tt.wantErr != "" { fakeClient.Err = fmt.Errorf(tt.wantErr) } fakeClient.Resp = tt.resp + // rest unit tests keep no state on the server side + // instead, whatever we want to be returned we populate as a resp.Body if tt.want != nil { b, _ := json.Marshal(tt.want) tt.resp.Body = io.NopCloser(bytes.NewReader(b)) @@ -167,10 +179,8 @@ func TestCreate(t *testing.T) { return } - // want := tt.want.DeepCopy() - // want.TypeMeta = metav1.TypeMeta{} - if !cmp.Equal(got, tt.want) { - t.Fatalf("Create (got %+v) failed (want %+v): diff\n%s", got, tt.want, pretty.Diff(got, tt.want)) + if s := cmp.Diff(got, tt.want, ignoreTypeMetaOpt); s != "" { + t.Fatalf("Create failed.\nGot: %+v\nWant: %+v\nDiff\n%s", got, tt.want, s) } }) } @@ -222,8 +232,8 @@ func TestList(t *testing.T) { return } - if !reflect.DeepEqual(got, tt.want) { - t.Fatalf("List() failed: got %v, want %v", got, tt.want) + if s := cmp.Diff(got, tt.want, ignoreTypeMetaOpt); s != "" { + t.Fatalf("List failed.\nGot: %+v\nWant: %+v\nDiff\n%s", got, tt.want, s) } }) } @@ -273,10 +283,8 @@ func TestGet(t *testing.T) { return } - want := tt.want.DeepCopy() - want.TypeMeta = metav1.TypeMeta{} - if !reflect.DeepEqual(got, want) { - t.Fatalf("Get() failed: got %v, want %v", got, want) + if s := cmp.Diff(got, tt.want, ignoreTypeMetaOpt); s != "" { + t.Fatalf("Get failed.\nGot: %+v\nWant: %+v\nDiff\n%s", got, tt.want, s) } }) } @@ -309,10 +317,12 @@ func TestDelete(t *testing.T) { t.Run(tt.desc, func(t *testing.T) { tc := cs.Srlinux("foo") + err := tc.Delete(context.Background(), "obj1", metav1.DeleteOptions{}) if s := errdiff.Substring(err, tt.wantErr); s != "" { t.Fatalf("unexpected error: %s", s) } + if tt.wantErr != "" { return } @@ -357,9 +367,9 @@ func TestWatch(t *testing.T) { return } - e := <-w.ResultChan() - if !reflect.DeepEqual(e, tt.want) { - t.Fatalf("Watch() failed: got %v, want %v", e, tt.want) + got := <-w.ResultChan() + if s := cmp.Diff(got, tt.want); s != "" { + t.Fatalf("Watch failed.\nGot: %+v\nWant: %+v\nDiff\n%s", got, tt.want, s) } }) } @@ -377,11 +387,11 @@ func TestUnstructured(t *testing.T) { in: "missingObj", wantErr: `"missingObj" not found`, }, { - desc: "Valid Node", + desc: "Valid Node 1", in: obj1.GetObjectMeta().GetName(), want: obj1, }, { - desc: "Valid Node", + desc: "Valid Node 2", in: obj2.GetObjectMeta().GetName(), want: obj2, }} @@ -403,8 +413,8 @@ func TestUnstructured(t *testing.T) { t.Fatalf("failed to turn response into a topology: %v", err) } - if !reflect.DeepEqual(uObj1, tt.want) { - t.Fatalf("Unstructured(%q) failed: got %+v, want %+v", tt.in, uObj1, tt.want) + if s := cmp.Diff(uObj1, tt.want); s != "" { + t.Fatalf("Unstructured (%q) failed.\nGot: %+v\nWant: %+v\nDiff\n%s", tt.in, uObj1, tt.want, s) } }) } @@ -431,7 +441,7 @@ func TestUpdate(t *testing.T) { }, wantErr: "doesnotexist", }, { - desc: "Valid Topology", + desc: "Valid Node", want: obj1, }} @@ -458,10 +468,8 @@ func TestUpdate(t *testing.T) { return } - want := tt.want.DeepCopy() - want.TypeMeta = metav1.TypeMeta{} - if !reflect.DeepEqual(got, updateObj) { - t.Fatalf("Update() failed: got %+v, want %+v", got, want) + if s := cmp.Diff(got, updateObj); s != "" { + t.Fatalf("Update failed.\nGot: %+v\nWant: %+v\nDiff\n%s", got, updateObj, s) } }) } diff --git a/go.mod b/go.mod index ce67c78..f28ab02 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,6 @@ require ( github.com/go-logr/logr v0.4.0 github.com/google/go-cmp v0.5.8 github.com/h-fam/errdiff v1.0.2 - github.com/kr/pretty v0.2.1 github.com/onsi/ginkgo v1.16.4 github.com/onsi/gomega v1.13.0 github.com/openconfig/kne v0.1.1 @@ -41,7 +40,6 @@ require ( github.com/hashicorp/golang-lru v0.5.4 // indirect github.com/imdario/mergo v0.3.12 // indirect github.com/json-iterator/go v1.1.11 // indirect - github.com/kr/text v0.2.0 // indirect github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect github.com/moby/spdystream v0.2.0 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect diff --git a/go.sum b/go.sum index acc610e..4d8f32e 100644 --- a/go.sum +++ b/go.sum @@ -487,7 +487,6 @@ github.com/konsorten/go-windows-terminal-sequences v1.0.3/go.mod h1:T0+1ngSBFLxv github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= -github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI= github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/pty v1.1.5/go.mod h1:9r2w37qlBe7rQ6e1fg1S/9xpWHSnaqNdHD3WcMdbPDA=