Skip to content

Commit

Permalink
Revert "Use internal comparator instead of go-cmp (#2132)"
Browse files Browse the repository at this point in the history
This reverts commit bf17b93.
  • Loading branch information
ykadowak committed Nov 30, 2023
1 parent 6dc6bcf commit 4044118
Show file tree
Hide file tree
Showing 10 changed files with 70 additions and 79 deletions.
7 changes: 0 additions & 7 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,6 @@ linters:
- path: internal/errors/errors_benchmark_test\.go
linters:
- depguard
- path: internal/test/comparator/standard\.go
linters:
- depguard
linters-settings:
gocritic:
enabled-checks:
Expand Down Expand Up @@ -179,10 +176,6 @@ linters-settings:
desc: "errors is allowed only by internal/errors"
- pkg: github.com/go-errors/errors
desc: "errors is allowed only by internal/errors"
- pkg: github.com/google/go-cmp/cmp
desc: "cmp is allowed only by internal/test/comparator"
- pkg: github.com/google/go-cmp/cmp/cmpopts
desc: "cmpopts is allowed only by internal/test/comparator"
govet:
check-shadowing: true
enable-all: true
12 changes: 6 additions & 6 deletions internal/cache/gache/option_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ import (
"testing"
"time"

"github.com/google/go-cmp/cmp"
gache "github.com/kpango/gache/v2"
"github.com/vdaas/vald/internal/errors"
"github.com/vdaas/vald/internal/test/comparator"
"github.com/vdaas/vald/internal/test/goleak"
)

Expand All @@ -45,14 +45,14 @@ func TestDefaultOptions(t *testing.T) {
}

defaultCheckFunc := func(w want, got *cache[any]) error {
opts := []comparator.Option{
comparator.AllowUnexported(*w.want),
comparator.AllowUnexported(*got),
comparator.Comparer(func(want, got *cache[any]) bool {
opts := []cmp.Option{
cmp.AllowUnexported(*w.want),
cmp.AllowUnexported(*got),
cmp.Comparer(func(want, got *cache[any]) bool {
return want.gache != nil && got.gache != nil
}),
}
if diff := comparator.Diff(w.want, got, opts...); diff != "" {
if diff := cmp.Diff(w.want, got, opts...); diff != "" {
return errors.Errorf("got = %v, want = %v", got, w.want)
}
return nil
Expand Down
13 changes: 7 additions & 6 deletions internal/core/algorithm/ngt/ngt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"sync"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/vdaas/vald/internal/core/algorithm"
"github.com/vdaas/vald/internal/errors"
"github.com/vdaas/vald/internal/file"
Expand All @@ -49,7 +50,7 @@ var (
}

searchResultComparator = []comparator.Option{
comparator.CompareField("Distance", comparator.Comparer(func(s1, s2 float32) bool {
comparator.CompareField("Distance", cmp.Comparer(func(s1, s2 float32) bool {
if s1 == 0 { // if vec1 is same as vec2, the distance should be same
return s2 == 0
}
Expand Down Expand Up @@ -102,7 +103,7 @@ func TestNew(t *testing.T) {
beforeFunc func(args)
afterFunc func(*testing.T, NGT) error
}
defaultComprators := append(ngtComparator, comparator.CompareField("idxPath", comparator.Comparer(func(s1, s2 string) bool {
defaultComprators := append(ngtComparator, comparator.CompareField("idxPath", cmp.Comparer(func(s1, s2 string) bool {
return s1 == s2
})))
defaultCheckFunc := func(w want, got NGT, err error, comparators ...comparator.Option) error {
Expand Down Expand Up @@ -141,7 +142,7 @@ func TestNew(t *testing.T) {
mu: &sync.RWMutex{},
},
},
comparators: append(ngtComparator, comparator.CompareField("idxPath", comparator.Comparer(func(s1, s2 string) bool {
comparators: append(ngtComparator, comparator.CompareField("idxPath", cmp.Comparer(func(s1, s2 string) bool {
return strings.HasPrefix(s1, "/tmp/ngt-") || strings.HasPrefix(s2, "/tmp/ngt-")
}))),
}
Expand Down Expand Up @@ -262,7 +263,7 @@ func TestLoad(t *testing.T) {
}

// comparator for idxPath
comparators := append(ngtComparator, comparator.CompareField("idxPath", comparator.Comparer(func(s1, s2 string) bool {
comparators := append(ngtComparator, comparator.CompareField("idxPath", cmp.Comparer(func(s1, s2 string) bool {
return s1 == s2
})))

Expand Down Expand Up @@ -670,7 +671,7 @@ func Test_gen(t *testing.T) {
beforeFunc func(*testing.T, args)
afterFunc func(*testing.T, NGT) error
}
defaultComprators := append(ngtComparator, comparator.CompareField("idxPath", comparator.Comparer(func(s1, s2 string) bool {
defaultComprators := append(ngtComparator, comparator.CompareField("idxPath", cmp.Comparer(func(s1, s2 string) bool {
return s1 == s2
})))
defaultCheckFunc := func(_ context.Context, w want, got NGT, err error, comparators ...comparator.Option) error {
Expand Down Expand Up @@ -708,7 +709,7 @@ func Test_gen(t *testing.T) {
mu: &sync.RWMutex{},
},
},
comparators: append(ngtComparator, comparator.CompareField("idxPath", comparator.Comparer(func(s1, s2 string) bool {
comparators: append(ngtComparator, comparator.CompareField("idxPath", cmp.Comparer(func(s1, s2 string) bool {
return strings.HasPrefix(s1, "/tmp/ngt-") || strings.HasPrefix(s2, "/tmp/ngt-")
}))),
},
Expand Down
47 changes: 24 additions & 23 deletions internal/db/kvs/redis/redis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,12 @@ import (
"time"

redis "github.com/go-redis/redis/v8"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/vdaas/vald/internal/errors"
"github.com/vdaas/vald/internal/log"
"github.com/vdaas/vald/internal/log/logger"
"github.com/vdaas/vald/internal/net"
"github.com/vdaas/vald/internal/test/comparator"
"github.com/vdaas/vald/internal/test/goleak"
)

Expand Down Expand Up @@ -564,24 +565,24 @@ func Test_redisClient_newClient(t *testing.T) {
got = gotc.Options()
)

opts := []comparator.Option{
comparator.IgnoreUnexported(*want),
comparator.IgnoreUnexported(*got),
comparator.IgnoreFields(redis.Options{}, "OnConnect"),
comparator.Comparer(func(want, got *tls.Config) bool {
opts := []cmp.Option{
cmpopts.IgnoreUnexported(*want),
cmpopts.IgnoreUnexported(*got),
cmpopts.IgnoreFields(redis.Options{}, "OnConnect"),
cmp.Comparer(func(want, got *tls.Config) bool {
return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer()
}),
comparator.Comparer(func(want, got func(ctx context.Context, network, addr string) (net.Conn, error)) bool {
cmp.Comparer(func(want, got func(ctx context.Context, network, addr string) (net.Conn, error)) bool {
return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer()
}),
comparator.Comparer(func(want, got func(*redis.Conn) error) bool {
cmp.Comparer(func(want, got func(*redis.Conn) error) bool {
return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer()
}),
comparator.Comparer(func(want, got []redis.Hook) bool {
cmp.Comparer(func(want, got []redis.Hook) bool {
return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer()
}),
}
if diff := comparator.Diff(want, got, opts...); diff != "" {
if diff := cmp.Diff(want, got, opts...); diff != "" {
return errors.Errorf("client options diff: %s", diff)
}

Expand Down Expand Up @@ -798,39 +799,39 @@ func Test_redisClient_newClusterClient(t *testing.T) {
got = gotc.Options()
)

opts := []comparator.Option{
comparator.IgnoreUnexported(*want),
comparator.IgnoreUnexported(*got),
comparator.Comparer(func(want, got func(opt *redis.Options) *redis.Client) bool {
opts := []cmp.Option{
cmpopts.IgnoreUnexported(*want),
cmpopts.IgnoreUnexported(*got),
cmp.Comparer(func(want, got func(opt *redis.Options) *redis.Client) bool {
// TODO fix this code later
return true
}),
comparator.Comparer(func(want, got func(*redis.Client)) bool {
cmp.Comparer(func(want, got func(*redis.Client)) bool {
return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer()
}),
comparator.Comparer(func(want, got func(context.Context) ([]redis.ClusterSlot, error)) bool {
cmp.Comparer(func(want, got func(context.Context) ([]redis.ClusterSlot, error)) bool {
return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer()
}),
comparator.Comparer(func(want, got func() ([]redis.ClusterSlot, error)) bool {
cmp.Comparer(func(want, got func() ([]redis.ClusterSlot, error)) bool {
return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer()
}),
comparator.Comparer(func(want, got func(ctx context.Context, network, addr string) (net.Conn, error)) bool {
cmp.Comparer(func(want, got func(ctx context.Context, network, addr string) (net.Conn, error)) bool {
return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer()
}),
comparator.Comparer(func(want, got func(context.Context, *redis.Conn) error) bool {
cmp.Comparer(func(want, got func(context.Context, *redis.Conn) error) bool {
return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer()
}),
comparator.Comparer(func(want, got func(*redis.Conn) error) bool {
cmp.Comparer(func(want, got func(*redis.Conn) error) bool {
return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer()
}),
comparator.Comparer(func(want, got *tls.Config) bool {
cmp.Comparer(func(want, got *tls.Config) bool {
return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer()
}),
comparator.Comparer(func(want, got []redis.Hook) bool {
cmp.Comparer(func(want, got []redis.Hook) bool {
return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer()
}),
}
if diff := comparator.Diff(want, got, opts...); diff != "" {
if diff := cmp.Diff(want, got, opts...); diff != "" {
fmt.Println(diff)
return errors.Errorf("got = %v, want = %v", got, want)
}
Expand Down
14 changes: 7 additions & 7 deletions internal/db/storage/blob/s3/option_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ import (
"testing"

"github.com/aws/aws-sdk-go/aws/session"
"github.com/google/go-cmp/cmp"
"github.com/vdaas/vald/internal/backoff"
"github.com/vdaas/vald/internal/db/storage/blob/s3/reader"
"github.com/vdaas/vald/internal/db/storage/blob/s3/writer"
"github.com/vdaas/vald/internal/errgroup"
"github.com/vdaas/vald/internal/errors"
"github.com/vdaas/vald/internal/test/comparator"
"github.com/vdaas/vald/internal/test/goleak"
)

Expand Down Expand Up @@ -561,17 +561,17 @@ func TestWithReaderBackoffOpts(t *testing.T) {
return errors.Errorf("got_error: \"%#v\",\n\t\t\t\twant: \"%#v\"", err, w.err)
}

opts := []comparator.Option{
comparator.AllowUnexported(*obj),
comparator.AllowUnexported(*w.obj),
comparator.Comparer(func(want, got []backoff.Option) bool {
opts := []cmp.Option{
cmp.AllowUnexported(*obj),
cmp.AllowUnexported(*w.obj),
cmp.Comparer(func(want, got []backoff.Option) bool {
return len(got) == len(want)
}),
comparator.Comparer(func(want, got backoff.Option) bool {
cmp.Comparer(func(want, got backoff.Option) bool {
return reflect.ValueOf(got).Pointer() == reflect.ValueOf(want).Pointer()
}),
}
if diff := comparator.Diff(w.obj, obj, opts...); diff != "" {
if diff := cmp.Diff(w.obj, obj, opts...); diff != "" {
return errors.Errorf("got: \"%#v\",\n\t\t\t\twant: \"%#v\"", obj, w.obj)
}

Expand Down
16 changes: 8 additions & 8 deletions internal/db/storage/blob/s3/reader/option_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ import (
"testing"

"github.com/aws/aws-sdk-go/service/s3"
"github.com/google/go-cmp/cmp"
"github.com/vdaas/vald/internal/backoff"
"github.com/vdaas/vald/internal/db/storage/blob/s3/sdk/s3/s3iface"
"github.com/vdaas/vald/internal/errgroup"
"github.com/vdaas/vald/internal/errors"
"github.com/vdaas/vald/internal/test/comparator"
"github.com/vdaas/vald/internal/test/goleak"
)

Expand Down Expand Up @@ -402,17 +402,17 @@ func TestWithBackoffOpts(t *testing.T) {
afterFunc func(args, *T)
}
defaultCheckFunc := func(w want, got *T) error {
opts := []comparator.Option{
comparator.AllowUnexported(*got),
comparator.AllowUnexported(*w.obj),
comparator.Comparer(func(want, got []backoff.Option) bool {
opts := []cmp.Option{
cmp.AllowUnexported(*got),
cmp.AllowUnexported(*w.obj),
cmp.Comparer(func(want, got []backoff.Option) bool {
return len(got) == len(want)
}),
comparator.Comparer(func(want, got backoff.Option) bool {
cmp.Comparer(func(want, got backoff.Option) bool {
return reflect.ValueOf(got).Pointer() == reflect.ValueOf(want).Pointer()
}),
}
if diff := comparator.Diff(w.obj, got, opts...); diff != "" {
if diff := cmp.Diff(w.obj, got, opts...); diff != "" {
return errors.Errorf("got: \"%#v\",\n\t\t\t\twant: \"%#v\"", got, w.obj)
}
return nil
Expand All @@ -435,7 +435,7 @@ func TestWithBackoffOpts(t *testing.T) {
}
}(),
func() test {
var defaultOptions []backoff.Option
defaultOptions := []backoff.Option{}
opts := []backoff.Option{
backoff.WithRetryCount(1),
}
Expand Down
16 changes: 7 additions & 9 deletions internal/net/dialer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,15 @@ import (
"testing"
"time"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/vdaas/vald/internal/cache"
"github.com/vdaas/vald/internal/cache/cacher"
"github.com/vdaas/vald/internal/cache/gache"
"github.com/vdaas/vald/internal/conv"
"github.com/vdaas/vald/internal/errors"
"github.com/vdaas/vald/internal/io"
"github.com/vdaas/vald/internal/strings"
"github.com/vdaas/vald/internal/test/comparator"
"github.com/vdaas/vald/internal/tls"
)

Expand Down Expand Up @@ -265,21 +266,18 @@ func TestNewDialer(t *testing.T) {
return errors.Errorf("got: \"%+v\" is not a dialer", gotDer)
}
// skipcq: VET-V0008
//nolint: govet,copylocks
if diff := comparator.Diff(*want, *got,
if diff := cmp.Diff(*want, *got,
// skipcq: VET-V0008
//nolint: govet,copylocks
comparator.IgnoreFields(*want, "dialer", "der", "addrs", "dnsCachedOnce", "dnsCache", "ctrl", "tmu"),
cmpopts.IgnoreFields(*want, "dialer", "der", "addrs", "dnsCachedOnce", "dnsCache", "ctrl", "tmu"),
// skipcq: VET-V0008
//nolint: govet,copylocks
comparator.AllowUnexported(*want),
comparator.Comparer(func(x, y cacher.Cache[*dialerCache]) bool {
cmp.AllowUnexported(*want),
cmp.Comparer(func(x, y cacher.Cache[*dialerCache]) bool {
if x == nil && y == nil {
return true
}
return reflect.DeepEqual(x, y)
}),
comparator.Comparer(func(x, y *tls.Config) bool {
cmp.Comparer(func(x, y *tls.Config) bool {
return reflect.DeepEqual(x, y)
}),
); diff != "" {
Expand Down
5 changes: 2 additions & 3 deletions internal/test/data/request/insert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,22 @@ package request
import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/vdaas/vald/apis/grpc/v1/payload"
"github.com/vdaas/vald/internal/errors"
"github.com/vdaas/vald/internal/test/comparator"
"github.com/vdaas/vald/internal/test/data/vector"
"github.com/vdaas/vald/internal/test/goleak"
)

var defaultMultiInsertReqComparators = []comparator.Option{
var defaultMultiInsertReqComparators = []cmp.Option{
comparator.IgnoreUnexported(payload.Insert_Request{}),
comparator.IgnoreUnexported(payload.Insert_MultiRequest{}),
comparator.IgnoreUnexported(payload.Object_Vector{}),
comparator.IgnoreUnexported(payload.Insert_Config{}),
}

func TestGenMultiInsertReq(t *testing.T) {
t.Parallel()
type args struct {
t ObjectType
dist vector.Distribution
Expand Down Expand Up @@ -244,7 +244,6 @@ func TestGenMultiInsertReq(t *testing.T) {
}

func TestGenSameVecMultiInsertReq(t *testing.T) {
t.Parallel()
type args struct {
num int
vec []float32
Expand Down
Loading

0 comments on commit 4044118

Please sign in to comment.