Browse Source

chore: add more tests (#3338)

Kevin Wan 1 year ago
parent
commit
f6bdb6e1de

+ 0 - 6
core/proc/goroutines+polyfill.go

@@ -1,6 +0,0 @@
-//go:build windows
-
-package proc
-
-func dumpGoroutines() {
-}

+ 12 - 2
core/proc/goroutines.go

@@ -18,7 +18,11 @@ const (
 	debugLevel       = 2
 )
 
-func dumpGoroutines() {
+type creator interface {
+	Create(name string) (file *os.File, err error)
+}
+
+func dumpGoroutines(ctor creator) {
 	command := path.Base(os.Args[0])
 	pid := syscall.Getpid()
 	dumpFile := path.Join(os.TempDir(), fmt.Sprintf("%s-%d-goroutines-%s.dump",
@@ -26,10 +30,16 @@ func dumpGoroutines() {
 
 	logx.Infof("Got dump goroutine signal, printing goroutine profile to %s", dumpFile)
 
-	if f, err := os.Create(dumpFile); err != nil {
+	if f, err := ctor.Create(dumpFile); err != nil {
 		logx.Errorf("Failed to dump goroutine profile, error: %v", err)
 	} else {
 		defer f.Close()
 		pprof.Lookup(goroutineProfile).WriteTo(f, debugLevel)
 	}
 }
+
+type fileCreator struct{}
+
+func (fc fileCreator) Create(name string) (file *os.File, err error) {
+	return os.Create(name)
+}

+ 29 - 3
core/proc/goroutines_test.go

@@ -1,6 +1,10 @@
+//go:build linux || darwin
+
 package proc
 
 import (
+	"errors"
+	"os"
 	"strings"
 	"testing"
 
@@ -9,7 +13,29 @@ import (
 )
 
 func TestDumpGoroutines(t *testing.T) {
-	buf := logtest.NewCollector(t)
-	dumpGoroutines()
-	assert.True(t, strings.Contains(buf.String(), ".dump"))
+	t.Run("real file", func(t *testing.T) {
+		buf := logtest.NewCollector(t)
+		dumpGoroutines(fileCreator{})
+		assert.True(t, strings.Contains(buf.String(), ".dump"))
+	})
+
+	t.Run("fake file", func(t *testing.T) {
+		const msg = "any message"
+		buf := logtest.NewCollector(t)
+		err := errors.New(msg)
+		dumpGoroutines(fakeCreator{
+			file: &os.File{},
+			err:  err,
+		})
+		assert.True(t, strings.Contains(buf.String(), msg))
+	})
+}
+
+type fakeCreator struct {
+	file *os.File
+	err  error
+}
+
+func (fc fakeCreator) Create(name string) (file *os.File, err error) {
+	return fc.file, fc.err
 }

+ 1 - 1
core/proc/signals.go

@@ -26,7 +26,7 @@ func init() {
 			v := <-signals
 			switch v {
 			case syscall.SIGUSR1:
-				dumpGoroutines()
+				dumpGoroutines(fileCreator{})
 			case syscall.SIGUSR2:
 				if profiler == nil {
 					profiler = StartProfile()

+ 2 - 0
core/proc/signals_test.go

@@ -1,3 +1,5 @@
+//go:build linux || darwin
+
 package proc
 
 import (

+ 3 - 3
core/search/tree.go

@@ -171,11 +171,11 @@ func add(nd *node, route string, item any) error {
 		token := route[:i]
 		children := nd.getChildren(token)
 		if child, ok := children[token]; ok {
-			if child != nil {
-				return add(child, route[i+1:], item)
+			if child == nil {
+				return errInvalidState
 			}
 
-			return errInvalidState
+			return add(child, route[i+1:], item)
 		}
 
 		child := newNode(nil)

+ 7 - 1
core/search/tree_test.go

@@ -11,7 +11,7 @@ import (
 
 type mockedRoute struct {
 	route string
-	value int
+	value any
 }
 
 func TestSearch(t *testing.T) {
@@ -187,6 +187,12 @@ func TestSearchInvalidItem(t *testing.T) {
 	assert.Equal(t, errEmptyItem, err)
 }
 
+func TestSearchInvalidState(t *testing.T) {
+	nd := newNode("0")
+	nd.children[0]["1"] = nil
+	assert.Error(t, add(nd, "1/2", "2"))
+}
+
 func BenchmarkSearchTree(b *testing.B) {
 	const (
 		avgLen  = 1000

+ 4 - 5
core/stores/sqlc/cachedsql.go

@@ -97,6 +97,9 @@ func (cc CachedConn) Exec(exec ExecFn, keys ...string) (sql.Result, error) {
 }
 
 // ExecCtx runs given exec on given keys, and returns execution result.
+// If DB operation succeeds, it will delete cache with given keys,
+// if DB operation fails, it will return nil result and non-nil error,
+// if DB operation succeeds but cache deletion fails, it will return result and non-nil error.
 func (cc CachedConn) ExecCtx(ctx context.Context, exec ExecCtxFn, keys ...string) (
 	sql.Result, error) {
 	res, err := exec(ctx, cc.db)
@@ -104,11 +107,7 @@ func (cc CachedConn) ExecCtx(ctx context.Context, exec ExecCtxFn, keys ...string
 		return nil, err
 	}
 
-	if err := cc.DelCacheCtx(ctx, keys...); err != nil {
-		return nil, err
-	}
-
-	return res, nil
+	return res, cc.DelCacheCtx(ctx, keys...)
 }
 
 // ExecNoCache runs exec with given sql statement, without affecting cache.

+ 27 - 25
core/stores/sqlc/cachedsql_test.go

@@ -471,31 +471,33 @@ func TestCachedConnExec(t *testing.T) {
 }
 
 func TestCachedConnExecDropCache(t *testing.T) {
-	r, err := miniredis.Run()
-	assert.Nil(t, err)
-	defer fx.DoWithTimeout(func() error {
-		r.Close()
-		return nil
-	}, time.Second)
-
-	const (
-		key   = "user"
-		value = "any"
-	)
-	var conn trackedConn
-	c := NewNodeConn(&conn, redis.New(r.Addr()), cache.WithExpiry(time.Second*30))
-	assert.Nil(t, c.SetCache(key, value))
-	_, err = c.Exec(func(conn sqlx.SqlConn) (result sql.Result, e error) {
-		return conn.Exec("delete from user_table where id='kevin'")
-	}, key)
-	assert.Nil(t, err)
-	assert.True(t, conn.execValue)
-	_, err = r.Get(key)
-	assert.Exactly(t, miniredis.ErrKeyNotFound, err)
-	_, err = c.Exec(func(conn sqlx.SqlConn) (result sql.Result, e error) {
-		return nil, errors.New("foo")
-	}, key)
-	assert.NotNil(t, err)
+	t.Run("drop cache", func(t *testing.T) {
+		r, err := miniredis.Run()
+		assert.Nil(t, err)
+		defer fx.DoWithTimeout(func() error {
+			r.Close()
+			return nil
+		}, time.Second)
+
+		const (
+			key   = "user"
+			value = "any"
+		)
+		var conn trackedConn
+		c := NewNodeConn(&conn, redis.New(r.Addr()), cache.WithExpiry(time.Second*30))
+		assert.Nil(t, c.SetCache(key, value))
+		_, err = c.Exec(func(conn sqlx.SqlConn) (result sql.Result, e error) {
+			return conn.Exec("delete from user_table where id='kevin'")
+		}, key)
+		assert.Nil(t, err)
+		assert.True(t, conn.execValue)
+		_, err = r.Get(key)
+		assert.Exactly(t, miniredis.ErrKeyNotFound, err)
+		_, err = c.Exec(func(conn sqlx.SqlConn) (result sql.Result, e error) {
+			return nil, errors.New("foo")
+		}, key)
+		assert.NotNil(t, err)
+	})
 }
 
 func TestCachedConn_SetCacheWithExpire(t *testing.T) {

+ 2 - 1
core/syncx/resourcemanager.go

@@ -42,7 +42,8 @@ func (manager *ResourceManager) Close() error {
 }
 
 // GetResource returns the resource associated with given key.
-func (manager *ResourceManager) GetResource(key string, create func() (io.Closer, error)) (io.Closer, error) {
+func (manager *ResourceManager) GetResource(key string, create func() (io.Closer, error)) (
+	io.Closer, error) {
 	val, err := manager.singleFlight.Do(key, func() (any, error) {
 		manager.lock.RLock()
 		resource, ok := manager.resources[key]

+ 13 - 0
internal/health/health_test.go

@@ -53,6 +53,19 @@ func TestComboHealthManager(t *testing.T) {
 		assert.True(t, chm.IsReady())
 	})
 
+	t.Run("is ready verbose", func(t *testing.T) {
+		chm := newComboHealthManager()
+		hm := NewHealthManager(probeName)
+
+		assert.True(t, chm.IsReady())
+		chm.addProbe(hm)
+		assert.False(t, chm.IsReady())
+		hm.MarkReady()
+		assert.True(t, chm.IsReady())
+		assert.Contains(t, chm.verboseInfo(), probeName)
+		assert.Contains(t, chm.verboseInfo(), "is ready")
+	})
+
 	t.Run("concurrent add probes", func(t *testing.T) {
 		chm := newComboHealthManager()