From 3c61c61105878b5d633ce795c58e66a4c8f58114 Mon Sep 17 00:00:00 2001
From: AllenZMC <zhongming.chang@daocloud.io>
Date: Wed, 9 Mar 2022 23:09:59 +0800
Subject: [PATCH] reduce unnecessary code operations in some cases and improve
 test coverage

Signed-off-by: AllenZMC <zhongming.chang@daocloud.io>
---
 pkg/util/binding.go      |   6 ++
 pkg/util/binding_test.go | 149 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 155 insertions(+)

diff --git a/pkg/util/binding.go b/pkg/util/binding.go
index e7f88696b..80e4c679f 100644
--- a/pkg/util/binding.go
+++ b/pkg/util/binding.go
@@ -87,6 +87,12 @@ func DivideReplicasByTargetCluster(clusters []workv1alpha2.TargetCluster, sum in
 
 // MergeTargetClusters will merge the replicas in two TargetCluster
 func MergeTargetClusters(old, new []workv1alpha2.TargetCluster) []workv1alpha2.TargetCluster {
+	switch {
+	case len(old) == 0:
+		return new
+	case len(new) == 0:
+		return old
+	}
 	// oldMap is a map of the result for the old replicas so that it can be merged with the new result easily
 	oldMap := make(map[string]int32)
 	for _, cluster := range old {
diff --git a/pkg/util/binding_test.go b/pkg/util/binding_test.go
index 1bc4ab67f..bd0b52e09 100644
--- a/pkg/util/binding_test.go
+++ b/pkg/util/binding_test.go
@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
 
 	policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1"
 	workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
@@ -358,3 +359,151 @@ func TestGetSumOfReplicas(t *testing.T) {
 		})
 	}
 }
+
+func TestConvertToClusterNames(t *testing.T) {
+	tests := []struct {
+		name     string
+		clusters []workv1alpha2.TargetCluster
+		expected sets.String
+	}{
+		{
+			name:     "empty",
+			clusters: []workv1alpha2.TargetCluster{},
+			expected: sets.String{},
+		},
+		{
+			name: "not empty",
+			clusters: []workv1alpha2.TargetCluster{
+				{
+					Name:     ClusterMember1,
+					Replicas: 2,
+				},
+				{
+					Name:     ClusterMember2,
+					Replicas: 3,
+				},
+			},
+			expected: sets.NewString(ClusterMember1, ClusterMember2),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := ConvertToClusterNames(tt.clusters)
+			if !reflect.DeepEqual(got, tt.expected) {
+				t.Errorf("ConvertToClusterNames() = %v, want %v", got, tt.expected)
+			}
+		})
+	}
+}
+
+func TestMergeTargetClusters(t *testing.T) {
+	tests := []struct {
+		name     string
+		old      []workv1alpha2.TargetCluster
+		new      []workv1alpha2.TargetCluster
+		expected []workv1alpha2.TargetCluster
+	}{
+		{
+			name:     "empty",
+			old:      []workv1alpha2.TargetCluster{},
+			new:      []workv1alpha2.TargetCluster{},
+			expected: []workv1alpha2.TargetCluster{},
+		},
+		{
+			name: "old clusters are empty",
+			old:  []workv1alpha2.TargetCluster{},
+			new: []workv1alpha2.TargetCluster{
+				{
+					Name:     ClusterMember2,
+					Replicas: 3,
+				},
+			},
+			expected: []workv1alpha2.TargetCluster{
+				{
+					Name:     ClusterMember2,
+					Replicas: 3,
+				},
+			},
+		},
+		{
+			name: "new clusters are empty",
+			old: []workv1alpha2.TargetCluster{
+				{
+					Name:     ClusterMember1,
+					Replicas: 3,
+				},
+			},
+			new: []workv1alpha2.TargetCluster{},
+			expected: []workv1alpha2.TargetCluster{
+				{
+					Name:     ClusterMember1,
+					Replicas: 3,
+				},
+			},
+		},
+		{
+			name: "no cluster with the same name",
+			old: []workv1alpha2.TargetCluster{
+				{
+					Name:     ClusterMember1,
+					Replicas: 2,
+				},
+			},
+			new: []workv1alpha2.TargetCluster{
+				{
+					Name:     ClusterMember2,
+					Replicas: 3,
+				},
+			},
+			expected: []workv1alpha2.TargetCluster{
+				{
+					Name:     ClusterMember1,
+					Replicas: 2,
+				},
+				{
+					Name:     ClusterMember2,
+					Replicas: 3,
+				},
+			},
+		},
+		{
+			name: "some clusters have the same name in the old and new clusters",
+			old: []workv1alpha2.TargetCluster{
+				{
+					Name:     ClusterMember1,
+					Replicas: 2,
+				},
+			},
+			new: []workv1alpha2.TargetCluster{
+				{
+					Name:     ClusterMember1,
+					Replicas: 4,
+				},
+				{
+					Name:     ClusterMember2,
+					Replicas: 3,
+				},
+			},
+			expected: []workv1alpha2.TargetCluster{
+				{
+					Name:     ClusterMember1,
+					Replicas: 6,
+				},
+				{
+					Name:     ClusterMember2,
+					Replicas: 3,
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := MergeTargetClusters(tt.old, tt.new)
+			if !testhelper.IsScheduleResultEqual(got, tt.expected) {
+				t.Errorf("MergeTargetClusters() = %v, want %v", got, tt.expected)
+			}
+		})
+	}
+}