Merge pull request #1419 from AllenZMC/improve_test_util

remove redundant code and improve test coverage

pkg/util/binding.go

@@ -30,10 +30,7 @@ func IsBindingReplicasChanged(bindingSpec *workv1alpha2.ResourceBindingSpec, str
 		return false
 	}
 	if strategy.ReplicaSchedulingType == policyv1alpha1.ReplicaSchedulingTypeDivided {
-		replicasSum := int32(0)
-		for _, targetCluster := range bindingSpec.Clusters {
-			replicasSum += targetCluster.Replicas
-		}
+		replicasSum := GetSumOfReplicas(bindingSpec.Clusters)
 		return replicasSum != bindingSpec.Replicas
 	}
 	return false

pkg/util/binding_test.go

@@ -1,8 +1,12 @@
 package util
 
 import (
+	"reflect"
 	"testing"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1"
 	workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
 	testhelper "github.com/karmada-io/karmada/test/helper"
 )
@@ -160,3 +164,197 @@ func TestDivideReplicasByTargetCluster(t *testing.T) {
 		})
 	}
 }
+
+func TestGetBindingClusterNames(t *testing.T) {
+	tests := []struct {
+		name     string
+		binding  *workv1alpha2.ResourceBinding
+		expected []string
+	}{
+		{
+			name: "nil",
+			binding: &workv1alpha2.ResourceBinding{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "demo-name",
+					Namespace: "demo-ns",
+				},
+				Spec: workv1alpha2.ResourceBindingSpec{
+					Clusters: []workv1alpha2.TargetCluster{},
+				},
+				Status: workv1alpha2.ResourceBindingStatus{},
+			},
+			expected: nil,
+		},
+		{
+			name: "not nil",
+			binding: &workv1alpha2.ResourceBinding{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "demo-name",
+					Namespace: "demo-ns",
+				},
+				Spec: workv1alpha2.ResourceBindingSpec{
+					Clusters: []workv1alpha2.TargetCluster{
+						{
+							Name: ClusterMember1,
+						},
+						{
+							Name: ClusterMember2,
+						},
+					},
+				},
+				Status: workv1alpha2.ResourceBindingStatus{},
+			},
+			expected: []string{ClusterMember1, ClusterMember2},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := GetBindingClusterNames(tt.binding)
+			if !reflect.DeepEqual(got, tt.expected) {
+				t.Errorf("GetBindingClusterNames() = %v, want %v", got, tt.expected)
+			}
+		})
+	}
+}
+
+func TestIsBindingReplicasChanged(t *testing.T) {
+	tests := []struct {
+		name        string
+		bindingSpec *workv1alpha2.ResourceBindingSpec
+		strategy    *policyv1alpha1.ReplicaSchedulingStrategy
+		expected    bool
+	}{
+		{
+			name:        "nil strategy",
+			bindingSpec: &workv1alpha2.ResourceBindingSpec{},
+			strategy:    nil,
+			expected:    false,
+		},
+		{
+			name:        "empty strategy",
+			bindingSpec: &workv1alpha2.ResourceBindingSpec{},
+			strategy:    &policyv1alpha1.ReplicaSchedulingStrategy{},
+			expected:    false,
+		},
+		{
+			name: "Duplicated strategy and replicas not changed",
+			bindingSpec: &workv1alpha2.ResourceBindingSpec{
+				Replicas: 5,
+				Clusters: []workv1alpha2.TargetCluster{
+					{
+						Name:     ClusterMember1,
+						Replicas: 5,
+					},
+					{
+						Name:     ClusterMember2,
+						Replicas: 5,
+					},
+				}},
+			strategy: &policyv1alpha1.ReplicaSchedulingStrategy{ReplicaSchedulingType: policyv1alpha1.ReplicaSchedulingTypeDuplicated},
+			expected: false,
+		},
+		{
+			name: "Duplicated strategy and replicas changed",
+			bindingSpec: &workv1alpha2.ResourceBindingSpec{
+				Replicas: 5,
+				Clusters: []workv1alpha2.TargetCluster{
+					{
+						Name:     ClusterMember1,
+						Replicas: 3,
+					},
+					{
+						Name:     ClusterMember2,
+						Replicas: 5,
+					},
+				}},
+			strategy: &policyv1alpha1.ReplicaSchedulingStrategy{ReplicaSchedulingType: policyv1alpha1.ReplicaSchedulingTypeDuplicated},
+			expected: true,
+		},
+		{
+			name: "Divided strategy and replicas not changed",
+			bindingSpec: &workv1alpha2.ResourceBindingSpec{
+				Replicas: 5,
+				Clusters: []workv1alpha2.TargetCluster{
+					{
+						Name:     ClusterMember1,
+						Replicas: 2,
+					},
+					{
+						Name:     ClusterMember2,
+						Replicas: 3,
+					},
+				},
+			},
+			strategy: &policyv1alpha1.ReplicaSchedulingStrategy{ReplicaSchedulingType: policyv1alpha1.ReplicaSchedulingTypeDivided},
+			expected: false,
+		},
+		{
+			name: "Divided strategy and replicas changed",
+			bindingSpec: &workv1alpha2.ResourceBindingSpec{
+				Replicas: 5,
+				Clusters: []workv1alpha2.TargetCluster{
+					{
+						Name:     ClusterMember1,
+						Replicas: 3,
+					},
+					{
+						Name:     ClusterMember2,
+						Replicas: 3,
+					},
+				},
+			},
+			strategy: &policyv1alpha1.ReplicaSchedulingStrategy{ReplicaSchedulingType: policyv1alpha1.ReplicaSchedulingTypeDivided},
+			expected: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.bindingSpec == nil {
+				t.FailNow()
+			}
+			got := IsBindingReplicasChanged(tt.bindingSpec, tt.strategy)
+			if got != tt.expected {
+				t.Errorf("IsBindingReplicasChanged() = %v, want %v", got, tt.expected)
+			}
+		})
+	}
+}
+
+func TestGetSumOfReplicas(t *testing.T) {
+	tests := []struct {
+		name     string
+		clusters []workv1alpha2.TargetCluster
+		expected int32
+	}{
+		{
+			name:     "empty",
+			clusters: []workv1alpha2.TargetCluster{},
+			expected: 0,
+		},
+		{
+			name: "not empty",
+			clusters: []workv1alpha2.TargetCluster{
+				{
+					Name:     ClusterMember1,
+					Replicas: 2,
+				},
+				{
+					Name:     ClusterMember2,
+					Replicas: 3,
+				},
+			},
+			expected: 5,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := GetSumOfReplicas(tt.clusters)
+			if got != tt.expected {
+				t.Errorf("GetSumOfReplicas() = %v, want %v", got, tt.expected)
+			}
+		})
+	}
+}