From 7732afbf7f797f0a946a6cb728826117577a1139 Mon Sep 17 00:00:00 2001 From: ykadowak Date: Thu, 30 Nov 2023 02:46:08 +0000 Subject: [PATCH] Revert "Use internal comparator instead of go-cmp (#2132)" This reverts commit bf17b939e988020d3d7680d9dc91d0b3a4cab38e. --- .golangci.yml | 7 --- internal/cache/gache/option_test.go | 12 ++--- internal/core/algorithm/ngt/ngt_test.go | 13 ++--- internal/db/kvs/redis/redis_test.go | 47 ++++++++++--------- internal/db/storage/blob/s3/option_test.go | 14 +++--- .../db/storage/blob/s3/reader/option_test.go | 16 +++---- internal/net/dialer_test.go | 16 +++---- internal/test/data/request/insert_test.go | 5 +- internal/test/data/request/object_test.go | 5 +- internal/worker/queue_test.go | 14 +++--- 10 files changed, 70 insertions(+), 79 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index d5b4709ea2..72c0f5908c 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -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: @@ -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 diff --git a/internal/cache/gache/option_test.go b/internal/cache/gache/option_test.go index 3baba7374c..28624141af 100644 --- a/internal/cache/gache/option_test.go +++ b/internal/cache/gache/option_test.go @@ -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" ) @@ -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 diff --git a/internal/core/algorithm/ngt/ngt_test.go b/internal/core/algorithm/ngt/ngt_test.go index 1fe3609d2f..e0dbb195d6 100644 --- a/internal/core/algorithm/ngt/ngt_test.go +++ b/internal/core/algorithm/ngt/ngt_test.go @@ -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" @@ -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 } @@ -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 { @@ -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-") }))), } @@ -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 }))) @@ -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 { @@ -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-") }))), }, diff --git a/internal/db/kvs/redis/redis_test.go b/internal/db/kvs/redis/redis_test.go index 4c68a7d7f3..155c33d379 100644 --- a/internal/db/kvs/redis/redis_test.go +++ b/internal/db/kvs/redis/redis_test.go @@ -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" ) @@ -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) } @@ -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) } diff --git a/internal/db/storage/blob/s3/option_test.go b/internal/db/storage/blob/s3/option_test.go index 2cdf592f95..92b931f5b5 100644 --- a/internal/db/storage/blob/s3/option_test.go +++ b/internal/db/storage/blob/s3/option_test.go @@ -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" ) @@ -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) } diff --git a/internal/db/storage/blob/s3/reader/option_test.go b/internal/db/storage/blob/s3/reader/option_test.go index d46f0cd4c6..bbaeb11eb2 100644 --- a/internal/db/storage/blob/s3/reader/option_test.go +++ b/internal/db/storage/blob/s3/reader/option_test.go @@ -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" ) @@ -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 @@ -435,7 +435,7 @@ func TestWithBackoffOpts(t *testing.T) { } }(), func() test { - var defaultOptions []backoff.Option + defaultOptions := []backoff.Option{} opts := []backoff.Option{ backoff.WithRetryCount(1), } diff --git a/internal/net/dialer_test.go b/internal/net/dialer_test.go index 9c2ff0dc33..efc9a60213 100644 --- a/internal/net/dialer_test.go +++ b/internal/net/dialer_test.go @@ -29,6 +29,8 @@ 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" @@ -36,7 +38,6 @@ import ( "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" ) @@ -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 != "" { diff --git a/internal/test/data/request/insert_test.go b/internal/test/data/request/insert_test.go index 5c6f2eab68..bb6797afcf 100644 --- a/internal/test/data/request/insert_test.go +++ b/internal/test/data/request/insert_test.go @@ -16,6 +16,7 @@ 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" @@ -23,7 +24,7 @@ import ( "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{}), @@ -31,7 +32,6 @@ var defaultMultiInsertReqComparators = []comparator.Option{ } func TestGenMultiInsertReq(t *testing.T) { - t.Parallel() type args struct { t ObjectType dist vector.Distribution @@ -244,7 +244,6 @@ func TestGenMultiInsertReq(t *testing.T) { } func TestGenSameVecMultiInsertReq(t *testing.T) { - t.Parallel() type args struct { num int vec []float32 diff --git a/internal/test/data/request/object_test.go b/internal/test/data/request/object_test.go index 5754bd6fce..5d0bb2f0a5 100644 --- a/internal/test/data/request/object_test.go +++ b/internal/test/data/request/object_test.go @@ -17,19 +17,19 @@ import ( "reflect" "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/goleak" ) -var defaultObjectLocationComparators = []comparator.Option{ +var defaultObjectLocationComparators = []cmp.Option{ comparator.IgnoreUnexported(payload.Object_Locations{}), comparator.IgnoreUnexported(payload.Object_Location{}), } func TestGenObjectLocations(t *testing.T) { - t.Parallel() type args struct { num int name string @@ -152,7 +152,6 @@ func TestGenObjectLocations(t *testing.T) { } func TestGenObjectStreamLocation(t *testing.T) { - t.Parallel() type args struct { num int name string diff --git a/internal/worker/queue_test.go b/internal/worker/queue_test.go index fe1c7e4ab0..a767c94e0a 100644 --- a/internal/worker/queue_test.go +++ b/internal/worker/queue_test.go @@ -24,9 +24,9 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" "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" ) @@ -72,15 +72,15 @@ func TestNewQueue(t *testing.T) { atomicComparator := func(want, got atomic.Value) bool { return reflect.DeepEqual(want.Load(), got.Load()) } - opts := []comparator.Option{ - comparator.AllowUnexported(*(w.want).(*queue)), - comparator.Comparer(egComparator), - comparator.Comparer(atomicComparator), - comparator.Comparer(func(want, got chan JobFunc) bool { + opts := []cmp.Option{ + cmp.AllowUnexported(*(w.want).(*queue)), + cmp.Comparer(egComparator), + cmp.Comparer(atomicComparator), + cmp.Comparer(func(want, got chan JobFunc) bool { return len(want) == len(got) }), } - if diff := comparator.Diff(w.want, got, opts...); diff != "" { + if diff := cmp.Diff(w.want, got, opts...); diff != "" { return errors.Errorf("diff = %s", diff) } return nil