From 4e0cbe0c0d021a54d6d301e996d636dd5b7d941f Mon Sep 17 00:00:00 2001 From: Nurzhan Saktaganov Date: Wed, 4 Sep 2024 23:12:50 +0300 Subject: [PATCH 1/5] Add tnt test for RouterCallImpl * don't run TestConncurrentTopologyChange parallel with other ones * fix callOpts for RouterCallImpl --- CHANGELOG.md | 1 + tests/tnt/concurrent_topology_test.go | 14 ++++++-- tests/tnt/router_call_test.go | 52 +++++++++++++++++++++++++++ tests/tnt/routermap_call_test.go | 2 ++ 4 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 tests/tnt/router_call_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 69586ad..5639736 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ REFACTOR: * resolve issue #38: simplify DiscoveryAllBuckets and remove suspicious if * resolve issue #46: drastically simplify RouterMapCallRWImpl and added tests with real tnt * Use typed nil pointers instead of memory allocation for EmptyMetrics and emptyLogger structs +* New test for RouterCallImpl (and fix the old one) ## 0.0.12 diff --git a/tests/tnt/concurrent_topology_test.go b/tests/tnt/concurrent_topology_test.go index 3811ea3..8bfdbeb 100644 --- a/tests/tnt/concurrent_topology_test.go +++ b/tests/tnt/concurrent_topology_test.go @@ -115,7 +115,13 @@ func TestConncurrentTopologyChange(t *testing.T) { return } - t.Parallel() + // suppress the below linter warning: + // unused-parameter: parameter 't' seems to be unused, consider removing or renaming it as _ (revive) + _ = t + + // Don't run this parallel with other tests, because this test is heavy and used to detect data races. + // Therefore this test may impact other ones. + // t.Parallel() tc := &concurrentTopologyProvider{} @@ -151,7 +157,11 @@ func TestConncurrentTopologyChange(t *testing.T) { bucketID := uint64((rand.Int() % totalBucketCount) + 1) args := []interface{}{"arg1"} - _, _, _ = router.RouterCallImpl(ctx, bucketID, vshardrouter.CallOpts{}, "echo", args) + callOpts := vshardrouter.CallOpts{ + VshardMode: vshardrouter.ReadMode, + } + + _, _, _ = router.RouterCallImpl(ctx, bucketID, callOpts, "echo", args) } }() diff --git a/tests/tnt/router_call_test.go b/tests/tnt/router_call_test.go new file mode 100644 index 0000000..dc599a6 --- /dev/null +++ b/tests/tnt/router_call_test.go @@ -0,0 +1,52 @@ +package tnt_test + +import ( + "context" + "log" + "math/rand" + "testing" + "time" + + vshardrouter "github.com/KaymeKaydex/go-vshard-router" + "github.com/KaymeKaydex/go-vshard-router/providers/static" + "github.com/stretchr/testify/require" + "github.com/tarantool/go-tarantool/v2/pool" +) + +func TestRouterCall(t *testing.T) { + if !isCorrectRun() { + log.Printf("Incorrect run of tnt-test framework") + return + } + + t.Parallel() + + ctx := context.Background() + + cfg := getCfg() + + router, err := vshardrouter.NewRouter(ctx, vshardrouter.Config{ + TopologyProvider: static.NewProvider(cfg), + DiscoveryTimeout: 5 * time.Second, + DiscoveryMode: vshardrouter.DiscoveryModeOn, + TotalBucketCount: totalBucketCount, + User: defaultTntUser, + Password: defaultTntPassword, + }) + + if err != nil { + panic(err) + } + + //nolint:gosec + bucketID := uint64((rand.Int() % totalBucketCount) + 1) + args := []interface{}{"arg1"} + + resp, _, err := router.RouterCallImpl(ctx, bucketID, vshardrouter.CallOpts{ + VshardMode: vshardrouter.ReadMode, + PoolMode: pool.PreferRO, + }, "echo", args) + + require.Nil(t, err, "RouterCallImpl echo finished with no err") + require.EqualValues(t, args, resp, "RouterCallImpl echo resp correct") +} diff --git a/tests/tnt/routermap_call_test.go b/tests/tnt/routermap_call_test.go index fb06d22..f1e6633 100644 --- a/tests/tnt/routermap_call_test.go +++ b/tests/tnt/routermap_call_test.go @@ -17,6 +17,8 @@ func TestRouterMapCall(t *testing.T) { return } + t.Parallel() + ctx := context.Background() cfg := getCfg() From c9ba529f95f9805beabb483a8e89cfa89919579b Mon Sep 17 00:00:00 2001 From: Nurzhan Saktaganov Date: Mon, 9 Sep 2024 23:24:15 +0300 Subject: [PATCH 2/5] review fix for PR #56: replace panic to require.Nil --- tests/tnt/concurrent_topology_test.go | 6 +++--- tests/tnt/router_call_test.go | 4 +--- tests/tnt/routermap_call_test.go | 4 +--- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/tests/tnt/concurrent_topology_test.go b/tests/tnt/concurrent_topology_test.go index 8bfdbeb..3fb0f67 100644 --- a/tests/tnt/concurrent_topology_test.go +++ b/tests/tnt/concurrent_topology_test.go @@ -10,6 +10,7 @@ import ( "time" vshardrouter "github.com/KaymeKaydex/go-vshard-router" + "github.com/stretchr/testify/require" ) type concurrentTopologyProvider struct { @@ -133,9 +134,8 @@ func TestConncurrentTopologyChange(t *testing.T) { User: defaultTntUser, Password: defaultTntPassword, }) - if err != nil { - panic(err) - } + + require.Nil(t, err, "NewRouter finished successfully") wg := sync.WaitGroup{} diff --git a/tests/tnt/router_call_test.go b/tests/tnt/router_call_test.go index dc599a6..5eccd2d 100644 --- a/tests/tnt/router_call_test.go +++ b/tests/tnt/router_call_test.go @@ -34,9 +34,7 @@ func TestRouterCall(t *testing.T) { Password: defaultTntPassword, }) - if err != nil { - panic(err) - } + require.Nil(t, err, "NewRouter finished successfully") //nolint:gosec bucketID := uint64((rand.Int() % totalBucketCount) + 1) diff --git a/tests/tnt/routermap_call_test.go b/tests/tnt/routermap_call_test.go index f1e6633..d0bdf3e 100644 --- a/tests/tnt/routermap_call_test.go +++ b/tests/tnt/routermap_call_test.go @@ -32,9 +32,7 @@ func TestRouterMapCall(t *testing.T) { Password: defaultTntPassword, }) - if err != nil { - panic(err) - } + require.Nil(t, err, "NewRouter finished successfully") const arg = "arg1" From 3fc43f9f74f0f99546ca0be4351334e451e39dda Mon Sep 17 00:00:00 2001 From: Nurzhan Saktaganov Date: Mon, 9 Sep 2024 23:57:57 +0300 Subject: [PATCH 3/5] tests/tnt: store logs of vshard storages --- tests/tnt/Makefile | 14 +++++++------- tests/tnt/cfgmaker.lua | 1 - 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/tnt/Makefile b/tests/tnt/Makefile index 8e82e1b..b3f720d 100644 --- a/tests/tnt/Makefile +++ b/tests/tnt/Makefile @@ -1,5 +1,5 @@ -NREPLICASETS?=5 -START_PORT?=33000 +export NREPLICASETS:=5 +export START_PORT:=33000 RED=\033[0;31m GREEN=\033[0;32m @@ -30,16 +30,16 @@ cluster-up: ln -sf `(pwd)`/cfgmaker.lua tmp/$${rsid}/master/cfgmaker.lua; \ ln -sf `(pwd)`/storage.lua tmp/$${rsid}/follower/storage_$${rsid}_follower.lua; \ ln -sf `(pwd)`/cfgmaker.lua tmp/$${rsid}/follower/cfgmaker.lua; \ - TT_WORK_DIR=tmp/$${rsid}/master/ TT_PID_FILE="tarantool.pid" TT_BACKGROUND=true START_PORT=${START_PORT} TT_LOG=/dev/null NREPLICASETS=${NREPLICASETS} tarantool tmp/$${rsid}/master/storage_$${rsid}_master.lua; \ - TT_WORK_DIR=tmp/$${rsid}/follower/ TT_PID_FILE="tarantool.pid" TT_BACKGROUND=true START_PORT=${START_PORT} TT_LOG=/dev/null NREPLICASETS=${NREPLICASETS} tarantool tmp/$${rsid}/follower/storage_$${rsid}_follower.lua; \ + TT_WORK_DIR=tmp/$${rsid}/master/ TT_PID_FILE=tarantool.pid TT_BACKGROUND=true TT_LOG=tarantool.log tarantool tmp/$${rsid}/master/storage_$${rsid}_master.lua; \ + TT_WORK_DIR=tmp/$${rsid}/follower/ TT_PID_FILE=tarantool.pid TT_BACKGROUND=true TT_LOG=tarantool.log tarantool tmp/$${rsid}/follower/storage_$${rsid}_follower.lua; \ ((rsid = rsid + 1)) ; \ - done + done # bootstrap vshard cluster using lua vshard.router bootstrap: @echo "${GREEN}STAGE: BOOTSTRAP CLUSTER${NC}" mkdir -p tmp/router_work_dir - TT_WORK_DIR=tmp/router_work_dir/ NREPLICASETS=${NREPLICASETS} START_PORT=${START_PORT} tarantool router.lua + TT_WORK_DIR=tmp/router_work_dir/ tarantool router.lua # stop vshard storage tarantool cluster-down: @@ -53,5 +53,5 @@ cluster-down: # run go tests, minus "-" signs before command allows failures, otherwise cluster-down stage won't run. gotest: @echo "${GREEN}STAGE: RUN GOTESTS${NC}" - -NREPLICASETS=${NREPLICASETS} START_PORT=${START_PORT} go test -race -parallel=20 -coverpkg="../../" -coverprofile cover.out -timeout=90s + -go test -race -parallel=20 -coverpkg="../../" -coverprofile cover.out -timeout=90s # go tool cover -html=cover.out diff --git a/tests/tnt/cfgmaker.lua b/tests/tnt/cfgmaker.lua index ec0f434..437bf27 100644 --- a/tests/tnt/cfgmaker.lua +++ b/tests/tnt/cfgmaker.lua @@ -89,7 +89,6 @@ end local function clustercfg(start_port, nreplicasets) local cfg = { sharding = {}, - replication_connect_quorum = 0, } for rs_id = 1, nreplicasets do From 1102d9fadcc30363d034ccb4ed7df86831e3d762 Mon Sep 17 00:00:00 2001 From: Nurzhan Saktaganov Date: Tue, 10 Sep 2024 00:04:18 +0300 Subject: [PATCH 4/5] review fix for PR #56 (export envvar) --- tests/tnt/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/tnt/Makefile b/tests/tnt/Makefile index b3f720d..14ec59c 100644 --- a/tests/tnt/Makefile +++ b/tests/tnt/Makefile @@ -1,5 +1,6 @@ export NREPLICASETS:=5 export START_PORT:=33000 +export OBJC_DISABLE_INITIALIZE_FORK_SAFETY:=YES # See review comments for PR #56 why RED=\033[0;31m GREEN=\033[0;32m From 72902d973370ae28e603d8af4acc05b33176c938 Mon Sep 17 00:00:00 2001 From: Nurzhan Saktaganov Date: Wed, 11 Sep 2024 15:03:58 +0300 Subject: [PATCH 5/5] review fix for #56 - add grants for guest user - update comments --- tests/tnt/Makefile | 2 +- tests/tnt/concurrent_topology_test.go | 4 ---- tests/tnt/storage.lua | 2 ++ 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/tnt/Makefile b/tests/tnt/Makefile index 14ec59c..b6b32d1 100644 --- a/tests/tnt/Makefile +++ b/tests/tnt/Makefile @@ -20,7 +20,7 @@ clean: # prepare vshard-storages, that contains ${NREPLICASETS} replicasets. # every replicaset has one master and one follower instance. -# every replicaset runs in background mode, no logs are stored (/dev/null) +# every replicaset runs in background mode cluster-up: @echo "${GREEN}STAGE: CLUSTER UP${NC}" mkdir -p tmp diff --git a/tests/tnt/concurrent_topology_test.go b/tests/tnt/concurrent_topology_test.go index 3fb0f67..972f03f 100644 --- a/tests/tnt/concurrent_topology_test.go +++ b/tests/tnt/concurrent_topology_test.go @@ -116,10 +116,6 @@ func TestConncurrentTopologyChange(t *testing.T) { return } - // suppress the below linter warning: - // unused-parameter: parameter 't' seems to be unused, consider removing or renaming it as _ (revive) - _ = t - // Don't run this parallel with other tests, because this test is heavy and used to detect data races. // Therefore this test may impact other ones. // t.Parallel() diff --git a/tests/tnt/storage.lua b/tests/tnt/storage.lua index 930f735..3587123 100644 --- a/tests/tnt/storage.lua +++ b/tests/tnt/storage.lua @@ -85,6 +85,8 @@ box.once("testapp:schema:1", function() box.schema.role.grant('public', 'execute', 'function', 'raise_luajit_error') box.schema.func.create('raise_client_error') box.schema.role.grant('public', 'execute', 'function', 'raise_client_error') + + box.schema.user.grant('guest', 'super') end)