feat: minor increase in rueidiscompat coverage #583
Conversation
message.go
Outdated
| @@ -1386,3 +1386,8 @@ func (m *prettyRedisMessage) MarshalJSON() ([]byte, error) { | |||
| } | |||
| return json.Marshal(obj) | |||
| } | |||
|
|
|||
| // NewMockResult is a mock method to send back RedisResult with err for test coverage | |||
| func NewMockResult(val RedisMessage, err error) RedisResult { | |||
There was a problem hiding this comment.
I have mixed thoughts about this
Because even though this will provide us a way to cover roughly 20 more lines of code in ruediscompat command, we should not probably expose this as a part of rueidis package
But the only alternative to cover most of the other parts inside command would be to introduce interfaces instead of passing concrete structs and a large refactor. Otherwise, it seems difficult to achieve near-100% coverage.
What do you think? @rueian
There was a problem hiding this comment.
we should not probably expose this as a part of rueidis package
Agreed. Actually, we don't need this function but we can use the same hack of the mock package instead:
Lines 22 to 25 in 45cdf7e
There was a problem hiding this comment.
Oh wow
This is a great use of unsafePtrs
rueidiscompat/command_test.go
Outdated
| @@ -29,6 +29,8 @@ package rueidiscompat | |||
| import ( | |||
| "errors" | |||
| "fmt" | |||
| "github.com/redis/rueidis" | |||
There was a problem hiding this comment.
The import should be moved to below.
rueidiscompat/command_test.go
Outdated
| @@ -29,6 +29,8 @@ package rueidiscompat | |||
| import ( | |||
| "errors" | |||
| "fmt" | |||
| "github.com/redis/rueidis" | |||
| "github.com/stretchr/testify/assert" | |||
There was a problem hiding this comment.
Could we avoid introducing a new test dependency?
SoulPancake
changed the title
| @@ -8,13 +8,15 @@ require ( | |||
| github.com/onsi/ginkgo/v2 v2.15.0 | |||
| github.com/onsi/gomega v1.31.1 | |||
| github.com/redis/rueidis v1.0.40 | |||
| github.com/redis/rueidis/mock v1.0.40 | |||
There was a problem hiding this comment.
Please also add a local replace directive for this.
| @@ -3,6 +3,7 @@ module github.com/redis/rueidis/rueidiscompat | |||
| go 1.20 | |||
|
|
|||
| replace github.com/redis/rueidis => ../ | |||
| replace github.com/redis/rueidis/mock => ../mock | |||
There was a problem hiding this comment.
We should run go mod tidy again after adding the replace directive.
|
LGTM. Thanks @SoulPancake! |
part of #487