Skip to content

Commit

Permalink
refactor: Replace reflect.DeepEqual by equality.Semantic.DeepEqual
Browse files Browse the repository at this point in the history
Signed-off-by: mevrin <[email protected]>
  • Loading branch information
lekaf974 committed May 30, 2024
1 parent bc356e9 commit 423dcaf
Show file tree
Hide file tree
Showing 42 changed files with 134 additions and 129 deletions.
4 changes: 2 additions & 2 deletions cmd/controller-manager/app/options/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ limitations under the License.
package options

import (
"reflect"
"testing"
"time"

"github.com/spf13/pflag"
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/tools/leaderelection/resourcelock"
"k8s.io/component-base/config"
Expand Down Expand Up @@ -79,7 +79,7 @@ func TestAddFlags(t *testing.T) {
WorkerThreadsForGC: 1,
}

if !reflect.DeepEqual(expected, s) {
if !equality.Semantic.DeepEqual(expected, s) {
t.Errorf("Got different run options than expected.\nGot: %+v\nExpected: %+v\n", s, expected)
}

Expand Down
4 changes: 2 additions & 2 deletions cmd/scheduler/app/options/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ limitations under the License.
package options

import (
"reflect"
"testing"
"time"

"github.com/spf13/pflag"
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/tools/leaderelection/resourcelock"
"k8s.io/component-base/config"
Expand Down Expand Up @@ -78,7 +78,7 @@ func TestAddFlags(t *testing.T) {
CacheDumpFileDir: "/tmp",
}

if !reflect.DeepEqual(expected, s) {
if !equality.Semantic.DeepEqual(expected, s) {
t.Errorf("Got different run options than expected.\nGot: %+v\nExpected: %+v\n", s, expected)
}
}
14 changes: 7 additions & 7 deletions docs/ut_coverage/UT_coverage_v1.4.0.html
Original file line number Diff line number Diff line change
Expand Up @@ -4005,7 +4005,7 @@
}</span>

<span class="cov8" title="1">minResources := cc.calcPGMinResources(job)
if pg.Spec.MinMember != job.Spec.MinAvailable || !reflect.DeepEqual(pg.Spec.MinResources, minResources) </span><span class="cov0" title="0">{
if pg.Spec.MinMember != job.Spec.MinAvailable || !equality.Semantic.DeepEqual(pg.Spec.MinResources, minResources) </span><span class="cov0" title="0">{
pg.Spec.MinMember = job.Spec.MinAvailable
pg.Spec.MinResources = minResources
pgShouldUpdate = true
Expand Down Expand Up @@ -4269,7 +4269,7 @@

// NOTE: Since we only reconcile job based on Spec, we will ignore other attributes
// For Job status, it's used internally and always been updated via our controller.
<span class="cov8" title="1">if reflect.DeepEqual(newJob.Spec, oldJob.Spec) &amp;&amp; newJob.Status.State.Phase == oldJob.Status.State.Phase </span><span class="cov0" title="0">{
<span class="cov8" title="1">if equality.Semantic.DeepEqual(newJob.Spec, oldJob.Spec) &amp;&amp; newJob.Status.State.Phase == oldJob.Status.State.Phase </span><span class="cov0" title="0">{
klog.V(6).Infof("Job update event is ignored since no update in 'Spec'.")
return
}</span>
Expand Down Expand Up @@ -6097,7 +6097,7 @@
}</span>

// ignore update when status does not change
<span class="cov8" title="1">if reflect.DeepEqual(queueStatus, queue.Status) </span><span class="cov0" title="0">{
<span class="cov8" title="1">if equality.Semantic.DeepEqual(queueStatus, queue.Status) </span><span class="cov0" title="0">{
return nil
}</span>

Expand Down Expand Up @@ -12437,7 +12437,7 @@
newTransitionID := newCond.TransitionID
newCond.TransitionID = oldCond.TransitionID

shouldUpdate := !reflect.DeepEqual(&amp;newCond, &amp;oldCond)
shouldUpdate := !equality.Semantic.DeepEqual(&amp;newCond, &amp;oldCond)

newCond.LastTransitionTime = newTime
newCond.TransitionID = newTransitionID
Expand All @@ -12455,7 +12455,7 @@
oldCondition := oldStatus.Conditions
oldStatus.Conditions = nil

return !reflect.DeepEqual(newStatus, oldStatus) || isPodGroupConditionsUpdated(newCondition, oldCondition)
return !equality.Semantic.DeepEqual(newStatus, oldStatus) || isPodGroupConditionsUpdated(newCondition, oldCondition)
}</span>

// updateJob update specified job
Expand Down Expand Up @@ -19010,7 +19010,7 @@
"k8s.io/api/admission/v1beta1"
whv1beta1 "k8s.io/api/admissionregistration/v1beta1"
v1 "k8s.io/api/core/v1"
apiequality "k8s.io/apimachinery/pkg/api/equality"
apieequality "k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
Expand Down Expand Up @@ -19234,7 +19234,7 @@
}</span>
}

<span class="cov8" title="1">if !apiequality.Semantic.DeepEqual(new.Spec, old.Spec) </span><span class="cov8" title="1">{
<span class="cov8" title="1">if !apieequality.Semantic.DeepEqual(new.Spec, old.Spec) </span><span class="cov8" title="1">{
return fmt.Errorf("job updates may not change fields other than `minAvailable`, `tasks[*].replicas under spec`")
}</span>

Expand Down
14 changes: 7 additions & 7 deletions docs/ut_coverage/UT_coverage_v1.5.0.html
Original file line number Diff line number Diff line change
Expand Up @@ -4265,7 +4265,7 @@
}</span>

<span class="cov8" title="1">minResources := cc.calcPGMinResources(job)
if pg.Spec.MinMember != job.Spec.MinAvailable || !reflect.DeepEqual(pg.Spec.MinResources, minResources) </span><span class="cov0" title="0">{
if pg.Spec.MinMember != job.Spec.MinAvailable || !equality.Semantic.DeepEqual(pg.Spec.MinResources, minResources) </span><span class="cov0" title="0">{
pg.Spec.MinMember = job.Spec.MinAvailable
pg.Spec.MinResources = minResources
pgShouldUpdate = true
Expand Down Expand Up @@ -4537,7 +4537,7 @@

// NOTE: Since we only reconcile job based on Spec, we will ignore other attributes
// For Job status, it's used internally and always been updated via our controller.
<span class="cov8" title="1">if reflect.DeepEqual(newJob.Spec, oldJob.Spec) &amp;&amp; newJob.Status.State.Phase == oldJob.Status.State.Phase </span><span class="cov0" title="0">{
<span class="cov8" title="1">if equality.Semantic.DeepEqual(newJob.Spec, oldJob.Spec) &amp;&amp; newJob.Status.State.Phase == oldJob.Status.State.Phase </span><span class="cov0" title="0">{
klog.V(6).Infof("Job update event is ignored since no update in 'Spec'.")
return
}</span>
Expand Down Expand Up @@ -6584,7 +6584,7 @@
}</span>

// ignore update when status does not change
<span class="cov8" title="1">if reflect.DeepEqual(queueStatus, queue.Status) </span><span class="cov0" title="0">{
<span class="cov8" title="1">if equality.Semantic.DeepEqual(queueStatus, queue.Status) </span><span class="cov0" title="0">{
return nil
}</span>

Expand Down Expand Up @@ -13646,7 +13646,7 @@
newTransitionID := newCond.TransitionID
newCond.TransitionID = oldCond.TransitionID

shouldUpdate := !reflect.DeepEqual(&amp;newCond, &amp;oldCond)
shouldUpdate := !equality.Semantic.DeepEqual(&amp;newCond, &amp;oldCond)

newCond.LastTransitionTime = newTime
newCond.TransitionID = newTransitionID
Expand All @@ -13664,7 +13664,7 @@
oldCondition := oldStatus.Conditions
oldStatus.Conditions = nil

return !reflect.DeepEqual(newStatus, oldStatus) || isPodGroupConditionsUpdated(newCondition, oldCondition)
return !equality.Semantic.DeepEqual(newStatus, oldStatus) || isPodGroupConditionsUpdated(newCondition, oldCondition)
}</span>

// updateJob update specified job
Expand Down Expand Up @@ -20285,7 +20285,7 @@
"k8s.io/api/admission/v1beta1"
whv1beta1 "k8s.io/api/admissionregistration/v1beta1"
v1 "k8s.io/api/core/v1"
apiequality "k8s.io/apimachinery/pkg/api/equality"
apieequality "k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
Expand Down Expand Up @@ -20521,7 +20521,7 @@
}</span>
}

<span class="cov8" title="1">if !apiequality.Semantic.DeepEqual(new.Spec, old.Spec) </span><span class="cov8" title="1">{
<span class="cov8" title="1">if !apieequality.Semantic.DeepEqual(new.Spec, old.Spec) </span><span class="cov8" title="1">{
return fmt.Errorf("job updates may not change fields other than `minAvailable`, `tasks[*].replicas under spec`")
}</span>

Expand Down
4 changes: 2 additions & 2 deletions pkg/controllers/job/job_controller_actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ package job
import (
"context"
"fmt"
"reflect"
"sort"
"sync"
"sync/atomic"
"time"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
quotav1 "k8s.io/apiserver/pkg/quota/v1"
Expand Down Expand Up @@ -712,7 +712,7 @@ func (cc *jobcontroller) createOrUpdatePodGroup(job *batch.Job) error {
}

minResources := cc.calcPGMinResources(job)
if pg.Spec.MinMember != job.Spec.MinAvailable || !reflect.DeepEqual(pg.Spec.MinResources, minResources) {
if pg.Spec.MinMember != job.Spec.MinAvailable || !equality.Semantic.DeepEqual(pg.Spec.MinResources, minResources) {
pg.Spec.MinMember = job.Spec.MinAvailable
pg.Spec.MinResources = minResources
pgShouldUpdate = true
Expand Down
4 changes: 2 additions & 2 deletions pkg/controllers/job/job_controller_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ package job
import (
"context"
"fmt"
"reflect"
"strconv"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/tools/cache"
Expand Down Expand Up @@ -97,7 +97,7 @@ func (cc *jobcontroller) updateJob(oldObj, newObj interface{}) {

// NOTE: Since we only reconcile job based on Spec, we will ignore other attributes
// For Job status, it's used internally and always been updated via our controller.
if reflect.DeepEqual(newJob.Spec, oldJob.Spec) && newJob.Status.State.Phase == oldJob.Status.State.Phase {
if equality.Semantic.DeepEqual(newJob.Spec, oldJob.Spec) && newJob.Status.State.Phase == oldJob.Status.State.Phase {
klog.V(6).Infof("Job update event is ignored since no update in 'Spec'.")
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ package pytorch

import (
"fmt"
"reflect"
"testing"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"volcano.sh/apis/pkg/apis/batch/v1alpha1"
Expand Down Expand Up @@ -365,7 +365,7 @@ func TestPytorch(t *testing.T) {
}
}

if !reflect.DeepEqual(testcase.Pod.Spec.Containers[0].Env, testcase.envs) {
if !equality.Semantic.DeepEqual(testcase.Pod.Spec.Containers[0].Env, testcase.envs) {
t.Errorf("Case %d (%s): wrong envs, got %v, expected %v", index, testcase.Name, testcase.Pod.Spec.Containers[0].Env, testcase.envs)
}
})
Expand Down
14 changes: 7 additions & 7 deletions pkg/controllers/jobflow/jobflow_controller_action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ package jobflow

import (
"context"
"reflect"
"testing"
"time"

"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/informers"
kubeclient "k8s.io/client-go/kubernetes/fake"
Expand Down Expand Up @@ -183,7 +183,7 @@ func TestSyncJobFlowFunc(t *testing.T) {
tt.args.jobFlow.Status.JobStatusList[i].RunningHistories[i2].StartTimestamp = metav1.Time{}
}
}
if !reflect.DeepEqual(&tt.args.jobFlow.Status, tt.want.jobFlowStatus) {
if !equality.Semantic.DeepEqual(&tt.args.jobFlow.Status, tt.want.jobFlowStatus) {
t.Error("not the expected result")
}
})
Expand Down Expand Up @@ -246,7 +246,7 @@ func TestGetRunningHistoriesFunc(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := getRunningHistories(tt.args.jobStatusList, tt.args.job); !reflect.DeepEqual(got, tt.want) {
if got := getRunningHistories(tt.args.jobStatusList, tt.args.job); !equality.Semantic.DeepEqual(got, tt.want) {
t.Errorf("getRunningHistories() = %v, want %v", got, tt.want)
}
})
Expand Down Expand Up @@ -406,7 +406,7 @@ func TestGetAllJobStatusFunc(t *testing.T) {
got.JobStatusList[0].RunningHistories[0].StartTimestamp = metav1.Time{}
got.JobStatusList[1].RunningHistories[0].StartTimestamp = metav1.Time{}
}
if !reflect.DeepEqual(got, tt.want) {
if !equality.Semantic.DeepEqual(got, tt.want) {
t.Errorf("getAllJobStatus() got = %v, want %v", got, tt.want)
}
})
Expand Down Expand Up @@ -486,13 +486,13 @@ func TestLoadJobTemplateAndSetJobFunc(t *testing.T) {
if got := fakeController.loadJobTemplateAndSetJob(tt.args.jobFlow, tt.args.flowName, tt.args.jobName, tt.args.job); got != tt.want.Err {
t.Error("Expected loadJobTemplateAndSetJob() return nil, but not nil")
}
if !reflect.DeepEqual(tt.args.job.OwnerReferences, tt.want.OwnerReference) {
if !equality.Semantic.DeepEqual(tt.args.job.OwnerReferences, tt.want.OwnerReference) {
t.Error("not expected job OwnerReferences")
}
if !reflect.DeepEqual(tt.args.job.Annotations, tt.want.Annotations) {
if !equality.Semantic.DeepEqual(tt.args.job.Annotations, tt.want.Annotations) {
t.Error("not expected job Annotations")
}
if !reflect.DeepEqual(tt.args.job.Labels, tt.want.Labels) {
if !equality.Semantic.DeepEqual(tt.args.job.Labels, tt.want.Labels) {
t.Error("not expected job Annotations")
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ package jobtemplate

import (
"context"
"reflect"
"k8s.io/apimachinery/pkg/api/equality"
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -115,7 +115,7 @@ func TestSyncJobTemplateFunc(t *testing.T) {
if got := fakeController.syncJobTemplate(tt.args.jobTemplate); got != tt.want.err {
t.Error("Expected deleteAllJobsCreateByJobFlow() return nil, but not nil")
}
if !reflect.DeepEqual(&tt.args.jobTemplate.Status, tt.want.jobTemplateStatus) {
if !equality.Semantic.DeepEqual(&tt.args.jobTemplate.Status, tt.want.jobTemplateStatus) {
t.Error("not the expected result")
}
})
Expand Down
4 changes: 2 additions & 2 deletions pkg/controllers/podgroup/pg_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ package podgroup

import (
"context"
"reflect"
"testing"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/informers"
Expand Down Expand Up @@ -166,7 +166,7 @@ func TestAddPodGroup(t *testing.T) {
t.Errorf("Case %s failed when getting podGroup for %v", testCase.name, err)
}

if false == reflect.DeepEqual(pg.OwnerReferences, testCase.expectedPodGroup.OwnerReferences) {
if false == equality.Semantic.DeepEqual(pg.OwnerReferences, testCase.expectedPodGroup.OwnerReferences) {
t.Errorf("Case %s failed, expect %v, got %v", testCase.name, testCase.expectedPodGroup, pg)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/controllers/queue/queue_controller_action.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ package queue
import (
"context"
"fmt"
"reflect"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/tools/cache"
"k8s.io/klog/v2"
Expand Down Expand Up @@ -75,7 +75,7 @@ func (c *queuecontroller) syncQueue(queue *schedulingv1beta1.Queue, updateStateF
}

// ignore update when status does not change
if reflect.DeepEqual(queueStatus, queue.Status) {
if equality.Semantic.DeepEqual(queueStatus, queue.Status) {
return nil
}

Expand Down
5 changes: 3 additions & 2 deletions pkg/scheduler/actions/allocate/allocate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/agiledragon/gomonkey/v2"
v1 "k8s.io/api/core/v1"
storagev1 "k8s.io/api/storage/v1"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/fake"
Expand Down Expand Up @@ -322,10 +323,10 @@ func TestAllocateWithDynamicPVC(t *testing.T) {

allocate.Execute(ssn)
bindResults := binder.Binds()
if !reflect.DeepEqual(test.expectedBind, bindResults) {
if !equality.Semantic.DeepEqual(test.expectedBind, bindResults) {
t.Errorf("expected: %v, got %v ", test.expectedBind, bindResults)
}
if !reflect.DeepEqual(test.expectedActions, fakeVolumeBinder.Actions) {
if !equality.Semantic.DeepEqual(test.expectedActions, fakeVolumeBinder.Actions) {
t.Errorf("expected: %v, got %v ", test.expectedActions, fakeVolumeBinder.Actions)
}
fakeVolumeBinder.Actions = make(map[string][]string)
Expand Down
4 changes: 2 additions & 2 deletions pkg/scheduler/api/helpers/helpers_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package helpers

import (
"reflect"
"testing"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"

"volcano.sh/volcano/pkg/scheduler/api"
)
Expand Down Expand Up @@ -36,7 +36,7 @@ func TestMax(t *testing.T) {
},
}
re := Max(l, r)
if !reflect.DeepEqual(expected, re) {
if !equality.Semantic.DeepEqual(expected, re) {
t.Errorf("expected: %#v, got: %#v", expected, re)
}
}
Loading

0 comments on commit 423dcaf

Please sign in to comment.