feature/tests: add more tests and refactor Makefile for race tests (#52)

* feature/tests: add more tests and refactor Makefile for race tests

* Makefile refactored for racetests
* Tests coverage up

* feature/tests: fix lint

* feature/tests: fix CHANGELOG

* feature/tests: fixes after review

---------

Co-authored-by: Maksim Konovalov <maksim.konovalov@vk.team>

CHANGELOG.md

@@ -22,7 +22,8 @@ REFACTOR:
 * Ignore .tmp files
 * Refactored provider creation test caused by golang-ci lint (#33)
 * Router implements directly TopologyController, no proxy object is used now
-
+* Makefile refactored for racetests
+* Tests coverage up 22% -> 33%
 
 
 ## 0.0.11

Makefile

@@ -1,4 +1,6 @@
 TEST_TIMEOUT?=20s
+EXTENDED_TEST_TIMEOUT=1m
+GO_CMD?=go
 LOCAL_BIN:=$(CURDIR)/bin
 # golang-ci tag
 GOLANGCI_TAG:=latest
@@ -9,16 +11,16 @@ GOLANGCI_BIN:=$(GOPATH)/bin/golangci-lint
 install-lint:
 ifeq ($(wildcard $(GOLANGCI_BIN)),)
 	$(info #Downloading swaggo latest)
-	go install github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCI_TAG)
+	$(GO_CMD) install github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCI_TAG)
 endif
 
 test:
-	go test ./... -parallel=10 -timeout=$(TEST_TIMEOUT) -coverprofile=coverage.out.tmp
+	$(GO_CMD) test ./... -parallel=10 -timeout=$(TEST_TIMEOUT) -coverprofile=coverage.out.tmp
 	@cat coverage.out.tmp | grep -v "mock" > coverage.out
 	@rm coverage.out.tmp
 
 cover: test
-	 go tool cover -html=coverage.out
+	 $(GO_CMD) tool cover -html=coverage.out
 
 test/integration:
 	@$(MAKE) -C ./tests/integration test
@@ -31,3 +33,7 @@ generate/mocks:
 lint: install-lint
 	$(GOLANGCI_BIN) run --config=.golangci.yaml ./...
 
+testrace: BUILD_TAGS+=testonly
+testrace:
+	@CGO_ENABLED=1 \
+	$(GO_CMD) test -tags='$(BUILD_TAGS)' -race  -timeout=$(EXTENDED_TEST_TIMEOUT) -parallel=20

api_test.go

@@ -0,0 +1,70 @@
+package vshard_router // nolint: revive
+
+import (
+	"context"
+	"fmt"
+	"testing"
+	"time"
+
+	mockpool "github.com/KaymeKaydex/go-vshard-router/mocks/pool"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+	"github.com/tarantool/go-tarantool/v2"
+)
+
+var emptyRouter = &Router{
+	cfg: Config{
+		TotalBucketCount: uint64(10),
+		Logger:           &EmptyLogger{},
+		Metrics:          &EmptyMetrics{},
+	},
+}
+
+func TestVshardMode_String_NotEmpty(t *testing.T) {
+	t.Parallel()
+	require.NotEmpty(t, ReadMode.String())
+	require.NotEmpty(t, WriteMode.String())
+}
+
+func TestRouter_RouterRouteAll(t *testing.T) {
+	t.Parallel()
+	m := emptyRouter.RouterRouteAll()
+	require.Empty(t, m)
+}
+
+func TestRouter_RouterCallImpl(t *testing.T) {
+	t.Parallel()
+	ctx := context.TODO()
+
+	t.Run("bucket id is out of range", func(t *testing.T) {
+		t.Parallel()
+
+		_, _, err := emptyRouter.RouterCallImpl(ctx, 100, CallOpts{}, "test", []byte("test"))
+		require.Errorf(t, err, "bucket id is out of range")
+	})
+	t.Run("future error when router call impl", func(t *testing.T) {
+		t.Parallel()
+		r := &Router{
+			cfg: Config{
+				TotalBucketCount: uint64(10),
+				Logger:           &EmptyLogger{},
+				Metrics:          &EmptyMetrics{},
+			},
+			routeMap: make([]*Replicaset, 11),
+		}
+
+		futureError := fmt.Errorf("testErr")
+		errFuture := tarantool.NewFuture(tarantool.NewCallRequest("test"))
+		errFuture.SetError(futureError)
+
+		mPool := mockpool.NewPool(t)
+		mPool.On("Do", mock.Anything, mock.Anything).Return(errFuture)
+
+		r.routeMap[5] = &Replicaset{
+			conn: mPool,
+		}
+
+		_, _, err := r.RouterCallImpl(ctx, 5, CallOpts{Timeout: time.Second}, "test", []byte("test"))
+		require.ErrorIs(t, futureError, err)
+	})
+}

discovery.go

@@ -103,6 +103,10 @@ func (r *Router) BucketDiscovery(ctx context.Context, bucketID uint64) (*Replica
 
 // BucketResolve resolve bucket id to replicaset
 func (r *Router) BucketResolve(ctx context.Context, bucketID uint64) (*Replicaset, error) {
+	if bucketID > r.cfg.TotalBucketCount {
+		return nil, fmt.Errorf("bucket id is out of range: %d (total %d)", bucketID, r.cfg.TotalBucketCount)
+	}
+
 	rs := r.routeMap[bucketID]
 	if rs != nil {
 		return rs, nil

discovery_test.go

@@ -1,6 +1,7 @@
 package vshard_router //nolint:revive
 
 import (
+	"context"
 	"sync"
 	"testing"
 	"time"
@@ -34,3 +35,18 @@ func TestSearchLock_WaitOnSearch(t *testing.T) {
 
 	require.True(t, time.Since(lockStart) < 12*time.Millisecond && time.Since(lockStart) > 9*time.Millisecond)
 }
+
+func TestRouter_BucketResolve_InvalidBucketID(t *testing.T) {
+	ctx := context.TODO()
+
+	r := Router{
+		cfg: Config{
+			TotalBucketCount: uint64(10),
+			Logger:           &EmptyLogger{},
+		},
+		routeMap: make([]*Replicaset, 11),
+	}
+
+	_, err := r.BucketResolve(ctx, 20)
+	require.Error(t, err)
+}

providers/static/provider_test.go

@@ -26,6 +26,7 @@ func TestNewProvider(t *testing.T) {
 
 	for _, tc := range tCases {
 		t.Run("provider", func(t *testing.T) {
+			t.Parallel()
 			if len(tc.Source) == 0 {
 
 				require.Panics(t, func() {
@@ -83,6 +84,7 @@ func TestProvider_Validate(t *testing.T) {
 
 	for _, tc := range tCases {
 		t.Run(fmt.Sprintf("is err: %v", tc.IsErr), func(t *testing.T) {
+			t.Parallel()
 			provider := NewProvider(tc.Source)
 			if tc.IsErr {
 				require.Error(t, provider.Validate())

topology_test.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"testing"
 
+	mockpool "github.com/KaymeKaydex/go-vshard-router/mocks/pool"
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/require"
 )
@@ -56,13 +57,40 @@ func TestController_RemoveInstance(t *testing.T) {
 func TestController_RemoveReplicaset(t *testing.T) {
 	ctx := context.Background()
 
-	t.Run("no such replicaset", func(t *testing.T) {
-		router := Router{
-			idToReplicaset: map[uuid.UUID]*Replicaset{},
-		}
+	uuidToRemove := uuid.New()
+	mPool := mockpool.NewPool(t)
+	mPool.On("CloseGraceful").Return(nil)
 
-		errs := router.Topology().RemoveReplicaset(ctx, uuid.New())
+	router := Router{
+		idToReplicaset: map[uuid.UUID]*Replicaset{
+			uuidToRemove: {conn: mPool},
+		},
+	}
 
+	t.Run("no such replicaset", func(t *testing.T) {
+		t.Parallel()
+		errs := router.Topology().RemoveReplicaset(ctx, uuid.New())
 		require.True(t, errors.Is(errs[0], ErrReplicasetNotExists))
 	})
+	t.Run("successfully remove", func(t *testing.T) {
+		t.Parallel()
+		errs := router.Topology().RemoveReplicaset(ctx, uuidToRemove)
+		require.Empty(t, errs)
+	})
+}
+
+func TestRouter_AddReplicaset_AlreadyExists(t *testing.T) {
+	ctx := context.TODO()
+
+	alreadyExistingRsUUID := uuid.New()
+
+	router := Router{
+		idToReplicaset: map[uuid.UUID]*Replicaset{
+			alreadyExistingRsUUID: {},
+		},
+	}
+
+	// Test that such replicaset already exists
+	err := router.AddReplicaset(ctx, ReplicasetInfo{UUID: alreadyExistingRsUUID}, []InstanceInfo{})
+	require.Equalf(t, ErrReplicasetExists, err, "such replicaset must already exists")
 }

vshard.go

@@ -16,6 +16,7 @@ import (
 var (
 	ErrInvalidConfig       = fmt.Errorf("config invalid")
 	ErrInvalidInstanceInfo = fmt.Errorf("invalid instance info")
+	ErrTopologyProvider    = fmt.Errorf("got error from topology provider")
 )
 
 type Router struct {
@@ -120,7 +121,7 @@ func NewRouter(ctx context.Context, cfg Config) (*Router, error) {
 	if err != nil {
 		router.log().Error(ctx, fmt.Sprintf("cant create new topology provider with err: %s", err))
 
-		return nil, err
+		return nil, fmt.Errorf("%w; cant init topology with err: %w", ErrTopologyProvider, err)
 	}
 
 	err = router.DiscoveryAllBuckets(ctx)

vshard_shadow_test.go

@@ -2,28 +2,46 @@ package vshard_router_test
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/require"
 
-	vshard_router "github.com/KaymeKaydex/go-vshard-router"
+	vshardrouter "github.com/KaymeKaydex/go-vshard-router"
 	"github.com/KaymeKaydex/go-vshard-router/providers/static"
 )
 
+type errorTopologyProvider struct{}
+
+func (e *errorTopologyProvider) Init(_ vshardrouter.TopologyController) error {
+	return fmt.Errorf("test error")
+}
+func (e *errorTopologyProvider) Close() {}
+
+func TestNewRouter_ProviderError(t *testing.T) {
+	ctx := context.TODO()
+	_, err := vshardrouter.NewRouter(ctx, vshardrouter.Config{
+		TotalBucketCount: 256000,
+		TopologyProvider: &errorTopologyProvider{},
+	})
+
+	require.ErrorIs(t, err, vshardrouter.ErrTopologyProvider)
+}
+
 func TestNewRouter_EmptyReplicasets(t *testing.T) {
 	ctx := context.TODO()
 
-	router, err := vshard_router.NewRouter(ctx, vshard_router.Config{})
+	router, err := vshardrouter.NewRouter(ctx, vshardrouter.Config{})
 	require.Error(t, err)
 	require.Nil(t, router)
 }
 
 func TestNewRouter_InvalidReplicasetUUID(t *testing.T) {
 	ctx := context.TODO()
 
-	router, err := vshard_router.NewRouter(ctx, vshard_router.Config{
-		TopologyProvider: static.NewProvider(map[vshard_router.ReplicasetInfo][]vshard_router.InstanceInfo{
+	router, err := vshardrouter.NewRouter(ctx, vshardrouter.Config{
+		TopologyProvider: static.NewProvider(map[vshardrouter.ReplicasetInfo][]vshardrouter.InstanceInfo{
 			{
 				Name: "123",
 			}: {
@@ -39,8 +57,8 @@ func TestNewRouter_InvalidReplicasetUUID(t *testing.T) {
 func TestNewRouter_InstanceAddr(t *testing.T) {
 	ctx := context.TODO()
 
-	router, err := vshard_router.NewRouter(ctx, vshard_router.Config{
-		TopologyProvider: static.NewProvider(map[vshard_router.ReplicasetInfo][]vshard_router.InstanceInfo{
+	router, err := vshardrouter.NewRouter(ctx, vshardrouter.Config{
+		TopologyProvider: static.NewProvider(map[vshardrouter.ReplicasetInfo][]vshardrouter.InstanceInfo{
 			{
 				Name: "123",
 				UUID: uuid.New(),
@@ -56,40 +74,40 @@ func TestNewRouter_InstanceAddr(t *testing.T) {
 
 func TestRouterBucketIDStrCRC32(t *testing.T) {
 	// required values from tarantool example
-	require.Equal(t, uint64(103202), vshard_router.BucketIDStrCRC32("2707623829", uint64(256000)))
-	require.Equal(t, uint64(35415), vshard_router.BucketIDStrCRC32("2706201716", uint64(256000)))
+	require.Equal(t, uint64(103202), vshardrouter.BucketIDStrCRC32("2707623829", uint64(256000)))
+	require.Equal(t, uint64(35415), vshardrouter.BucketIDStrCRC32("2706201716", uint64(256000)))
 }
 
 func BenchmarkRouterBucketIDStrCRC32(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		vshard_router.BucketIDStrCRC32("test_bench_key", uint64(256000))
+		vshardrouter.BucketIDStrCRC32("test_bench_key", uint64(256000))
 	}
 }
 
 func TestInstanceInfo_Validate(t *testing.T) {
 	tCases := []struct {
 		Name  string
-		II    vshard_router.InstanceInfo
+		II    vshardrouter.InstanceInfo
 		Valid bool
 	}{
 		{
 			Name:  "no info",
-			II:    vshard_router.InstanceInfo{},
+			II:    vshardrouter.InstanceInfo{},
 			Valid: false,
 		},
 		{
 			Name:  "no uuid",
-			II:    vshard_router.InstanceInfo{Addr: "first.internal:1212"},
+			II:    vshardrouter.InstanceInfo{Addr: "first.internal:1212"},
 			Valid: false,
 		},
 		{
 			Name:  "no addr",
-			II:    vshard_router.InstanceInfo{UUID: uuid.New()},
+			II:    vshardrouter.InstanceInfo{UUID: uuid.New()},
 			Valid: false,
 		},
 		{
 			Name:  "ok",
-			II:    vshard_router.InstanceInfo{UUID: uuid.New(), Addr: "first.internal:1212"},
+			II:    vshardrouter.InstanceInfo{UUID: uuid.New(), Addr: "first.internal:1212"},
 			Valid: true,
 		},
 	}