From 7bcf875998f15e47435beda9f158b3ff5178642d Mon Sep 17 00:00:00 2001 From: Britania Rodriguez Reyes <145056127+britaniar@users.noreply.github.com> Date: Mon, 6 Nov 2023 13:11:03 -0800 Subject: [PATCH 1/7] Add Negative unit tests --- pkg/controllers/rollout/controller_test.go | 69 ++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/pkg/controllers/rollout/controller_test.go b/pkg/controllers/rollout/controller_test.go index 9f92c1397..b9882dc10 100644 --- a/pkg/controllers/rollout/controller_test.go +++ b/pkg/controllers/rollout/controller_test.go @@ -12,6 +12,7 @@ import ( "testing" "time" + "github.com/crossplane/crossplane-runtime/pkg/test" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" @@ -226,6 +227,74 @@ func TestReconcilerUpdateBindings(t *testing.T) { wantErr bool }{ // TODO: Add negative test cases with fake client + "test update bindings with no bindings": { + name: "test3", + Client: &test.MockClient{ + MockUpdate: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + return nil + }, + }, + latestResourceSnapshotName: "snapshot-2", + toBeUpgradedBinding: []*fleetv1beta1.ClusterResourceBinding{}, + wantErr: false, + }, + + "test update binding with no latestResourceSnapshotName": { + name: "Empty latestResourceSnapshotName", + Client: &test.MockClient{ + MockUpdate: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + return nil + }, + }, + latestResourceSnapshotName: "", + toBeUpgradedBinding: []*fleetv1beta1.ClusterResourceBinding{ + generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, "snapshot-1", cluster1), + generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster2), + generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster3), + generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster4), + generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster5), + }, + wantErr: false, + }, + "test update binding with nil toBeUpgradedBinding": { + name: "Nil toBeUpgradedBinding", + Client: &test.MockClient{ + MockUpdate: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + return nil + }, + }, + latestResourceSnapshotName: "snapshot-2", + toBeUpgradedBinding: nil, + wantErr: false, + }, + "test update binding with empty toBeUpgradedBinding": { + name: "Empty toBeUpgradedBinding", + Client: &test.MockClient{ + MockUpdate: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + return nil + }, + }, + latestResourceSnapshotName: "snapshot-2", + toBeUpgradedBinding: []*fleetv1beta1.ClusterResourceBinding{}, + wantErr: false, + }, + "test update binding with failed binding": { + name: "Failed Binding", + Client: &test.MockClient{ + MockUpdate: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + return errors.New("Failed to update binding") + }, + }, + latestResourceSnapshotName: "snapshot1", + toBeUpgradedBinding: []*fleetv1beta1.ClusterResourceBinding{ + generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, "snapshot-1", cluster1), + generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster2), + generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster3), + generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster4), + generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster5), + }, + wantErr: true, + }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { From 28225b64d73bcd24b3cf122db41859045e63e608 Mon Sep 17 00:00:00 2001 From: Britania Rodriguez Reyes <145056127+britaniar@users.noreply.github.com> Date: Mon, 6 Nov 2023 13:29:54 -0800 Subject: [PATCH 2/7] Fix formatting --- pkg/controllers/rollout/controller_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/controllers/rollout/controller_test.go b/pkg/controllers/rollout/controller_test.go index b9882dc10..a22f1a37d 100644 --- a/pkg/controllers/rollout/controller_test.go +++ b/pkg/controllers/rollout/controller_test.go @@ -240,8 +240,8 @@ func TestReconcilerUpdateBindings(t *testing.T) { }, "test update binding with no latestResourceSnapshotName": { - name: "Empty latestResourceSnapshotName", - Client: &test.MockClient{ + name: "Empty latestResourceSnapshotName", + Client: &test.MockClient{ MockUpdate: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { return nil }, @@ -257,8 +257,8 @@ func TestReconcilerUpdateBindings(t *testing.T) { wantErr: false, }, "test update binding with nil toBeUpgradedBinding": { - name: "Nil toBeUpgradedBinding", - Client: &test.MockClient{ + name: "Nil toBeUpgradedBinding", + Client: &test.MockClient{ MockUpdate: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { return nil }, @@ -268,8 +268,8 @@ func TestReconcilerUpdateBindings(t *testing.T) { wantErr: false, }, "test update binding with empty toBeUpgradedBinding": { - name: "Empty toBeUpgradedBinding", - Client: &test.MockClient{ + name: "Empty toBeUpgradedBinding", + Client: &test.MockClient{ MockUpdate: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { return nil }, @@ -279,8 +279,8 @@ func TestReconcilerUpdateBindings(t *testing.T) { wantErr: false, }, "test update binding with failed binding": { - name: "Failed Binding", - Client: &test.MockClient{ + name: "Failed Binding", + Client: &test.MockClient{ MockUpdate: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { return errors.New("Failed to update binding") }, @@ -293,7 +293,7 @@ func TestReconcilerUpdateBindings(t *testing.T) { generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster4), generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster5), }, - wantErr: true, + wantErr: true, }, } for name, tt := range tests { From 67e612641936e224a85436e2745c46688661d10a Mon Sep 17 00:00:00 2001 From: Britania Rodriguez Reyes <145056127+britaniar@users.noreply.github.com> Date: Mon, 6 Nov 2023 15:25:49 -0800 Subject: [PATCH 3/7] Add test cases for different states --- pkg/controllers/rollout/controller_test.go | 60 ++++++++++++++++------ 1 file changed, 43 insertions(+), 17 deletions(-) diff --git a/pkg/controllers/rollout/controller_test.go b/pkg/controllers/rollout/controller_test.go index a22f1a37d..0b769f8ba 100644 --- a/pkg/controllers/rollout/controller_test.go +++ b/pkg/controllers/rollout/controller_test.go @@ -227,20 +227,8 @@ func TestReconcilerUpdateBindings(t *testing.T) { wantErr bool }{ // TODO: Add negative test cases with fake client - "test update bindings with no bindings": { - name: "test3", - Client: &test.MockClient{ - MockUpdate: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - return nil - }, - }, - latestResourceSnapshotName: "snapshot-2", - toBeUpgradedBinding: []*fleetv1beta1.ClusterResourceBinding{}, - wantErr: false, - }, - "test update binding with no latestResourceSnapshotName": { - name: "Empty latestResourceSnapshotName", + name: "testResourceSnapshot1", Client: &test.MockClient{ MockUpdate: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { return nil @@ -279,22 +267,60 @@ func TestReconcilerUpdateBindings(t *testing.T) { wantErr: false, }, "test update binding with failed binding": { - name: "Failed Binding", + name: "Binding State with update error", Client: &test.MockClient{ MockUpdate: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { return errors.New("Failed to update binding") }, }, - latestResourceSnapshotName: "snapshot1", + latestResourceSnapshotName: "snapshot-2", toBeUpgradedBinding: []*fleetv1beta1.ClusterResourceBinding{ - generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, "snapshot-1", cluster1), + generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster1), generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster2), - generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster3), + generateFailedToApplyClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster3), generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster4), generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster5), }, wantErr: true, }, + "test update binding with error for scheduled state": { + name: "Scheduled state with update error", + Client: &test.MockClient{ + MockUpdate: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + // Return an error for the scheduled state + if obj.(*fleetv1beta1.ClusterResourceBinding).Spec.State == fleetv1beta1.BindingStateBound { + return errors.New("Failed to mark binding") + } + return nil + }, + }, + latestResourceSnapshotName: "snapshot-1", + toBeUpgradedBinding: []*fleetv1beta1.ClusterResourceBinding{ + generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, "snapshot-1", cluster1), + generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, "snapshot-1", cluster2), + generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, "snapshot-1", cluster3), + generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, "snapshot-1", cluster4), + generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, "snapshot-1", cluster5), + }, + wantErr: true, + }, + "test update binding with unscheduled state": { + name: "Delete unscheduled state", + Client: &test.MockClient{ + MockDelete: func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { + return nil + }, + }, + latestResourceSnapshotName: "snapshot-2", + toBeUpgradedBinding: []*fleetv1beta1.ClusterResourceBinding{ + generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster1), + generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster2), + generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster3), + generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster4), + generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster5), + }, + wantErr: false, + }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { From 7dd1a2f841927249699411efd5bee86ea4f25114 Mon Sep 17 00:00:00 2001 From: Britania Rodriguez Reyes <145056127+britaniar@users.noreply.github.com> Date: Tue, 7 Nov 2023 11:50:39 -0800 Subject: [PATCH 4/7] fix formatting --- pkg/controllers/rollout/controller_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controllers/rollout/controller_test.go b/pkg/controllers/rollout/controller_test.go index 0b769f8ba..28a20d08c 100644 --- a/pkg/controllers/rollout/controller_test.go +++ b/pkg/controllers/rollout/controller_test.go @@ -320,7 +320,7 @@ func TestReconcilerUpdateBindings(t *testing.T) { generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster5), }, wantErr: false, - }, + }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { From f33e0700135381ffaf320fd0859b6c9c1705331c Mon Sep 17 00:00:00 2001 From: Britania Rodriguez Reyes <145056127+britaniar@users.noreply.github.com> Date: Tue, 7 Nov 2023 14:30:15 -0800 Subject: [PATCH 5/7] Add delete error for unscheduled state test case --- pkg/controllers/rollout/controller_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/pkg/controllers/rollout/controller_test.go b/pkg/controllers/rollout/controller_test.go index 28a20d08c..2a79df1ce 100644 --- a/pkg/controllers/rollout/controller_test.go +++ b/pkg/controllers/rollout/controller_test.go @@ -321,6 +321,23 @@ func TestReconcilerUpdateBindings(t *testing.T) { }, wantErr: false, }, + "test update binding with error for unscheduled state": { + name: "Delete unscheduled state with error", + Client: &test.MockClient{ + MockDelete: func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { + return errors.New("Failed to delete binding") + }, + }, + latestResourceSnapshotName: "snapshot-2", + toBeUpgradedBinding: []*fleetv1beta1.ClusterResourceBinding{ + generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster1), + generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster2), + generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster3), + generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster4), + generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster5), + }, + wantErr: true, + }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { From 2fb609863da7072289152f2dc272cb4bdd6fa173 Mon Sep 17 00:00:00 2001 From: Britania Rodriguez Reyes <145056127+britaniar@users.noreply.github.com> Date: Wed, 8 Nov 2023 14:33:22 -0800 Subject: [PATCH 6/7] Add IsNotFound test case --- pkg/controllers/rollout/controller_test.go | 39 ++++++++++------------ 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/pkg/controllers/rollout/controller_test.go b/pkg/controllers/rollout/controller_test.go index 2a79df1ce..520360460 100644 --- a/pkg/controllers/rollout/controller_test.go +++ b/pkg/controllers/rollout/controller_test.go @@ -14,7 +14,9 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/test" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/util/workqueue" "k8s.io/utils/pointer" @@ -226,7 +228,6 @@ func TestReconcilerUpdateBindings(t *testing.T) { toBeUpgradedBinding []*fleetv1beta1.ClusterResourceBinding wantErr bool }{ - // TODO: Add negative test cases with fake client "test update binding with no latestResourceSnapshotName": { name: "testResourceSnapshot1", Client: &test.MockClient{ @@ -238,9 +239,6 @@ func TestReconcilerUpdateBindings(t *testing.T) { toBeUpgradedBinding: []*fleetv1beta1.ClusterResourceBinding{ generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, "snapshot-1", cluster1), generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster2), - generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster3), - generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster4), - generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster5), }, wantErr: false, }, @@ -275,11 +273,7 @@ func TestReconcilerUpdateBindings(t *testing.T) { }, latestResourceSnapshotName: "snapshot-2", toBeUpgradedBinding: []*fleetv1beta1.ClusterResourceBinding{ - generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster1), - generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster2), - generateFailedToApplyClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster3), - generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster4), - generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster5), + generateFailedToApplyClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster1), }, wantErr: true, }, @@ -297,10 +291,6 @@ func TestReconcilerUpdateBindings(t *testing.T) { latestResourceSnapshotName: "snapshot-1", toBeUpgradedBinding: []*fleetv1beta1.ClusterResourceBinding{ generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, "snapshot-1", cluster1), - generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, "snapshot-1", cluster2), - generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, "snapshot-1", cluster3), - generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, "snapshot-1", cluster4), - generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, "snapshot-1", cluster5), }, wantErr: true, }, @@ -314,10 +304,6 @@ func TestReconcilerUpdateBindings(t *testing.T) { latestResourceSnapshotName: "snapshot-2", toBeUpgradedBinding: []*fleetv1beta1.ClusterResourceBinding{ generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster1), - generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster2), - generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster3), - generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster4), - generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster5), }, wantErr: false, }, @@ -325,19 +311,28 @@ func TestReconcilerUpdateBindings(t *testing.T) { name: "Delete unscheduled state with error", Client: &test.MockClient{ MockDelete: func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { - return errors.New("Failed to delete binding") + return errors.New("Failed to delete unselected binding") }, }, latestResourceSnapshotName: "snapshot-2", toBeUpgradedBinding: []*fleetv1beta1.ClusterResourceBinding{ generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster1), - generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster2), - generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster3), - generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster4), - generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster5), }, wantErr: true, }, + "test update binding with IsNotFound error for unscheduled state": { + name: "Delete unscheduled state with IsNotFound error", + Client: &test.MockClient{ + MockDelete: func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { + return apierrors.NewNotFound(schema.GroupResource{}, "invalid") + }, + }, + latestResourceSnapshotName: "snapshot-2", + toBeUpgradedBinding: []*fleetv1beta1.ClusterResourceBinding{ + generateClusterResourceBinding(fleetv1beta1.BindingStateUnscheduled, "snapshot-1", cluster1), + }, + wantErr: false, + }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { From 71e7a4b632b125ffc21dea0645c0fac0f04682e0 Mon Sep 17 00:00:00 2001 From: Britania Rodriguez Reyes <145056127+britaniar@users.noreply.github.com> Date: Fri, 10 Nov 2023 14:56:54 -0800 Subject: [PATCH 7/7] Resolve comments --- pkg/controllers/rollout/controller_test.go | 30 +++------------------- 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/pkg/controllers/rollout/controller_test.go b/pkg/controllers/rollout/controller_test.go index 520360460..0b92a207b 100644 --- a/pkg/controllers/rollout/controller_test.go +++ b/pkg/controllers/rollout/controller_test.go @@ -228,38 +228,16 @@ func TestReconcilerUpdateBindings(t *testing.T) { toBeUpgradedBinding []*fleetv1beta1.ClusterResourceBinding wantErr bool }{ - "test update binding with no latestResourceSnapshotName": { - name: "testResourceSnapshot1", - Client: &test.MockClient{ - MockUpdate: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - return nil - }, - }, - latestResourceSnapshotName: "", - toBeUpgradedBinding: []*fleetv1beta1.ClusterResourceBinding{ - generateClusterResourceBinding(fleetv1beta1.BindingStateScheduled, "snapshot-1", cluster1), - generateClusterResourceBinding(fleetv1beta1.BindingStateBound, "snapshot-1", cluster2), - }, - wantErr: false, - }, "test update binding with nil toBeUpgradedBinding": { - name: "Nil toBeUpgradedBinding", - Client: &test.MockClient{ - MockUpdate: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - return nil - }, - }, + name: "Nil toBeUpgradedBinding", + Client: &test.MockClient{}, latestResourceSnapshotName: "snapshot-2", toBeUpgradedBinding: nil, wantErr: false, }, "test update binding with empty toBeUpgradedBinding": { - name: "Empty toBeUpgradedBinding", - Client: &test.MockClient{ - MockUpdate: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - return nil - }, - }, + name: "Empty toBeUpgradedBinding", + Client: &test.MockClient{}, latestResourceSnapshotName: "snapshot-2", toBeUpgradedBinding: []*fleetv1beta1.ClusterResourceBinding{}, wantErr: false,