go-redis / redismock

Redis client Mock
https://redis.uptrace.dev
BSD 2-Clause "Simplified" License
283 stars 63 forks source link

Commands which return slices cannot be mocked #27

Open ideasculptor opened 2 years ago

ideasculptor commented 2 years ago

There are plenty of commands mocked which purport to return StringSlice or IntSlice, but there are no tests which exercise them. When I attempted to test code which returns a slice from a command, I always receive an error:

redis: unexpected type=[]int64 for Slice

which is not at all surprising when you look at the code here: https://github.com/go-redis/redis/blob/cc09f96b8fcabfa25bff2654209c8b711cc9c561/command.go#L375

It always stores the response val for a command as []interface{} and only creates a strongly typed slice after first selecting for []interface{} and then copying each value into a new, strongly typed slice. A strongly typed slice does not satisfy []interface{}

The mock code, on the other hand, assigns a strongly typed slice directly into the command's return val, which causes the call to Slice() to always throw an exception about the wrong type. It seems that SetVal for ExpectedIntSlice and ExpectedStringSlice should both take []interface{} rather than []int64 and []string respectively, or else it should copy into an []interface{} when copying the slice passed to SetVal.

ideasculptor commented 2 years ago

It seems like maybe the redis client itself is broken when handling slices. I realized my new code in redismock was using redis.NewCmd instead of redis.NewIntSliceCmd, and I thought that might be the source of the problem. However, when I corrected it, I went right back to the error about unexpected type=[]int64 for Slice because the client code always calls Slice() from within Int64Slice(). However, if one looks at IntSliceCmd, it definitely creates an []int64 from the response, which makes me think it is always going to fail to return the value becaue []int64 doesn't satisfy the []interface{} check in Slice(). The same problem appears to exist for all slice commands except plain SliceCmd since those actually use []interface{}

func (cmd *Cmd) Slice() ([]interface{}, error) {
    if cmd.err != nil {
        return nil, cmd.err
    }
    switch val := cmd.val.(type) {
    case []interface{}:
        return val, nil
    default:
        return nil, fmt.Errorf("redis: unexpected type=%T for Slice", val)
    }
}

func (cmd *Cmd) Int64Slice() ([]int64, error) {
    slice, err := cmd.Slice()
    if err != nil {
        return nil, err
    }

    nums := make([]int64, len(slice))
    for i, iface := range slice {
        val, err := toInt64(iface)
        if err != nil {
            return nil, err
        }
        nums[i] = val
    }
    return nums, nil
}
func (cmd *IntSliceCmd) readReply(rd *proto.Reader) error {
    _, err := rd.ReadArrayReply(func(rd *proto.Reader, n int64) (interface{}, error) {
        cmd.val = make([]int64, n)
        for i := 0; i < len(cmd.val); i++ {
            num, err := rd.ReadIntReply()
            if err != nil {
                return nil, err
            }
            cmd.val[i] = num
        }
        return nil, nil
    })
    return err
}
ideasculptor commented 2 years ago

This problem only exists for my patched code in https://github.com/go-redis/redismock/pull/26, which implements ExpectIntSliceDo() and ExpectStringSliceDo(), and then only because the code being tested attempts to use Cmd.Int64Slice() and Cmd.StringSlice(). Those methods are broken in the redis client itself (https://github.com/go-redis/redis/issues/2007 ) because of the mistaken type check against []interface{}.

That code does function correctly so long as the code being tested doesn't attempt to use Cmd.Int64Slice() and Cmd.StringSlice() and the problem with those methods should be corrected in the redis client itself.