From bf17b939e988020d3d7680d9dc91d0b3a4cab38e Mon Sep 17 00:00:00 2001 From: Kevin Diu Date: Wed, 26 Jul 2023 15:28:06 +0900 Subject: [PATCH] Use internal comparator instead of go-cmp (#2132) * use internal comparator instead of go-cmp Signed-off-by: kevindiu * update golangci rule Signed-off-by: kevindiu * Apply suggestions from code review * fix deepsource Signed-off-by: kevindiu --------- Signed-off-by: kevindiu --- .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, 79 insertions(+), 70 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 72c0f5908c..d5b4709ea2 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -149,6 +149,9 @@ linters: - path: internal/errors/errors_benchmark_test\.go linters: - depguard + - path: internal/test/comparator/standard\.go + linters: + - depguard linters-settings: gocritic: enabled-checks: @@ -176,6 +179,10 @@ 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 28624141af..3baba7374c 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 := []cmp.Option{ - cmp.AllowUnexported(*w.want), - cmp.AllowUnexported(*got), - cmp.Comparer(func(want, got *cache[any]) bool { + opts := []comparator.Option{ + comparator.AllowUnexported(*w.want), + comparator.AllowUnexported(*got), + comparator.Comparer(func(want, got *cache[any]) bool { return want.gache != nil && got.gache != nil }), } - if diff := cmp.Diff(w.want, got, opts...); diff != "" { + if diff := comparator.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 e0dbb195d6..1fe3609d2f 100644 --- a/internal/core/algorithm/ngt/ngt_test.go +++ b/internal/core/algorithm/ngt/ngt_test.go @@ -28,7 +28,6 @@ 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" @@ -50,7 +49,7 @@ var ( } searchResultComparator = []comparator.Option{ - comparator.CompareField("Distance", cmp.Comparer(func(s1, s2 float32) bool { + comparator.CompareField("Distance", comparator.Comparer(func(s1, s2 float32) bool { if s1 == 0 { // if vec1 is same as vec2, the distance should be same return s2 == 0 } @@ -103,7 +102,7 @@ func TestNew(t *testing.T) { beforeFunc func(args) afterFunc func(*testing.T, NGT) error } - defaultComprators := append(ngtComparator, comparator.CompareField("idxPath", cmp.Comparer(func(s1, s2 string) bool { + defaultComprators := append(ngtComparator, comparator.CompareField("idxPath", comparator.Comparer(func(s1, s2 string) bool { return s1 == s2 }))) defaultCheckFunc := func(w want, got NGT, err error, comparators ...comparator.Option) error { @@ -142,7 +141,7 @@ func TestNew(t *testing.T) { mu: &sync.RWMutex{}, }, }, - comparators: append(ngtComparator, comparator.CompareField("idxPath", cmp.Comparer(func(s1, s2 string) bool { + comparators: append(ngtComparator, comparator.CompareField("idxPath", comparator.Comparer(func(s1, s2 string) bool { return strings.HasPrefix(s1, "/tmp/ngt-") || strings.HasPrefix(s2, "/tmp/ngt-") }))), } @@ -263,7 +262,7 @@ func TestLoad(t *testing.T) { } // comparator for idxPath - comparators := append(ngtComparator, comparator.CompareField("idxPath", cmp.Comparer(func(s1, s2 string) bool { + comparators := append(ngtComparator, comparator.CompareField("idxPath", comparator.Comparer(func(s1, s2 string) bool { return s1 == s2 }))) @@ -671,7 +670,7 @@ func Test_gen(t *testing.T) { beforeFunc func(*testing.T, args) afterFunc func(*testing.T, NGT) error } - defaultComprators := append(ngtComparator, comparator.CompareField("idxPath", cmp.Comparer(func(s1, s2 string) bool { + defaultComprators := append(ngtComparator, comparator.CompareField("idxPath", comparator.Comparer(func(s1, s2 string) bool { return s1 == s2 }))) defaultCheckFunc := func(_ context.Context, w want, got NGT, err error, comparators ...comparator.Option) error { @@ -709,7 +708,7 @@ func Test_gen(t *testing.T) { mu: &sync.RWMutex{}, }, }, - comparators: append(ngtComparator, comparator.CompareField("idxPath", cmp.Comparer(func(s1, s2 string) bool { + comparators: append(ngtComparator, comparator.CompareField("idxPath", comparator.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 155c33d379..4c68a7d7f3 100644 --- a/internal/db/kvs/redis/redis_test.go +++ b/internal/db/kvs/redis/redis_test.go @@ -26,12 +26,11 @@ 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" ) @@ -565,24 +564,24 @@ func Test_redisClient_newClient(t *testing.T) { got = gotc.Options() ) - opts := []cmp.Option{ - cmpopts.IgnoreUnexported(*want), - cmpopts.IgnoreUnexported(*got), - cmpopts.IgnoreFields(redis.Options{}, "OnConnect"), - cmp.Comparer(func(want, got *tls.Config) bool { + opts := []comparator.Option{ + comparator.IgnoreUnexported(*want), + comparator.IgnoreUnexported(*got), + comparator.IgnoreFields(redis.Options{}, "OnConnect"), + comparator.Comparer(func(want, got *tls.Config) bool { return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer() }), - cmp.Comparer(func(want, got func(ctx context.Context, network, addr string) (net.Conn, error)) bool { + comparator.Comparer(func(want, got func(ctx context.Context, network, addr string) (net.Conn, error)) bool { return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer() }), - cmp.Comparer(func(want, got func(*redis.Conn) error) bool { + comparator.Comparer(func(want, got func(*redis.Conn) error) bool { return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer() }), - cmp.Comparer(func(want, got []redis.Hook) bool { + comparator.Comparer(func(want, got []redis.Hook) bool { return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer() }), } - if diff := cmp.Diff(want, got, opts...); diff != "" { + if diff := comparator.Diff(want, got, opts...); diff != "" { return errors.Errorf("client options diff: %s", diff) } @@ -799,39 +798,39 @@ func Test_redisClient_newClusterClient(t *testing.T) { got = gotc.Options() ) - opts := []cmp.Option{ - cmpopts.IgnoreUnexported(*want), - cmpopts.IgnoreUnexported(*got), - cmp.Comparer(func(want, got func(opt *redis.Options) *redis.Client) bool { + opts := []comparator.Option{ + comparator.IgnoreUnexported(*want), + comparator.IgnoreUnexported(*got), + comparator.Comparer(func(want, got func(opt *redis.Options) *redis.Client) bool { // TODO fix this code later return true }), - cmp.Comparer(func(want, got func(*redis.Client)) bool { + comparator.Comparer(func(want, got func(*redis.Client)) bool { return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer() }), - cmp.Comparer(func(want, got func(context.Context) ([]redis.ClusterSlot, error)) bool { + comparator.Comparer(func(want, got func(context.Context) ([]redis.ClusterSlot, error)) bool { return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer() }), - cmp.Comparer(func(want, got func() ([]redis.ClusterSlot, error)) bool { + comparator.Comparer(func(want, got func() ([]redis.ClusterSlot, error)) bool { return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer() }), - cmp.Comparer(func(want, got func(ctx context.Context, network, addr string) (net.Conn, error)) bool { + comparator.Comparer(func(want, got func(ctx context.Context, network, addr string) (net.Conn, error)) bool { return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer() }), - cmp.Comparer(func(want, got func(context.Context, *redis.Conn) error) bool { + comparator.Comparer(func(want, got func(context.Context, *redis.Conn) error) bool { return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer() }), - cmp.Comparer(func(want, got func(*redis.Conn) error) bool { + comparator.Comparer(func(want, got func(*redis.Conn) error) bool { return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer() }), - cmp.Comparer(func(want, got *tls.Config) bool { + comparator.Comparer(func(want, got *tls.Config) bool { return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer() }), - cmp.Comparer(func(want, got []redis.Hook) bool { + comparator.Comparer(func(want, got []redis.Hook) bool { return reflect.ValueOf(want).Pointer() == reflect.ValueOf(got).Pointer() }), } - if diff := cmp.Diff(want, got, opts...); diff != "" { + if diff := comparator.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 92b931f5b5..2cdf592f95 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 := []cmp.Option{ - cmp.AllowUnexported(*obj), - cmp.AllowUnexported(*w.obj), - cmp.Comparer(func(want, got []backoff.Option) bool { + opts := []comparator.Option{ + comparator.AllowUnexported(*obj), + comparator.AllowUnexported(*w.obj), + comparator.Comparer(func(want, got []backoff.Option) bool { return len(got) == len(want) }), - cmp.Comparer(func(want, got backoff.Option) bool { + comparator.Comparer(func(want, got backoff.Option) bool { return reflect.ValueOf(got).Pointer() == reflect.ValueOf(want).Pointer() }), } - if diff := cmp.Diff(w.obj, obj, opts...); diff != "" { + if diff := comparator.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 bbaeb11eb2..d46f0cd4c6 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 := []cmp.Option{ - cmp.AllowUnexported(*got), - cmp.AllowUnexported(*w.obj), - cmp.Comparer(func(want, got []backoff.Option) bool { + opts := []comparator.Option{ + comparator.AllowUnexported(*got), + comparator.AllowUnexported(*w.obj), + comparator.Comparer(func(want, got []backoff.Option) bool { return len(got) == len(want) }), - cmp.Comparer(func(want, got backoff.Option) bool { + comparator.Comparer(func(want, got backoff.Option) bool { return reflect.ValueOf(got).Pointer() == reflect.ValueOf(want).Pointer() }), } - if diff := cmp.Diff(w.obj, got, opts...); diff != "" { + if diff := comparator.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 { - defaultOptions := []backoff.Option{} + var defaultOptions []backoff.Option opts := []backoff.Option{ backoff.WithRetryCount(1), } diff --git a/internal/net/dialer_test.go b/internal/net/dialer_test.go index efc9a60213..9c2ff0dc33 100644 --- a/internal/net/dialer_test.go +++ b/internal/net/dialer_test.go @@ -29,8 +29,6 @@ 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" @@ -38,6 +36,7 @@ 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" ) @@ -266,18 +265,21 @@ func TestNewDialer(t *testing.T) { return errors.Errorf("got: \"%+v\" is not a dialer", gotDer) } // skipcq: VET-V0008 - if diff := cmp.Diff(*want, *got, + //nolint: govet,copylocks + if diff := comparator.Diff(*want, *got, // skipcq: VET-V0008 - cmpopts.IgnoreFields(*want, "dialer", "der", "addrs", "dnsCachedOnce", "dnsCache", "ctrl", "tmu"), + //nolint: govet,copylocks + comparator.IgnoreFields(*want, "dialer", "der", "addrs", "dnsCachedOnce", "dnsCache", "ctrl", "tmu"), // skipcq: VET-V0008 - cmp.AllowUnexported(*want), - cmp.Comparer(func(x, y cacher.Cache[*dialerCache]) bool { + //nolint: govet,copylocks + comparator.AllowUnexported(*want), + comparator.Comparer(func(x, y cacher.Cache[*dialerCache]) bool { if x == nil && y == nil { return true } return reflect.DeepEqual(x, y) }), - cmp.Comparer(func(x, y *tls.Config) bool { + comparator.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 bb6797afcf..5c6f2eab68 100644 --- a/internal/test/data/request/insert_test.go +++ b/internal/test/data/request/insert_test.go @@ -16,7 +16,6 @@ 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" @@ -24,7 +23,7 @@ import ( "github.com/vdaas/vald/internal/test/goleak" ) -var defaultMultiInsertReqComparators = []cmp.Option{ +var defaultMultiInsertReqComparators = []comparator.Option{ comparator.IgnoreUnexported(payload.Insert_Request{}), comparator.IgnoreUnexported(payload.Insert_MultiRequest{}), comparator.IgnoreUnexported(payload.Object_Vector{}), @@ -32,6 +31,7 @@ var defaultMultiInsertReqComparators = []cmp.Option{ } func TestGenMultiInsertReq(t *testing.T) { + t.Parallel() type args struct { t ObjectType dist vector.Distribution @@ -244,6 +244,7 @@ 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 5d0bb2f0a5..5754bd6fce 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 = []cmp.Option{ +var defaultObjectLocationComparators = []comparator.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,6 +152,7 @@ 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 a767c94e0a..fe1c7e4ab0 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 := []cmp.Option{ - cmp.AllowUnexported(*(w.want).(*queue)), - cmp.Comparer(egComparator), - cmp.Comparer(atomicComparator), - cmp.Comparer(func(want, got chan JobFunc) bool { + opts := []comparator.Option{ + comparator.AllowUnexported(*(w.want).(*queue)), + comparator.Comparer(egComparator), + comparator.Comparer(atomicComparator), + comparator.Comparer(func(want, got chan JobFunc) bool { return len(want) == len(got) }), } - if diff := cmp.Diff(w.want, got, opts...); diff != "" { + if diff := comparator.Diff(w.want, got, opts...); diff != "" { return errors.Errorf("diff = %s", diff) } return nil