Use internal comparator instead of go-cmp (#2132)

* use internal comparator instead of go-cmp Signed-off-by: kevindiu <[email protected]> * update golangci rule Signed-off-by: kevindiu <[email protected]> * Apply suggestions from code review * fix deepsource Signed-off-by: kevindiu <[email protected]> --------- Signed-off-by: kevindiu <[email protected]>
vdaas · Jul 26, 2023 · bf17b93 · bf17b93
1 parent f35756b
commit bf17b93
Show file tree

Hide file tree

Showing 10 changed files with 79 additions and 70 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -29,15 +29,14 @@ 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"
 )
 
@@ -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
@@ -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 = []cmp.Option{
+var defaultMultiInsertReqComparators = []comparator.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
@@ -244,6 +244,7 @@ func TestGenMultiInsertReq(t *testing.T) {
 }
 
 func TestGenSameVecMultiInsertReq(t *testing.T) {
+	t.Parallel()
 	type args struct {
 		num int
 		vec []float32