Skip to content

Commit

Permalink
replaced reflect with cmp
Browse files Browse the repository at this point in the history
Signed-off-by: tariq-hasan <[email protected]>
  • Loading branch information
tariq-hasan committed Mar 10, 2024
1 parent fc858d1 commit 7185c18
Show file tree
Hide file tree
Showing 10 changed files with 32 additions and 29 deletions.
1 change: 1 addition & 0 deletions katib
10 changes: 5 additions & 5 deletions pkg/controller.v1beta1/experiment/manifest/generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ package manifest
import (
"errors"
"math"
"reflect"
"testing"

"github.com/golang/mock/gomock"
"github.com/google/go-cmp/cmp"
batchv1 "k8s.io/api/batch/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -152,8 +152,8 @@ func TestGetRunSpecWithHP(t *testing.T) {
} else if !tc.Err {
if err != nil {
t.Errorf("Case: %v failed. Expected nil, got %v", tc.testDescription, err)
} else if !reflect.DeepEqual(tc.expectedRunSpec, actualRunSpec) {
t.Errorf("Case: %v failed. Expected %v\n got %v", tc.testDescription, tc.expectedRunSpec.Object, actualRunSpec.Object)
} else if diff := cmp.Diff(tc.expectedRunSpec.Object, actualRunSpec.Object); diff != "" {
t.Errorf("Case: %v failed. Diff (-expected, +actual):\n%s", tc.testDescription, diff)
}
}
}
Expand Down Expand Up @@ -335,8 +335,8 @@ spec:
} else if !tc.Err {
if err != nil {
t.Errorf("Case: %v failed. Expected nil, got %v", tc.testDescription, err)
} else if !reflect.DeepEqual(expectedRunSpec, actualRunSpec) {
t.Errorf("Case: %v failed. Expected %v\n got %v", tc.testDescription, expectedRunSpec.Object, actualRunSpec.Object)
} else if diff := cmp.Diff(expectedRunSpec.Object, actualRunSpec.Object); diff != "" {
t.Errorf("Case: %v failed. Diff (-expected, +actual):\n%s", tc.testDescription, diff)
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/controller.v1beta1/suggestion/composer/composer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ import (
stdlog "log"
"os"
"path/filepath"
"reflect"
"strings"
"sync"
"testing"
"time"

"github.com/google/go-cmp/cmp"
"github.com/onsi/gomega"
"github.com/spf13/viper"
appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -631,8 +631,8 @@ func TestDesiredRBAC(t *testing.T) {
func metaEqual(expected, actual metav1.ObjectMeta) bool {
return expected.Name == actual.Name &&
expected.Namespace == actual.Namespace &&
reflect.DeepEqual(expected.Labels, actual.Labels) &&
reflect.DeepEqual(expected.Annotations, actual.Annotations) &&
cmp.Equal(expected.Labels, actual.Labels) &&
cmp.Equal(expected.Annotations, actual.Annotations) &&
(len(actual.OwnerReferences) > 0 &&
expected.OwnerReferences[0].APIVersion == actual.OwnerReferences[0].APIVersion &&
expected.OwnerReferences[0].Kind == actual.OwnerReferences[0].Kind &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ package suggestionclient
import (
"errors"
"fmt"
"reflect"
"testing"
"time"

"github.com/golang/mock/gomock"
"github.com/google/go-cmp/cmp"
"github.com/onsi/gomega"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -578,8 +578,8 @@ func TestConvertTrialObservation(t *testing.T) {
}
for _, tc := range tcs {
actualObservation := convertTrialObservation(tc.strategies, tc.inObservation)
if !reflect.DeepEqual(actualObservation, tc.expectedObservation) {
t.Errorf("Case: %v failed.\nExpected observation: %v \ngot: %v", tc.testDescription, tc.expectedObservation, actualObservation)
if diff := cmp.Diff(actualObservation, tc.expectedObservation); diff != "" {
t.Errorf("Case: %v failed.\nDiff (-actual, +expected):\n%s", tc.testDescription, diff)
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/controller.v1beta1/trial/util/job_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ limitations under the License.
package util

import (
"reflect"
"testing"

batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"

"github.com/google/go-cmp/cmp"
trialsv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/trials/v1beta1"
"github.com/kubeflow/katib/pkg/controller.v1beta1/util"
)
Expand Down Expand Up @@ -114,8 +114,8 @@ func TestGetDeployedJobStatus(t *testing.T) {
} else if !tc.err {
if err != nil {
t.Errorf("Case: %v failed. Expected nil, got %v", tc.testDescription, err)
} else if !reflect.DeepEqual(tc.expectedTrialJobStatus, actualTrialJobStatus) {
t.Errorf("Case: %v failed. Expected %v\n got %v", tc.testDescription, tc.expectedTrialJobStatus, actualTrialJobStatus)
} else if diff := cmp.Diff(tc.expectedTrialJobStatus, actualTrialJobStatus); diff != "" {
t.Errorf("Case: %v failed. Diff (-expected, +actual):\n%s", tc.testDescription, diff)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ package sidecarmetricscollector
import (
"os"
"path/filepath"
"reflect"
"testing"
"time"

"github.com/google/go-cmp/cmp"
commonv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/common/v1beta1"
v1beta1 "github.com/kubeflow/katib/pkg/apis/manager/v1beta1"
"github.com/kubeflow/katib/pkg/controller.v1beta1/consts"
Expand Down Expand Up @@ -159,8 +159,8 @@ func TestCollectObservationLog(t *testing.T) {
if (err != nil) != test.err {
t.Errorf("\nGOT: \n%v\nWANT: %v\n", err, test.err)
} else {
if !reflect.DeepEqual(actual, test.expected) {
t.Errorf("Expected %v\n got %v", test.expected, actual)
if diff := cmp.Diff(actual, test.expected); diff != "" {
t.Errorf("Diff (-actual, +expected):\n%s", diff)
}
}
})
Expand Down
4 changes: 2 additions & 2 deletions pkg/suggestion/v1beta1/goptuna/converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ func toGoptunaParams(
}
ir := float64(p)
internalParams[name] = ir
// externalParams[name] = p is prohibited because of reflect.DeepEqual() will be false
// externalParams[name] = p is prohibited because of cmp.Diff() will not be empty
// at findGoptunaTrialIDByParam() function.
externalParams[name] = d.ToExternalRepr(ir)
case goptuna.StepIntUniformDistribution:
Expand All @@ -321,7 +321,7 @@ func toGoptunaParams(
}
ir := float64(p)
internalParams[name] = ir
// externalParams[name] = p is prohibited because of reflect.DeepEqual() will be false
// externalParams[name] = p is prohibited because of cmp.Diff() will not be empty
// at findGoptunaTrialIDByParam() function.
externalParams[name] = d.ToExternalRepr(ir)
case goptuna.CategoricalDistribution:
Expand Down
6 changes: 3 additions & 3 deletions pkg/suggestion/v1beta1/goptuna/converter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ limitations under the License.
package suggestion_goptuna_v1beta1

import (
"reflect"
"testing"

"github.com/c-bata/goptuna"
"github.com/google/go-cmp/cmp"
api_v1_beta1 "github.com/kubeflow/katib/pkg/apis/manager/v1beta1"
)

Expand Down Expand Up @@ -185,8 +185,8 @@ func Test_toGoptunaSearchSpace(t *testing.T) {
t.Errorf("toGoptunaSearchSpace() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("toGoptunaSearchSpace() got = %v, want %v", got, tt.want)
if diff := cmp.Diff(got, tt.want); diff != "" {
t.Errorf("toGoptunaSearchSpace() mismatch (-want +got):\n%s", diff)
}
})
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/suggestion/v1beta1/goptuna/sample.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ package suggestion_goptuna_v1beta1

import (
"fmt"
"reflect"
"strconv"

"github.com/c-bata/goptuna"
"github.com/google/go-cmp/cmp"
api_v1_beta1 "github.com/kubeflow/katib/pkg/apis/manager/v1beta1"
)

Expand Down Expand Up @@ -119,7 +119,7 @@ func findGoptunaTrialIDByParam(study *goptuna.Study, trialMapping map[string]int
continue
}

if reflect.DeepEqual(ktrial.Params, trials[i].Params) {
if diff := cmp.Diff(ktrial.Params, trials[i].Params); diff == "" {
return trials[i].ID, nil
}
}
Expand Down
12 changes: 7 additions & 5 deletions pkg/webhook/v1beta1/pod/inject_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ import (
"context"
"fmt"
"path/filepath"
"reflect"
"sync"
"testing"
"time"

"github.com/google/go-cmp/cmp"
"github.com/onsi/gomega"
appsv1 "k8s.io/api/apps/v1"
batchv1 "k8s.io/api/batch/v1"
Expand Down Expand Up @@ -541,8 +541,10 @@ func TestGetMetricsCollectorArgs(t *testing.T) {
t.Errorf("Case: %v failed. Expected nil, got %v", tc.Name, err)
} else if tc.Err && err == nil {
t.Errorf("Case: %v failed. Expected err, got nil", tc.Name)
} else if !tc.Err && !reflect.DeepEqual(tc.ExpectedArgs, args) {
t.Errorf("Case %v failed. ExpectedArgs: %v, got %v", tc.Name, tc.ExpectedArgs, args)
} else if !tc.Err {
if diff := cmp.Diff(tc.ExpectedArgs, args); diff != "" {
t.Errorf("Case %v failed. Diff (-expected +got): %v", tc.Name, diff)
}
}
}
}
Expand Down Expand Up @@ -1061,8 +1063,8 @@ func TestMutatePodMetadata(t *testing.T) {

for _, tc := range testCases {
mutatePodMetadata(tc.pod, tc.trial)
if !reflect.DeepEqual(tc.mutatedPod, tc.pod) {
t.Errorf("Case %v. Expected Pod %v, got %v", tc.testDescription, tc.mutatedPod, tc.pod)
if diff := cmp.Diff(tc.mutatedPod, tc.pod); diff != "" {
t.Errorf("Case %v failed. Diff (-expected +got): %v", tc.testDescription, diff)
}
}
}

0 comments on commit 7185c18

Please sign in to comment.