-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for LMPOP #2440
Add support for LMPOP #2440
Conversation
You should handle redis-server response data correctly. |
@monkey92t I'm a little confused about decoding the RESP array parts , In the other tests with array reply it looked straightforward however here the string slice is in that format , Do you have any suggestions for writing meaningful tests for that part? Is this something I should refer to ? https://redis.io/docs/reference/protocol-spec/#resp-arrays:~:text=the%20reply%20object).-,RESP%20Arrays,-Clients%20send%20commands |
@monkey92t Can you suggest a function I could refer to regarding Array Replies like these so I make tests similarly? |
You should look at the return value of the command: LMPOP 2 mylist mylist2 right count 3
*2
$6
mylist
*3
$3
one
$3
two
$5
three |
The type Data struct {
key string
Element [] string
} |
@monkey92t So I should change my approach from a stringSliceCmd to a custom struct correct? |
Do we already have a struct for running such complex structure command responses that I can re-use or do I need to create it as well? |
Yes, when you find that there is no suitable data structure in |
If you need a test environment, welcome to use the tool I made: "https://github.com/monkey92t/grte" 😁 |
@monkey92t Thanks a lot and sorry for bothering so many times, I am new to this I am trying to learn and help. |
OK, "https://github.com/monkey92t/grte" This tool is relatively simple, but use it to do some simple testing work, the following is an example: monkey@MonkeyiMac redis % grte go test ./...
[go-redis] 🐳 Prepare docker image ==> goredis/grte:v9
[go-redis] 🐳 Use image cache, ID ==> sha256:f4a6149b55e000ed40b5a68041615e7223b52119178d6ec4190f4714b8b88a49
[go-redis] 🐳 Create docker container...
[go-redis] 🐳 ContainerID: 28f0d7c79293a106bf0c54a76ea3a4ede41ee052332fe49263e24f8609a2aa7d
[go-redis] 🐳 WorkDir: /Users/monkey/Desktop/redis
[go-redis] 🐳 Command: [go test ./...]
go: downloading github.com/bsm/gomega v1.26.0
go: downloading github.com/bsm/ginkgo/v2 v2.7.0
go: downloading github.com/cespare/xxhash/v2 v2.2.0
go: downloading github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f
go: downloading github.com/stretchr/testify v1.8.1
go: downloading github.com/davecgh/go-spew v1.1.1
go: downloading github.com/pmezard/go-difflib v1.0.0
go: downloading gopkg.in/yaml.v3 v3.0.1
ok github.com/redis/go-redis/v9 103.717s
ok github.com/redis/go-redis/v9/internal 0.007s
ok github.com/redis/go-redis/v9/internal/hashtag 0.008s
ok github.com/redis/go-redis/v9/internal/hscan 0.009s
ok github.com/redis/go-redis/v9/internal/pool 1.589s
ok github.com/redis/go-redis/v9/internal/proto 0.009s
? github.com/redis/go-redis/v9/internal/rand [no test files]
? github.com/redis/go-redis/v9/internal/util [no test files]
[go-redis] ✅ Success! If you want to test other directories: monkey@MonkeyiMac redis % cd internal/pool
monkey@MonkeyiMac pool % grte go test ./...
[go-redis] 🐳 Prepare docker image ==> goredis/grte:v9
[go-redis] 🐳 Use image cache, ID ==> sha256:f4a6149b55e000ed40b5a68041615e7223b52119178d6ec4190f4714b8b88a49
[go-redis] 🐳 Create docker container...
[go-redis] 🐳 ContainerID: b094b74369998707dbcfeb128827a14dace8af67783460b5cbaa2d27d922eb45
[go-redis] 🐳 WorkDir: /Users/monkey/Desktop/redis/internal/pool
[go-redis] 🐳 Command: [go test ./...]
ok github.com/redis/go-redis/v9/internal/pool 1.529s
[go-redis] ✅ Success!
monkey@MonkeyiMac pool % hope it helps you.... |
Thanks a lot @monkey92t |
@monkey92t The test failure that I can see seems to be unrelated from the implementation or test, Can you take a look at it once? |
The |
@monkey92t I had referred to Line 2046 in 08b4cc5
To be able to use SliceCmd this was an approach but I guess now a custom struct is a nicer approach? |
Signed-off-by: monkey92t <golang@88.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Adding command and command test for LMPOP command.