From 380c99e5397deca43064d901f73ec8c00aaf1899 Mon Sep 17 00:00:00 2001
From: Kevin Diu <kevin_diu@yahoo.com.hk>
Date: Wed, 15 Jul 2020 15:58:28 +0900
Subject: [PATCH] :white_check_mark: add cache test (#576)

* add cache test

* Update internal/cache/cache_test.go

Co-authored-by: Kiichiro YUKAWA <kyukawa315@gmail.com>

* fix

Co-authored-by: Kiichiro YUKAWA <kyukawa315@gmail.com>
---
 internal/cache/cache.go      |  7 ++-
 internal/cache/cache_test.go | 82 +++++++++++++++++++++++++-----------
 internal/cache/option.go     | 32 ++++++--------
 3 files changed, 72 insertions(+), 49 deletions(-)

diff --git a/internal/cache/cache.go b/internal/cache/cache.go
index 49ed3ae461..41c9e4a345 100644
--- a/internal/cache/cache.go
+++ b/internal/cache/cache.go
@@ -26,6 +26,7 @@ import (
 	"github.com/vdaas/vald/internal/errors"
 )
 
+// Cache represent the cache interface to store cache
 type Cache interface {
 	Start(context.Context)
 	Get(string) (interface{}, bool)
@@ -41,13 +42,11 @@ type cache struct {
 	expiredHook    func(context.Context, string)
 }
 
+// New returns the Cache instance or error
 func New(opts ...Option) (cc Cache, err error) {
 	c := new(cache)
 	for _, opt := range append(defaultOpts, opts...) {
-		err = opt(c)
-		if err != nil {
-			return nil, err
-		}
+		opt(c)
 	}
 	switch c.cacher {
 	case cacher.GACHE:
diff --git a/internal/cache/cache_test.go b/internal/cache/cache_test.go
index 1e37531b4d..daf7e431cf 100644
--- a/internal/cache/cache_test.go
+++ b/internal/cache/cache_test.go
@@ -26,6 +26,13 @@ import (
 	"go.uber.org/goleak"
 )
 
+var (
+	// Goroutine leak is detected by `fastime`, but it should be ignored in the test because it is an external package.
+	goleakIgnoreOptions = []goleak.Option{
+		goleak.IgnoreTopFunction("github.com/kpango/fastime.(*Fastime).StartTimerD.func1"),
+	}
+)
+
 func TestNew(t *testing.T) {
 	type args struct {
 		opts []Option
@@ -52,36 +59,61 @@ func TestNew(t *testing.T) {
 		return nil
 	}
 	tests := []test{
-		// TODO test cases
-		/*
-		   {
-		       name: "test_case_1",
-		       args: args {
-		           opts: nil,
-		       },
-		       want: want{},
-		       checkFunc: defaultCheckFunc,
-		   },
-		*/
+		{
+			name: "return gache cacher",
+			args: args{
+				opts: []Option{WithType("gache")},
+			},
+			checkFunc: func(w want, got Cache, err error) error {
+				if err != nil {
+					return err
+				}
+				if got == nil {
+					return errors.New("got cache is nil")
+				}
+
+				return nil
+			},
+		},
+		{
+			name: "return unknown error when type is unknown",
+			args: args{
+				opts: []Option{WithType("unknown")},
+			},
+			want: want{
+				err: errors.ErrInvalidCacherType,
+			},
+		},
+		{
+			name: "return cache when type is empty",
+			args: args{
+				opts: []Option{WithType("")},
+			},
+			checkFunc: func(w want, got Cache, err error) error {
+				if err != nil {
+					return err
+				}
+				if got == nil {
+					return errors.New("got cache is nil")
+				}
 
-		// TODO test cases
-		/*
-		   func() test {
-		       return test {
-		           name: "test_case_2",
-		           args: args {
-		           opts: nil,
-		           },
-		           want: want{},
-		           checkFunc: defaultCheckFunc,
-		       }
-		   }(),
-		*/
+				return nil
+			},
+		},
+		{
+			name: "return unknown error when type is dummy string",
+			args: args{
+				opts: []Option{WithType("dummy")},
+			},
+			want: want{
+				err: errors.ErrInvalidCacherType,
+			},
+		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(tt *testing.T) {
-			defer goleak.VerifyNone(t)
+			defer goleak.VerifyNone(tt, goleakIgnoreOptions...)
 			if test.beforeFunc != nil {
 				test.beforeFunc(test.args)
 			}
diff --git a/internal/cache/option.go b/internal/cache/option.go
index b33490e8f1..18136535f7 100644
--- a/internal/cache/option.go
+++ b/internal/cache/option.go
@@ -21,11 +21,10 @@ import (
 	"context"
 
 	"github.com/vdaas/vald/internal/cache/cacher"
-	"github.com/vdaas/vald/internal/errors"
 	"github.com/vdaas/vald/internal/timeutil"
 )
 
-type Option func(*cache) error
+type Option func(*cache)
 
 var (
 	defaultOpts = []Option{
@@ -36,52 +35,45 @@ var (
 )
 
 func WithExpiredHook(f func(context.Context, string)) Option {
-	return func(c *cache) error {
+	return func(c *cache) {
 		if f != nil {
 			c.expiredHook = f
 		}
-		return nil
 	}
 }
 
 func WithType(mo string) Option {
-	return func(c *cache) error {
+	return func(c *cache) {
 		if len(mo) == 0 {
-			return nil
+			return
 		}
-		m := cacher.ToType(mo)
-		if m == cacher.Unknown {
-			return errors.ErrInvalidCacherType
-		}
-		c.cacher = m
-		return nil
+
+		c.cacher = cacher.ToType(mo)
 	}
 }
 
 func WithExpireDuration(dur string) Option {
-	return func(c *cache) error {
+	return func(c *cache) {
 		if len(dur) == 0 {
-			return nil
+			return
 		}
 		d, err := timeutil.Parse(dur)
 		if err != nil {
-			return nil
+			return
 		}
 		c.expireDur = d
-		return nil
 	}
 }
 
 func WithExpireCheckDuration(dur string) Option {
-	return func(c *cache) error {
+	return func(c *cache) {
 		if len(dur) == 0 {
-			return nil
+			return
 		}
 		d, err := timeutil.Parse(dur)
 		if err != nil {
-			return nil
+			return
 		}
 		c.expireCheckDur = d
-		return nil
 	}
 }