You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by GitBox <gi...@apache.org> on 2022/09/06 19:46:41 UTC
[GitHub] [incubator-kvrocks] suica opened a new pull request, #833: Rewrite test unit/type/list-2 in Go
suica opened a new pull request, #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833
fixes #817
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964359408
##########
tests/gocase/unit/type/list/list-2_test.go:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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/type/list-2.tcl
+ */
+
+package list
+
+import (
+ "context"
+ "fmt"
+ "math/rand"
+ "strconv"
+ "testing"
+
+ "github.com/apache/incubator-kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func randInt64Str() string {
+ return strconv.FormatInt(rand.Int63(), 10)
+}
+
+func min(a, b int) int {
Review Comment:
It will be wonderful if you could implement a generic version and put into tests/gocase/util. So many pain in golang 😢
i.e. `func min[T constraints.Ordered](a, b T) T`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] suica commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
suica commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964115773
##########
tests/gocase/unit/type/list/list-2_test.go:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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/type/list-2.tcl
+ */
+
+package list
+
+import (
+ "context"
+ "fmt"
+ "math/rand"
+ "strconv"
+ "testing"
+
+ "github.com/apache/incubator-kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func randInt64Str() string {
+ return strconv.FormatInt(rand.Int63(), 10)
+}
+
+func min(a, b int) int {
Review Comment:
Is it ok to place utils here, or such things should be placed elsewhere?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] suica commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
suica commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964113973
##########
tests/gocase/unit/type/list/list-2_test.go:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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/type/list-2.tcl
+ */
+
+package list
+
+import (
+ "context"
+ "fmt"
+ "math/rand"
+ "strconv"
+ "testing"
+
+ "github.com/apache/incubator-kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func randInt64Str() string {
+ return strconv.FormatInt(rand.Int63(), 10)
+}
+
+func min(a, b int) int {
+ if a < b {
+ return a
+ }
+ return b
+}
+
+func Test(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()) }()
+
+ key := "myList"
+ rand.Seed(0)
+ for typ, largeStr := range LargeValue {
+ t.Run(fmt.Sprintf("LTRIM stress testing - %s", typ), func(t *testing.T) {
+ myList := []string{}
+ startLen := 32
+ rdb.Del(ctx, key)
+ rdb.RPush(ctx, key, largeStr)
+ myList = append(myList, largeStr)
+ for i := 0; i < 1000; i++ {
+ s := randInt64Str()
+ rdb.RPush(ctx, key, s)
+ myList = append(myList, s)
+ }
+ for i := 0; i < 1000; i++ {
+ low := int(rand.Float64() * float64(startLen))
+ high := int(float64(low) + rand.Float64()*float64(startLen))
+ myList = myList[low:min(high+1, len(myList))]
Review Comment:
Here I clamp the end index by `len(myList)`, since original tcl test case uses `lrange` function with it an inclusive end index. Otherwise, when `cap(myList)>=high>len(myList)`, result of `myList[low:high+1]` is not as expected or even panics if `cap(myList)==high`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] suica commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
suica commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964370925
##########
tests/gocase/unit/type/list/list-2_test.go:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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/type/list-2.tcl
+ */
+
+package list
+
+import (
+ "context"
+ "fmt"
+ "math/rand"
+ "strconv"
+ "testing"
+
+ "github.com/apache/incubator-kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func randInt64Str() string {
+ return strconv.FormatInt(rand.Int63(), 10)
+}
+
+func min(a, b int) int {
Review Comment:
> I'd prefer to use an existing mature lib like https://pkg.go.dev/modernc.org/mathutil#Min.
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] suica commented on pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
suica commented on PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#issuecomment-1238903202
> For the first time I know that Go files can have `-` in name :)
Maybe we need to use `list_common.go` & `list_2_test.go` to comply the Go naming conventions?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun commented on pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#issuecomment-1238904255
@suica I don't have a strong feeling as long as it can work.
My preference is:
```
list
- common.go
- list_test.go
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964402952
##########
tests/gocase/go.mod:
##########
@@ -7,10 +7,13 @@ require (
github.com/stretchr/testify v1.8.0
)
+require github.com/remyoudompheng/bigfft v0.0.0-20200410134404-eec4a21b6bb0 // indirect
+
require (
github.com/cespare/xxhash/v2 v2.1.2 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
+ modernc.org/mathutil v1.5.0
Review Comment:
I think you can put it in the first `require` clause, since it is not a indirect package.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun commented on pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#issuecomment-1238983729
Thanks for your contribution! Merging...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964367803
##########
tests/tcl/tests/unit/type/list-2.tcl:
##########
@@ -1,70 +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/type/list-2.tcl
-
-start_server {
- tags {"list"}
- overrides {
- "list-max-ziplist-size" 4
- }
Review Comment:
Interesting. It seems we don't populate this config in the new test case but it still passes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
git-hulk commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964357217
##########
tests/gocase/unit/type/list/list-2_test.go:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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/type/list-2.tcl
+ */
+
+package list
+
+import (
+ "context"
+ "fmt"
+ "math/rand"
+ "strconv"
+ "testing"
+
+ "github.com/apache/incubator-kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func randInt64Str() string {
+ return strconv.FormatInt(rand.Int63(), 10)
+}
+
+func min(a, b int) int {
Review Comment:
How about moving them into [tests/gocase/util](https://github.com/apache/incubator-kvrocks/tree/unstable/tests/gocase/util)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] suica commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
suica commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964113973
##########
tests/gocase/unit/type/list/list-2_test.go:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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/type/list-2.tcl
+ */
+
+package list
+
+import (
+ "context"
+ "fmt"
+ "math/rand"
+ "strconv"
+ "testing"
+
+ "github.com/apache/incubator-kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func randInt64Str() string {
+ return strconv.FormatInt(rand.Int63(), 10)
+}
+
+func min(a, b int) int {
+ if a < b {
+ return a
+ }
+ return b
+}
+
+func Test(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()) }()
+
+ key := "myList"
+ rand.Seed(0)
+ for typ, largeStr := range LargeValue {
+ t.Run(fmt.Sprintf("LTRIM stress testing - %s", typ), func(t *testing.T) {
+ myList := []string{}
+ startLen := 32
+ rdb.Del(ctx, key)
+ rdb.RPush(ctx, key, largeStr)
+ myList = append(myList, largeStr)
+ for i := 0; i < 1000; i++ {
+ s := randInt64Str()
+ rdb.RPush(ctx, key, s)
+ myList = append(myList, s)
+ }
+ for i := 0; i < 1000; i++ {
+ low := int(rand.Float64() * float64(startLen))
+ high := int(float64(low) + rand.Float64()*float64(startLen))
+ myList = myList[low:min(high+1, len(myList))]
Review Comment:
Here I clamp the end index by `len(myList)`, since original tcl test case uses `lrange` function with an inclusive end index. Otherwise, when `cap(myList)>=high>len(myList)`, result of `myList[low:high+1]` will be not as expected or even panics if `cap(myList)<=high`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun commented on pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#issuecomment-1238909999
@suica I guess those numbers are there for historical issues. We can firstly add them add together, and if there're some issues we should split tests, we write it with a meaningful name instead of counting.
Previously, I split command.tcl to package command and info, and protocol.tcl into two files.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] suica commented on pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
suica commented on PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#issuecomment-1238907461
Oh yes, that naming would be more concise and consistent.
And since the file is about the cases from `list-2.tcl`, how about keeping the number in its name like this?
```
list
- common.go
- list_2_test.go
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] suica commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
suica commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964405158
##########
tests/gocase/go.mod:
##########
@@ -7,10 +7,13 @@ require (
github.com/stretchr/testify v1.8.0
)
+require github.com/remyoudompheng/bigfft v0.0.0-20200410134404-eec4a21b6bb0 // indirect
Review Comment:
Oops, maybe it is because that i misplaced the `modernc.org/mathutil v1.5.0` to the indirect packages area... I'll fix both of them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] suica commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
suica commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964406781
##########
tests/gocase/go.mod:
##########
@@ -7,10 +7,13 @@ require (
github.com/stretchr/testify v1.8.0
)
+require github.com/remyoudompheng/bigfft v0.0.0-20200410134404-eec4a21b6bb0 // indirect
+
require (
github.com/cespare/xxhash/v2 v2.1.2 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
+ modernc.org/mathutil v1.5.0
Review Comment:
also fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] suica commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
suica commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964374147
##########
tests/tcl/tests/unit/type/list-2.tcl:
##########
@@ -1,70 +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/type/list-2.tcl
-
-start_server {
- tags {"list"}
- overrides {
- "list-max-ziplist-size" 4
- }
Review Comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun commented on pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#issuecomment-1238889812
For the first time I know that Go files can have `-` in name :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] suica commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
suica commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964115773
##########
tests/gocase/unit/type/list/list-2_test.go:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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/type/list-2.tcl
+ */
+
+package list
+
+import (
+ "context"
+ "fmt"
+ "math/rand"
+ "strconv"
+ "testing"
+
+ "github.com/apache/incubator-kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func randInt64Str() string {
+ return strconv.FormatInt(rand.Int63(), 10)
+}
+
+func min(a, b int) int {
Review Comment:
Is it ok to place utils here, or this such things should be placed elsewhere?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#issuecomment-1238953692
> cc @PragmaTwice I notice that test like "LTRIM stress testing" can adopt a Go benchmark flavor and when you wire it into `x.py`, you may think of add `-bench=.` if possible.
The script will forward all unparsed options, e.g.
```
./x.py test go build -bench=...
```
equals to
```
go test -v ./... -bench=...
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964359408
##########
tests/gocase/unit/type/list/list-2_test.go:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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/type/list-2.tcl
+ */
+
+package list
+
+import (
+ "context"
+ "fmt"
+ "math/rand"
+ "strconv"
+ "testing"
+
+ "github.com/apache/incubator-kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func randInt64Str() string {
+ return strconv.FormatInt(rand.Int63(), 10)
+}
+
+func min(a, b int) int {
Review Comment:
It will be wonderful if you could implement a generic version and put into tests/gocase/util. So many pain in golang 😢
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964368173
##########
tests/tcl/tests/unit/type/list-2.tcl:
##########
@@ -1,70 +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/type/list-2.tcl
-
-start_server {
- tags {"list"}
- overrides {
- "list-max-ziplist-size" 4
- }
Review Comment:
To populate config, see:
https://github.com/apache/incubator-kvrocks/blob/3f0fb401030332aff76a4d2638887cbf0de240fc/tests/gocase/unit/auth/auth_test.go#L45-L47
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun merged pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
tisonkun merged PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
git-hulk commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964357217
##########
tests/gocase/unit/type/list/list-2_test.go:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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/type/list-2.tcl
+ */
+
+package list
+
+import (
+ "context"
+ "fmt"
+ "math/rand"
+ "strconv"
+ "testing"
+
+ "github.com/apache/incubator-kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func randInt64Str() string {
+ return strconv.FormatInt(rand.Int63(), 10)
+}
+
+func min(a, b int) int {
Review Comment:
How about moving them into `https://github.com/apache/incubator-kvrocks/tree/unstable/tests/gocase/util`?
##########
tests/gocase/unit/type/list/list-2_test.go:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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/type/list-2.tcl
+ */
+
+package list
+
+import (
+ "context"
+ "fmt"
+ "math/rand"
+ "strconv"
+ "testing"
+
+ "github.com/apache/incubator-kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func randInt64Str() string {
+ return strconv.FormatInt(rand.Int63(), 10)
+}
+
+func min(a, b int) int {
Review Comment:
How about moving them into https://github.com/apache/incubator-kvrocks/tree/unstable/tests/gocase/util?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964367489
##########
tests/gocase/unit/type/list/list-2_test.go:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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/type/list-2.tcl
+ */
+
+package list
+
+import (
+ "context"
+ "fmt"
+ "math/rand"
+ "strconv"
+ "testing"
+
+ "github.com/apache/incubator-kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func randInt64Str() string {
+ return strconv.FormatInt(rand.Int63(), 10)
+}
+
+func min(a, b int) int {
Review Comment:
I'd prefer to use an existing mature lib like https://pkg.go.dev/modernc.org/mathutil#Min.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] suica commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
suica commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964113973
##########
tests/gocase/unit/type/list/list-2_test.go:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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/type/list-2.tcl
+ */
+
+package list
+
+import (
+ "context"
+ "fmt"
+ "math/rand"
+ "strconv"
+ "testing"
+
+ "github.com/apache/incubator-kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func randInt64Str() string {
+ return strconv.FormatInt(rand.Int63(), 10)
+}
+
+func min(a, b int) int {
+ if a < b {
+ return a
+ }
+ return b
+}
+
+func Test(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()) }()
+
+ key := "myList"
+ rand.Seed(0)
+ for typ, largeStr := range LargeValue {
+ t.Run(fmt.Sprintf("LTRIM stress testing - %s", typ), func(t *testing.T) {
+ myList := []string{}
+ startLen := 32
+ rdb.Del(ctx, key)
+ rdb.RPush(ctx, key, largeStr)
+ myList = append(myList, largeStr)
+ for i := 0; i < 1000; i++ {
+ s := randInt64Str()
+ rdb.RPush(ctx, key, s)
+ myList = append(myList, s)
+ }
+ for i := 0; i < 1000; i++ {
+ low := int(rand.Float64() * float64(startLen))
+ high := int(float64(low) + rand.Float64()*float64(startLen))
+ myList = myList[low:min(high+1, len(myList))]
Review Comment:
Here I clamp the end index by `len(myList)`, since original tcl test case uses `lrange` function with it an inclusive end index. Otherwise, when `cap(myList)>=high>len(myList)`, result of `myList[low:high+1]` is not as expected or even panics.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964359408
##########
tests/gocase/unit/type/list/list-2_test.go:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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/type/list-2.tcl
+ */
+
+package list
+
+import (
+ "context"
+ "fmt"
+ "math/rand"
+ "strconv"
+ "testing"
+
+ "github.com/apache/incubator-kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func randInt64Str() string {
+ return strconv.FormatInt(rand.Int63(), 10)
+}
+
+func min(a, b int) int {
Review Comment:
It will be wonderful if you could implement a generic version and put into tests/gocase/util.
i.e. `func min[T constraints.Ordered](a, b T) T`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964359408
##########
tests/gocase/unit/type/list/list-2_test.go:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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/type/list-2.tcl
+ */
+
+package list
+
+import (
+ "context"
+ "fmt"
+ "math/rand"
+ "strconv"
+ "testing"
+
+ "github.com/apache/incubator-kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func randInt64Str() string {
+ return strconv.FormatInt(rand.Int63(), 10)
+}
+
+func min(a, b int) int {
Review Comment:
It will be wonderful if you could implement a generic version and put into tests/gocase/util. So many pain in golang.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964402495
##########
tests/gocase/go.mod:
##########
@@ -7,10 +7,13 @@ require (
github.com/stretchr/testify v1.8.0
)
+require github.com/remyoudompheng/bigfft v0.0.0-20200410134404-eec4a21b6bb0 // indirect
Review Comment:
Just curious, why there is a new `require` clause rather than a single `require` with all indirect packages?
##########
tests/gocase/go.mod:
##########
@@ -7,10 +7,13 @@ require (
github.com/stretchr/testify v1.8.0
)
+require github.com/remyoudompheng/bigfft v0.0.0-20200410134404-eec4a21b6bb0 // indirect
Review Comment:
Just curious, why there is a new `require` clause rather than a single `require` with all indirect packages? :rofl:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun commented on pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#issuecomment-1238950361
@suica I send a patch onto your PR https://github.com/suica/incubator-kvrocks/pull/1.
cc @PragmaTwice I notice that test like "LTRIM stress testing" can adopt a Go benchmark flavor and when you wire it into `x.py`, you may think of add `-bench=.` if possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964372224
##########
tests/gocase/util/random.go:
##########
@@ -0,0 +1,29 @@
+/*
+ * 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 (
+ "math/rand"
+ "strconv"
+)
+
+func RandInt64Str() string {
+ return strconv.FormatInt(rand.Int63(), 10)
+}
Review Comment:
You can just inline this oneliner instead of a handy utility.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] suica commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
suica commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964113973
##########
tests/gocase/unit/type/list/list-2_test.go:
##########
@@ -0,0 +1,88 @@
+/*
+ * 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/type/list-2.tcl
+ */
+
+package list
+
+import (
+ "context"
+ "fmt"
+ "math/rand"
+ "strconv"
+ "testing"
+
+ "github.com/apache/incubator-kvrocks/tests/gocase/util"
+ "github.com/stretchr/testify/require"
+)
+
+func randInt64Str() string {
+ return strconv.FormatInt(rand.Int63(), 10)
+}
+
+func min(a, b int) int {
+ if a < b {
+ return a
+ }
+ return b
+}
+
+func Test(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()) }()
+
+ key := "myList"
+ rand.Seed(0)
+ for typ, largeStr := range LargeValue {
+ t.Run(fmt.Sprintf("LTRIM stress testing - %s", typ), func(t *testing.T) {
+ myList := []string{}
+ startLen := 32
+ rdb.Del(ctx, key)
+ rdb.RPush(ctx, key, largeStr)
+ myList = append(myList, largeStr)
+ for i := 0; i < 1000; i++ {
+ s := randInt64Str()
+ rdb.RPush(ctx, key, s)
+ myList = append(myList, s)
+ }
+ for i := 0; i < 1000; i++ {
+ low := int(rand.Float64() * float64(startLen))
+ high := int(float64(low) + rand.Float64()*float64(startLen))
+ myList = myList[low:min(high+1, len(myList))]
Review Comment:
Here I clamp the end index by `len(myList)`, since original tcl test case uses `lrange` function with it an inclusive end index. Otherwise, when `cap(myList)>=high>len(myList)`, result `myList[low:high+1]` is not expected or even panics.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964402952
##########
tests/gocase/go.mod:
##########
@@ -7,10 +7,13 @@ require (
github.com/stretchr/testify v1.8.0
)
+require github.com/remyoudompheng/bigfft v0.0.0-20200410134404-eec4a21b6bb0 // indirect
+
require (
github.com/cespare/xxhash/v2 v2.1.2 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
+ modernc.org/mathutil v1.5.0
Review Comment:
I think you can put it in the first `require` clause, since it is not an indirect package.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] suica commented on a diff in pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
suica commented on code in PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#discussion_r964374947
##########
tests/gocase/util/random.go:
##########
@@ -0,0 +1,29 @@
+/*
+ * 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 (
+ "math/rand"
+ "strconv"
+)
+
+func RandInt64Str() string {
+ return strconv.FormatInt(rand.Int63(), 10)
+}
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] tisonkun commented on pull request #833: Rewrite test unit/type/list-2 in Go
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #833:
URL: https://github.com/apache/incubator-kvrocks/pull/833#issuecomment-1238974333
@PragmaTwice yes. We can discuss in detail in the dedicated PR. Just FYI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org