You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kvrocks.apache.org by ti...@apache.org on 2022/09/05 07:08:59 UTC
[incubator-kvrocks] branch unstable updated: refactor: unit/protocol to gocase (#810)
This is an automated email from the ASF dual-hosted git repository.
tison pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/incubator-kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new e8b042a refactor: unit/protocol to gocase (#810)
e8b042a is described below
commit e8b042a4f84d5a4ff30c824639ea9ed3ea77e0cb
Author: tison <wa...@gmail.com>
AuthorDate: Mon Sep 5 15:08:54 2022 +0800
refactor: unit/protocol to gocase (#810)
Signed-off-by: tison <wa...@gmail.com>
---
tests/gocase/unit/command/command_test.go | 4 +-
tests/gocase/unit/info/info_test.go | 4 +-
tests/gocase/unit/protocol/protocol_test.go | 142 ++++++++++++++++++++++++++
tests/gocase/unit/protocol/regression_test.go | 64 ++++++++++++
tests/gocase/util/server.go | 29 +++---
tests/gocase/util/tcp_client.go | 60 +++++++++++
tests/tcl/tests/test_helper.tcl | 1 -
tests/tcl/tests/unit/protocol.tcl | 124 ----------------------
8 files changed, 281 insertions(+), 147 deletions(-)
diff --git a/tests/gocase/unit/command/command_test.go b/tests/gocase/unit/command/command_test.go
index b576c05..a2a424f 100644
--- a/tests/gocase/unit/command/command_test.go
+++ b/tests/gocase/unit/command/command_test.go
@@ -15,7 +15,6 @@
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
- *
*/
package command
@@ -29,8 +28,7 @@ import (
)
func TestCommand(t *testing.T) {
- srv, err := util.StartServer(t, map[string]string{})
- require.NoError(t, err)
+ srv := util.StartServer(t, map[string]string{})
defer srv.Close()
ctx := context.Background()
diff --git a/tests/gocase/unit/info/info_test.go b/tests/gocase/unit/info/info_test.go
index 54acac0..1fde8cf 100644
--- a/tests/gocase/unit/info/info_test.go
+++ b/tests/gocase/unit/info/info_test.go
@@ -15,7 +15,6 @@
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
- *
*/
package command
@@ -34,8 +33,7 @@ import (
)
func TestInfo(t *testing.T) {
- srv, err := util.StartServer(t, map[string]string{})
- require.NoError(t, err)
+ srv := util.StartServer(t, map[string]string{})
defer srv.Close()
ctx := context.Background()
diff --git a/tests/gocase/unit/protocol/protocol_test.go b/tests/gocase/unit/protocol/protocol_test.go
new file mode 100644
index 0000000..85a320d
--- /dev/null
+++ b/tests/gocase/unit/protocol/protocol_test.go
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 protocol
+
+import (
+ "context"
+ "testing"
+
+ "github.com/apache/incubator-kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func TestProtocolNetwork(t *testing.T) {
+ srv := util.StartServer(t, map[string]string{})
+ defer srv.Close()
+
+ t.Run("handle an empty array", func(t *testing.T) {
+ c := srv.NewTcpClient()
+ defer func() { require.NoError(t, c.Close()) }()
+ require.NoError(t, c.Write("\r\n"))
+ require.NoError(t, c.Write("*1\r\n$4\r\nPING\r\n"))
+ r, err := c.ReadLine()
+ require.NoError(t, err)
+ require.Equal(t, "+PONG", r)
+ })
+
+ t.Run("out of range multibulk length", func(t *testing.T) {
+ c := srv.NewTcpClient()
+ defer func() { require.NoError(t, c.Close()) }()
+ require.NoError(t, c.Write("*20000000\r\n"))
+ r, err := c.ReadLine()
+ require.NoError(t, err)
+ require.Contains(t, r, "invalid multibulk length")
+ })
+
+ t.Run("wrong multibulk payload header", func(t *testing.T) {
+ c := srv.NewTcpClient()
+ defer func() { require.NoError(t, c.Close()) }()
+ require.NoError(t, c.Write("*3\r\n$3\r\nSET\r\n$1\r\nx\r\nfoo\r\n"))
+ r, err := c.ReadLine()
+ require.NoError(t, err)
+ require.Contains(t, r, "expected '$'")
+ })
+
+ t.Run("negative multibulk payload length", func(t *testing.T) {
+ c := srv.NewTcpClient()
+ defer func() { require.NoError(t, c.Close()) }()
+ require.NoError(t, c.Write("*3\r\n$3\r\nSET\r\n$1\r\nx\r\n$-10\r\n"))
+ r, err := c.ReadLine()
+ require.NoError(t, err)
+ require.Contains(t, r, "invalid bulk length")
+ })
+
+ t.Run("out of range multibulk payload length", func(t *testing.T) {
+ c := srv.NewTcpClient()
+ defer func() { require.NoError(t, c.Close()) }()
+ require.NoError(t, c.Write("*3\r\n$3\r\nSET\r\n$1\r\nx\r\n$2000000000\r\n"))
+ r, err := c.ReadLine()
+ require.NoError(t, err)
+ require.Contains(t, r, "invalid bulk length")
+ })
+
+ t.Run("non-number multibulk payload length", func(t *testing.T) {
+ c := srv.NewTcpClient()
+ defer func() { require.NoError(t, c.Close()) }()
+ require.NoError(t, c.Write("*3\r\n$3\r\nSET\r\n$1\r\nx\r\n$foo\r\n"))
+ r, err := c.ReadLine()
+ require.NoError(t, err)
+ require.Contains(t, r, "invalid bulk length")
+ })
+
+ t.Run("multibulk request not followed by bulk arguments", func(t *testing.T) {
+ c := srv.NewTcpClient()
+ defer func() { require.NoError(t, c.Close()) }()
+ require.NoError(t, c.Write("*1\r\nfoo\r\n"))
+ r, err := c.ReadLine()
+ require.NoError(t, err)
+ require.Contains(t, r, "expected '$'")
+ })
+
+ t.Run("generic wrong number of args", func(t *testing.T) {
+ rdb := srv.NewClient()
+ defer func() { require.NoError(t, rdb.Close()) }()
+ v := rdb.Do(context.Background(), "ping", "x", "y")
+ require.EqualError(t, v.Err(), "ERR wrong number of arguments")
+ })
+
+ t.Run("empty array parsed", func(t *testing.T) {
+ c := srv.NewTcpClient()
+ defer func() { require.NoError(t, c.Close()) }()
+ require.NoError(t, c.Write("*-1\r\n*3\r\n$3\r\nset\r\n$3\r\nkey\r\n$3\r\nval\r\n"))
+ r, err := c.ReadLine()
+ require.NoError(t, err)
+ require.Equal(t, r, "+OK")
+ })
+
+ t.Run("allow only LF protocol separator", func(t *testing.T) {
+ c := srv.NewTcpClient()
+ defer func() { require.NoError(t, c.Close()) }()
+ require.NoError(t, c.Write("set foo 123\n"))
+ r, err := c.ReadLine()
+ require.NoError(t, err)
+ require.Equal(t, r, "+OK")
+ })
+
+ t.Run("mix LF/CRLF protocol separator", func(t *testing.T) {
+ c := srv.NewTcpClient()
+ defer func() { require.NoError(t, c.Close()) }()
+ require.NoError(t, c.Write("*-1\r\nset foo 123\nget foo\r\n*3\r\n$3\r\nset\r\n$3\r\nkey\r\n$3\r\nval\r\n"))
+ for _, res := range []string{"+OK", "$3", "123", "+OK"} {
+ r, err := c.ReadLine()
+ require.NoError(t, err)
+ require.Equal(t, res, r)
+ }
+ })
+
+ t.Run("invalid LF in multi bulk protocol", func(t *testing.T) {
+ c := srv.NewTcpClient()
+ defer func() { require.NoError(t, c.Close()) }()
+ require.NoError(t, c.Write("*3\n$3\r\nset\r\n$3\r\nkey\r\n$3\r\nval\r\n"))
+ r, err := c.ReadLine()
+ require.NoError(t, err)
+ require.Contains(t, r, "invalid multibulk length")
+ })
+}
diff --git a/tests/gocase/unit/protocol/regression_test.go b/tests/gocase/unit/protocol/regression_test.go
new file mode 100644
index 0000000..3d17e51
--- /dev/null
+++ b/tests/gocase/unit/protocol/regression_test.go
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 protocol
+
+import (
+ "context"
+ "fmt"
+ "testing"
+
+ "github.com/apache/incubator-kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func TestRegression(t *testing.T) {
+ srv := util.StartServer(t, map[string]string{})
+ defer srv.Close()
+
+ ctx := context.Background()
+ rdb := srv.NewClient()
+ defer func() { require.NoError(t, rdb.Close()) }()
+
+ c := srv.NewTcpClient()
+ defer func() { require.NoError(t, c.Close()) }()
+
+ proto := "*3\r\n$5\r\nBLPOP\r\n$6\r\nhandle\r\n$1\r\n0\r\n"
+ require.NoError(t, c.Write(fmt.Sprintf("%s%s", proto, proto)))
+
+ resList := []string{"*2", "$6", "handle", "$1", "a"}
+
+ v := rdb.RPush(ctx, "handle", "a")
+ require.EqualValues(t, 1, v.Val())
+ for _, res := range resList {
+ r, err := c.ReadLine()
+ require.NoError(t, err)
+ require.Equal(t, res, r)
+ }
+
+ v = rdb.RPush(ctx, "handle", "a")
+ require.EqualValues(t, 1, v.Val())
+
+ // TODO should read the second pushed element
+ //for _, res := range resList {
+ // r, err := c.ReadLine()
+ // require.NoError(t, err)
+ // require.Equal(t, res, r)
+ //}
+}
diff --git a/tests/gocase/util/server.go b/tests/gocase/util/server.go
index 1b15572..7316fa6 100644
--- a/tests/gocase/util/server.go
+++ b/tests/gocase/util/server.go
@@ -15,7 +15,6 @@
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
- *
*/
package util
@@ -46,39 +45,39 @@ func (s *KvrocksServer) NewClient() *redis.Client {
return redis.NewClient(&redis.Options{Addr: s.addr.String()})
}
+func (s *KvrocksServer) NewTcpClient() *tcpClient {
+ c, err := net.Dial(s.addr.Network(), s.addr.String())
+ require.NoError(s.t, err)
+ return newTcpClient(c)
+}
+
func (s *KvrocksServer) Close() {
require.NoError(s.t, s.cmd.Process.Kill())
require.EqualError(s.t, s.cmd.Wait(), "signal: killed")
s.clean()
}
-func StartServer(t *testing.T, configs map[string]string) (*KvrocksServer, error) {
+func StartServer(t *testing.T, configs map[string]string) *KvrocksServer {
b := os.Getenv("KVROCKS_BIN_PATH")
cmd := exec.Command(b)
addr, err := findFreePort()
- if err != nil {
- return nil, err
- }
+ require.NoError(t, err)
configs["bind"] = addr.IP.String()
configs["port"] = fmt.Sprintf("%d", addr.Port)
dir := os.Getenv("GO_CASE_WORKSPACE")
require.NoError(t, err)
- dir, err = os.MkdirTemp(dir, "Server-*")
+ dir, err = os.MkdirTemp(dir, fmt.Sprintf("%s-%d-*", t.Name(), time.Now().UnixMilli()))
require.NoError(t, err)
configs["dir"] = dir
f, err := os.Create(filepath.Join(dir, "kvrocks.conf"))
- if err != nil {
- return nil, err
- }
+ require.NoError(t, err)
for k := range configs {
_, err := f.WriteString(fmt.Sprintf("%s %s\n", k, configs[k]))
- if err != nil {
- return nil, err
- }
+ require.NoError(t, err)
}
cmd.Args = append(cmd.Args, "-c", f.Name())
@@ -90,9 +89,7 @@ func StartServer(t *testing.T, configs map[string]string) (*KvrocksServer, error
require.NoError(t, err)
cmd.Stderr = stderr
- if err := cmd.Start(); err != nil {
- return nil, err
- }
+ require.NoError(t, cmd.Start())
c := redis.NewClient(&redis.Options{Addr: addr.String()})
defer func() { require.NoError(t, c.Close()) }()
@@ -108,7 +105,7 @@ func StartServer(t *testing.T, configs map[string]string) (*KvrocksServer, error
require.NoError(t, stdout.Close())
require.NoError(t, stderr.Close())
},
- }, nil
+ }
}
func findFreePort() (*net.TCPAddr, error) {
diff --git a/tests/gocase/util/tcp_client.go b/tests/gocase/util/tcp_client.go
new file mode 100644
index 0000000..0226dc7
--- /dev/null
+++ b/tests/gocase/util/tcp_client.go
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 util
+
+import (
+ "bufio"
+ "net"
+ "strings"
+)
+
+type tcpClient struct {
+ c net.Conn
+ r *bufio.Reader
+ w *bufio.Writer
+}
+
+func newTcpClient(c net.Conn) *tcpClient {
+ return &tcpClient{
+ c: c,
+ r: bufio.NewReader(c),
+ w: bufio.NewWriter(c),
+ }
+}
+
+func (c *tcpClient) Close() error {
+ return c.c.Close()
+}
+
+func (c *tcpClient) ReadLine() (string, error) {
+ r, err := c.r.ReadString('\n')
+ if err != nil {
+ return "", err
+ }
+ return strings.TrimSuffix(r, "\r\n"), nil
+}
+
+func (c *tcpClient) Write(s string) error {
+ _, err := c.w.WriteString(s)
+ if err != nil {
+ return err
+ }
+ return c.w.Flush()
+}
diff --git a/tests/tcl/tests/test_helper.tcl b/tests/tcl/tests/test_helper.tcl
index b037aa5..04c98c3 100644
--- a/tests/tcl/tests/test_helper.tcl
+++ b/tests/tcl/tests/test_helper.tcl
@@ -34,7 +34,6 @@ source tests/support/util.tcl
set ::all_tests {
unit/auth
- unit/protocol
unit/keyspace
unit/scan
unit/type/string
diff --git a/tests/tcl/tests/unit/protocol.tcl b/tests/tcl/tests/unit/protocol.tcl
deleted file mode 100644
index bb8b915..0000000
--- a/tests/tcl/tests/unit/protocol.tcl
+++ /dev/null
@@ -1,124 +0,0 @@
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements. See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership. The ASF licenses this file
-# to you 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.
-
-# Copyright (c) 2006-2020, Salvatore Sanfilippo
-# See bundled license file licenses/LICENSE.redis for details.
-
-# This file is copied and modified from the Redis project,
-# which started out as: https://github.com/redis/redis/blob/dbcc0a8/tests/unit/protocol.tcl
-
-start_server {tags {"protocol network"}} {
- test "Handle an empty query" {
- reconnect
- r write "\r\n"
- r flush
- assert_equal "PONG" [r ping]
- }
-
- test "Out of range multibulk length" {
- reconnect
- r write "*20000000\r\n"
- r flush
- assert_error "*invalid multibulk length*" {r read}
- }
-
- test "Wrong multibulk payload header" {
- reconnect
- r write "*3\r\n\$3\r\nSET\r\n\$1\r\nx\r\nfooz\r\n"
- r flush
- assert_error "*expected '$'*" {r read}
- }
-
- test "Negative multibulk payload length" {
- reconnect
- r write "*3\r\n\$3\r\nSET\r\n\$1\r\nx\r\n\$-10\r\n"
- r flush
- assert_error "*invalid bulk length*" {r read}
- }
-
- test "Out of range multibulk payload length" {
- reconnect
- r write "*3\r\n\$3\r\nSET\r\n\$1\r\nx\r\n\$2000000000\r\n"
- r flush
- assert_error "*invalid bulk length*" {r read}
- }
-
- test "Non-number multibulk payload length" {
- reconnect
- r write "*3\r\n\$3\r\nSET\r\n\$1\r\nx\r\n\$blabla\r\n"
- r flush
- assert_error "*invalid bulk length*" {r read}
- }
-
- test "Multi bulk request not followed by bulk arguments" {
- reconnect
- r write "*1\r\nfoo\r\n"
- r flush
- assert_error "*expected '$'*" {r read}
- }
-
- test "Generic wrong number of args" {
- reconnect
- assert_error "*wrong*arguments*" {r ping x y z}
- }
-
- test "Empty array parsed" {
- reconnect
- r write "*-1\r\n*3\r\n\$3\r\nset\r\n\$3\r\nkey\r\n\$3\r\nval\r\n"
- r flush
- assert_equal "OK" [r read]
- }
-
- test "Allow only LF protocol separator" {
- reconnect
- r write "set foo 123\n"
- r flush
- assert_equal "OK" [r read]
- }
-
- test "Mix LF/CRLF protocol separator" {
- reconnect
- r write "*-1\r\nset foo 123\nget foo\r\n*3\r\n\$3\r\nset\r\n\$3\r\nkey\r\n\$3\r\nval\r\n"
- r flush
- assert_equal "OK" [r read]
- assert_equal "123" [r read]
- assert_equal "OK" [r read]
- }
-
- test "invalid LF in multi bulk protocol" {
- reconnect
- r write "*3\n\$3\r\nset\r\n\$3\r\nkey\r\n\$3\r\nval\r\n"
- r flush
- assert_error "*invalid multibulk length*" {r read}
- }
-}
-
-start_server {tags {"regression"}} {
- test "Regression for a crash with blocking ops and pipelining" {
- set rd [redis_deferring_client]
- set fd [r channel]
- set proto "*3\r\n\$5\r\nBLPOP\r\n\$6\r\nnolist\r\n\$1\r\n0\r\n"
- puts -nonewline $fd $proto$proto
- flush $fd
- set res {}
-
- $rd rpush nolist a
- $rd read
- $rd rpush nolist a
- $rd read
- }
-}