Increase covd coverage for paramtable (#7583)

Signed-off-by: Ncai.zhang <cai.zhang@zilliz.com>

internal/indexcoord/index_coord.go

@@ -106,6 +106,7 @@ func (i *IndexCoord) Register() error {
 }
 
 func (i *IndexCoord) Init() error {
+	Params.Init()
 	log.Debug("IndexCoord", zap.Any("etcd endpoints", Params.EtcdEndpoints))
 	i.UpdateStateCode(internalpb.StateCode_Initializing)
 

internal/indexcoord/paramtable.go

@@ -27,8 +27,6 @@ type ParamTable struct {
 	Address string
 	Port    int
 
-	RootCoordAddress string
-
 	EtcdEndpoints []string
 	KvRootPath    string
 	MetaRootPath  string
@@ -50,7 +48,6 @@ func (pt *ParamTable) Init() {
 		pt.BaseTable.Init()
 		pt.initLogCfg()
 		pt.initEtcdEndpoints()
-		pt.initRootCoordAddress()
 		pt.initMetaRootPath()
 		pt.initKvRootPath()
 		pt.initMinIOAddress()
@@ -93,14 +90,6 @@ func (pt *ParamTable) initKvRootPath() {
 	pt.KvRootPath = rootPath + "/" + subPath
 }
 
-func (pt *ParamTable) initRootCoordAddress() {
-	ret, err := pt.Load("_RootCoordAddress")
-	if err != nil {
-		panic(err)
-	}
-	pt.RootCoordAddress = ret
-}
-
 func (pt *ParamTable) initMinIOAddress() {
 	ret, err := pt.Load("_MinioAddress")
 	if err != nil {

internal/indexcoord/paramtable_test.go

+// Copyright (C) 2019-2020 Zilliz. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software distributed under the License
+// is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+// or implied. See the License for the specific language governing permissions and limitations under the License.
+
+package indexcoord
+
+import (
+	"testing"
+)
+
+func TestParamTable(t *testing.T) {
+	Params.Init()
+
+	t.Run("Address", func(t *testing.T) {
+		t.Logf("Address: %v", Params.Address)
+	})
+
+	t.Run("Port", func(t *testing.T) {
+		t.Logf("Port: %v", Params.Port)
+	})
+
+	t.Run("EtcdEndpoints", func(t *testing.T) {
+		t.Logf("EtcdEndpoints: %v", Params.EtcdEndpoints)
+	})
+
+	t.Run("KvRootPath", func(t *testing.T) {
+		t.Logf("KvRootPath: %v", Params.KvRootPath)
+	})
+
+	t.Run("MetaRootPath", func(t *testing.T) {
+		t.Logf("MetaRootPath: %v", Params.MetaRootPath)
+	})
+
+	t.Run("MinIOAddress", func(t *testing.T) {
+		t.Logf("MinIOAddress: %v", Params.MinIOAddress)
+	})
+
+	t.Run("MinIOAccessKeyID", func(t *testing.T) {
+		t.Logf("MinIOAccessKeyID: %v", Params.MinIOAccessKeyID)
+	})
+
+	t.Run("MinIOSecretAccessKey", func(t *testing.T) {
+		t.Logf("MinIOSecretAccessKey: %v", Params.MinIOSecretAccessKey)
+	})
+
+	t.Run("MinIOUseSSL", func(t *testing.T) {
+		t.Logf("MinIOUseSSL: %v", Params.MinIOUseSSL)
+	})
+
+	t.Run("MinioBucketName", func(t *testing.T) {
+		t.Logf("MinioBucketName: %v", Params.MinioBucketName)
+	})
+}
+
+//TODO: Params Load should be return error when key does not exist.
+//func shouldPanic(t *testing.T, name string, f func()) {
+//	defer func() {
+//		if r := recover(); r != nil {
+//			t.Logf("%v recover: %v", name, r)
+//		}
+//	}()
+//	f()
+//}
+//
+//func TestParamTable_Panic(t *testing.T) {
+//	Params.Init()
+//
+//
+//	t.Run("initEtcdEndpoints", func(t *testing.T) {
+//		err := Params.Remove("_EtcdEndpoints")
+//		assert.Nil(t, err)
+//
+//		shouldPanic(t, "initEtcdEndpoints", func() {
+//			Params.initEtcdEndpoints()
+//		})
+//	})
+//
+//	t.Run("initMetaRootPath", func(t *testing.T) {
+//		err := Params.Remove("etcd.rootPath")
+//		assert.Nil(t, err)
+//
+//		shouldPanic(t, "initMetaRootPath", func() {
+//			Params.initMetaRootPath()
+//		})
+//	})
+//
+//	t.Run("initKvRootPath", func(t *testing.T) {
+//		err := Params.Remove("etcd.rootPath")
+//		assert.Nil(t, err)
+//
+//		shouldPanic(t, "initKvRootPath", func() {
+//			Params.initKvRootPath()
+//		})
+//
+//		err = Params.Save("etcd.rootPath", "test")
+//		assert.Nil(t, err)
+//
+//		err = Params.Remove("etcd.kvSubPath")
+//		assert.Nil(t, err)
+//
+//		shouldPanic(t, "initKvRootPath", func() {
+//			Params.initKvRootPath()
+//		})
+//	})
+//
+//	t.Run("initMinIOAddress", func(t *testing.T) {
+//		err := Params.Remove("_MinioAddress")
+//		assert.Nil(t, err)
+//
+//		shouldPanic(t, "initMinIOAddress", func() {
+//			Params.initMinIOAddress()
+//		})
+//	})
+//
+//	t.Run("initMinIOAccessKeyID", func(t *testing.T) {
+//		err := Params.Remove("minio.accessKeyID")
+//		assert.Nil(t, err)
+//
+//		shouldPanic(t, "initMinIOAccessKeyID", func() {
+//			Params.initMinIOAccessKeyID()
+//		})
+//	})
+//
+//	t.Run("initMinIOSecretAccessKey", func(t *testing.T) {
+//		err := Params.Remove("minio.secretAccessKey")
+//		assert.Nil(t, err)
+//
+//		shouldPanic(t, "initMinIOSecretAccessKey", func() {
+//			Params.initMinIOSecretAccessKey()
+//		})
+//	})
+//
+//	t.Run("initMinIOUseSSL", func(t *testing.T) {
+//		err := Params.Remove("minio.useSSL")
+//		assert.Nil(t, err)
+//
+//		shouldPanic(t, "initMinIOUseSSL", func() {
+//			Params.initMinIOUseSSL()
+//		})
+//	})
+//
+//	t.Run("initMinioBucketName", func(t *testing.T) {
+//		err := Params.Remove("minio.bucketName")
+//		assert.Nil(t, err)
+//
+//		shouldPanic(t, "initMinioBucketName", func() {
+//			Params.initMinioBucketName()
+//		})
+//	})
+//}

internal/indexnode/index.go

@@ -56,7 +56,11 @@ func (index *CIndex) Serialize() ([]*Blob, error) {
 	var cBinary C.CBinary
 
 	status := C.SerializeToSlicedBuffer(index.indexPtr, &cBinary)
-	defer C.DeleteCBinary(cBinary)
+	defer func() {
+		if cBinary != nil {
+			C.DeleteCBinary(cBinary)
+		}
+	}()
 	errorCode := status.error_code
 	if errorCode != 0 {
 		errorMsg := C.GoString(status.error_msg)

internal/indexnode/index_test.go

@@ -320,3 +320,36 @@ func TestCIndex_Delete(t *testing.T) {
 		assert.Equal(t, err, nil)
 	}
 }
+
+func TestCIndex_Error(t *testing.T) {
+	indexPtr, err := NewCIndex(nil, nil)
+	assert.Nil(t, err)
+
+	t.Run("Serialize error", func(t *testing.T) {
+		blobs, err := indexPtr.Serialize()
+		assert.NotNil(t, err)
+		assert.Nil(t, blobs)
+	})
+
+	t.Run("Load error", func(t *testing.T) {
+		blobs := []*Blob{{
+			Key:   "test",
+			Value: []byte("value"),
+		},
+		}
+		err = indexPtr.Load(blobs)
+		assert.NotNil(t, err)
+	})
+
+	t.Run("BuildFloatVecIndexWithoutIds error", func(t *testing.T) {
+		floatVectors := []float32{1.1, 2.2, 3.3}
+		err = indexPtr.BuildFloatVecIndexWithoutIds(floatVectors)
+		assert.NotNil(t, err)
+	})
+
+	t.Run("BuildBinaryVecIndexWithoutIds error", func(t *testing.T) {
+		binaryVectors := []byte("binaryVectors")
+		err = indexPtr.BuildBinaryVecIndexWithoutIds(binaryVectors)
+		assert.NotNil(t, err)
+	})
+}

internal/indexnode/indexnode.go

@@ -91,6 +91,7 @@ func (i *IndexNode) Register() error {
 }
 
 func (i *IndexNode) Init() error {
+	Params.Init()
 	i.UpdateStateCode(internalpb.StateCode_Initializing)
 	log.Debug("IndexNode", zap.Any("State", internalpb.StateCode_Initializing))
 	connectEtcdFn := func() error {

internal/indexnode/indexnode_mock.go

@@ -15,7 +15,6 @@ import (
 	"context"
 	"errors"
 	"sync"
-	"time"
 
 	"github.com/milvus-io/milvus/internal/log"
 
@@ -69,7 +68,6 @@ func (inm *Mock) buildIndexTask() {
 				_ = proto.UnmarshalText(values[0], &indexMeta)
 				indexMeta.IndexFilePaths = []string{"IndexFilePath-1", "IndexFilePath-2"}
 				indexMeta.State = commonpb.IndexState_Failed
-				time.Sleep(4 * time.Second)
 				_ = inm.etcdKV.CompareVersionAndSwap(req.MetaPath, versions[0],
 					proto.MarshalTextString(&indexMeta))
 				continue
@@ -79,7 +77,6 @@ func (inm *Mock) buildIndexTask() {
 			_ = proto.UnmarshalText(values[0], &indexMeta)
 			indexMeta.IndexFilePaths = []string{"IndexFilePath-1", "IndexFilePath-2"}
 			indexMeta.State = commonpb.IndexState_Failed
-			time.Sleep(4 * time.Second)
 			_ = inm.etcdKV.CompareVersionAndSwap(req.MetaPath, versions[0],
 				proto.MarshalTextString(&indexMeta))
 			indexMeta.Version = indexMeta.Version + 1

internal/indexnode/indexnode_test.go

@@ -12,6 +12,7 @@
 package indexnode
 
 import (
+	"container/list"
 	"context"
 	"path"
 	"strconv"
@@ -727,3 +728,26 @@ func TestIndexNode_Error(t *testing.T) {
 	err = in.Stop()
 	assert.Nil(t, err)
 }
+
+func TestIndexNode_InitError(t *testing.T) {
+	ctx := context.Background()
+	in := &IndexNode{
+		sched: &TaskScheduler{
+			IndexBuildQueue: &IndexBuildTaskQueue{
+				BaseTaskQueue: BaseTaskQueue{
+					unissuedTasks: list.New(),
+					activeTasks:   make(map[UniqueID]task),
+					maxTaskNum:    0,
+					utBufChan:     make(chan int, 1024),
+				},
+			},
+		},
+	}
+	in.UpdateStateCode(internalpb.StateCode_Healthy)
+
+	t.Run("CreateIndex", func(t *testing.T) {
+		status, err := in.CreateIndex(ctx, &indexpb.CreateIndexRequest{})
+		assert.Nil(t, err)
+		assert.Equal(t, commonpb.ErrorCode_UnexpectedError, status.ErrorCode)
+	})
+}

internal/indexnode/paramtable.go

@@ -36,8 +36,6 @@ type ParamTable struct {
 	NodeID int64
 	Alias  string
 
-	RootCoordAddress string
-
 	EtcdEndpoints []string
 	MetaRootPath  string
 

internal/indexnode/paramtable_test.go

@@ -12,37 +12,152 @@
 package indexnode
 
 import (
-	"fmt"
 	"testing"
-
-	"github.com/stretchr/testify/assert"
 )
 
-func TestParamTable_Init(t *testing.T) {
+func TestParamTable(t *testing.T) {
 	Params.Init()
-}
 
-func TestParamTable_Address(t *testing.T) {
-	address := Params.Address
-	fmt.Println(address)
-}
+	t.Run("IP", func(t *testing.T) {
+		t.Logf("IP: %v", Params.IP)
+	})
 
-func TestParamTable_MinIOAddress(t *testing.T) {
-	address := Params.MinIOAddress
-	fmt.Println(address)
-}
+	t.Run("Address", func(t *testing.T) {
+		t.Logf("Address: %v", Params.Address)
+	})
 
-func TestParamTable_MinIOAccessKeyID(t *testing.T) {
-	accessKeyID := Params.MinIOAccessKeyID
-	assert.Equal(t, accessKeyID, "minioadmin")
-}
+	t.Run("Port", func(t *testing.T) {
+		t.Logf("Port: %v", Params.Port)
+	})
 
-func TestParamTable_MinIOSecretAccessKey(t *testing.T) {
-	secretAccessKey := Params.MinIOSecretAccessKey
-	assert.Equal(t, secretAccessKey, "minioadmin")
-}
+	t.Run("NodeID", func(t *testing.T) {
+		t.Logf("NodeID: %v", Params.NodeID)
+	})
+
+	t.Run("Alias", func(t *testing.T) {
+		t.Logf("Alias: %v", Params.Alias)
+	})
+
+	t.Run("EtcdEndpoints", func(t *testing.T) {
+		t.Logf("EtcdEndpoints: %v", Params.EtcdEndpoints)
+	})
+
+	t.Run("MetaRootPath", func(t *testing.T) {
+		t.Logf("MetaRootPath: %v", Params.MetaRootPath)
+	})
 
-func TestParamTable_MinIOUseSSL(t *testing.T) {
-	useSSL := Params.MinIOUseSSL
-	assert.Equal(t, useSSL, false)
+	t.Run("MinIOAddress", func(t *testing.T) {
+		t.Logf("MinIOAddress: %v", Params.MinIOAddress)
+	})
+
+	t.Run("MinIOAccessKeyID", func(t *testing.T) {
+		t.Logf("MinIOAccessKeyID: %v", Params.MinIOAccessKeyID)
+	})
+
+	t.Run("MinIOSecretAccessKey", func(t *testing.T) {
+		t.Logf("MinIOSecretAccessKey: %v", Params.MinIOSecretAccessKey)
+	})
+
+	t.Run("MinIOUseSSL", func(t *testing.T) {
+		t.Logf("MinIOUseSSL: %v", Params.MinIOUseSSL)
+	})
+
+	t.Run("MinioBucketName", func(t *testing.T) {
+		t.Logf("MinioBucketName: %v", Params.MinioBucketName)
+	})
 }
+
+//TODO: Params Load should be return error when key does not exist.
+//func shouldPanic(t *testing.T, name string, f func()) {
+//	defer func() {
+//		if r := recover(); r != nil {
+//			t.Logf("%v recover: %v", name, r)
+//		}
+//	}()
+//	f()
+//}
+//
+//func TestParamTable_Panic(t *testing.T) {
+//	Params.Init()
+//
+//	t.Run("initMinIOAddress", func(t *testing.T) {
+//		err := Params.Remove("_MinioAddress")
+//		assert.Nil(t, err)
+//
+//		shouldPanic(t, "initMinIOAddress", func() {
+//			Params.initMinIOAddress()
+//		})
+//	})
+//
+//	t.Run("initMinIOAccessKeyID", func(t *testing.T) {
+//		err := Params.Remove("minio.accessKeyID")
+//		assert.Nil(t, err)
+//
+//		shouldPanic(t, "initMinIOAccessKeyID", func() {
+//			Params.initMinIOAccessKeyID()
+//		})
+//	})
+//
+//	t.Run("initMinIOAccessKeyID", func(t *testing.T) {
+//		err := Params.Remove("minio.accessKeyID")
+//		assert.Nil(t, err)
+//
+//		shouldPanic(t, "initMinIOAccessKeyID", func() {
+//			Params.initMinIOAccessKeyID()
+//		})
+//	})
+//
+//	t.Run("initMinIOSecretAccessKey", func(t *testing.T) {
+//		err := Params.Remove("minio.secretAccessKey")
+//		assert.Nil(t, err)
+//
+//		shouldPanic(t, "initMinIOSecretAccessKey", func() {
+//			Params.initMinIOSecretAccessKey()
+//		})
+//	})
+//
+//	t.Run("initMinIOUseSSL", func(t *testing.T) {
+//		err := Params.Remove("minio.useSSL")
+//		assert.Nil(t, err)
+//
+//		shouldPanic(t, "initMinIOUseSSL", func() {
+//			Params.initMinIOUseSSL()
+//		})
+//	})
+//
+//	t.Run("initEtcdEndpoints", func(t *testing.T) {
+//		err := Params.Remove("_EtcdEndpoints")
+//		assert.Nil(t, err)
+//
+//		shouldPanic(t, "initEtcdEndpoints", func() {
+//			Params.initEtcdEndpoints()
+//		})
+//	})
+//
+//	t.Run("initMetaRootPath", func(t *testing.T) {
+//		err := Params.Remove("etcd.rootPath")
+//		assert.Nil(t, err)
+//
+//		shouldPanic(t, "initMetaRootPath", func() {
+//			Params.initMetaRootPath()
+//		})
+//
+//		err = Params.Save("etcd.rootPath", "test")
+//		assert.Nil(t, err)
+//		err = Params.Remove("etcd.metaSubPath")
+//		assert.Nil(t, err)
+//
+//		shouldPanic(t, "initMetaRootPath", func() {
+//			Params.initMetaRootPath()
+//		})
+//	})
+//
+//	t.Run("initMinioBucketName", func(t *testing.T) {
+//		err := Params.Remove("minio.bucketName")
+//		assert.Nil(t, err)
+//
+//		shouldPanic(t, "initMinioBucketName", func() {
+//			Params.initMinioBucketName()
+//		})
+//	})
+//}