go-redis / redismock

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

Mocked Pipelined with expected error does not match Redis' behavior #78

Open amayausky opened 9 months ago

amayausky commented 9 months ago

I'm attempting to test a Pipelined function that experiences a failure partway through execution of a set of commands. With Redis (and go-redis) the behavior seen is all commands are run, and each one returns a response. So if an error occurs partway through it does not stop execution of the other errors while returning its error with the response array. With redismock I'm seeing the execution stop after the first error.

Here are some sample tests:

func Test_RedisPipelined(t *testing.T) {
    c := getRedisClient(t) // returns a redis.NewClient
    defer c.Close()

    ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
    defer cancel()

    // prep keys
    if c.Set(ctx, "foo", "bar", 0).Err() != nil {
        t.Fatal("failed to set foo key")
    }
    if c.Set(ctx, "bar", "baz", 0).Err() != nil {
        t.Fatal("failed to set bar key")
    }

    cmds, err := c.Pipelined(ctx, func(p redis.Pipeliner) error {
        p.Get(ctx, "foo") // should exist
        p.Get(ctx, "baz") // should not exist
        p.Get(ctx, "bar") // should exist
        return nil
    })

    // go-redis returns the first error found in the chain as this error
    // other errors can also be returned
    if err != nil {
        if err != redis.Nil {
            t.Fatalf("expected error to be %v, got %v", redis.Nil, err)
        }
    } else {
        t.Fatal("expected error returned")
    }

    if len(cmds) != 3 {
        t.Fatalf("expected %v cmds, got %v", 3, len(cmds))
    }
    if cmds[0].Err() != nil {
        t.Fatalf("first command returned error: %v", cmds[0].Err())
    }
    if cmds[1].Err() == nil {
        t.Fatal("second command returned nil error")
    }
    if cmds[2].Err() != nil {
        t.Fatalf("third command returned error: %v", cmds[2].Err())
    }
}

func Test_MockedPipelined(t *testing.T) {
    db, mock := redismock.NewClientMock()

    mock.ExpectGet("foo").SetVal("foo")
    mock.ExpectGet("baz").RedisNil()
    mock.ExpectGet("bar").SetVal("bar")

    ctx := context.Background()
    db.Pipelined(ctx, func(p redis.Pipeliner) error {
        p.Get(ctx, "foo")
        p.Get(ctx, "baz")
        p.Get(ctx, "bar")
        return nil
    })

    if err := mock.ExpectationsWereMet(); err != nil {
        t.Error(err)
    }
}

The Test_RedisPipelined test which uses an actual Redis server succeeds, while the mock one fails with:

--- FAIL: Test_MockedPipelined (0.00s)
    main_test.go:133: there is a remaining expectation which was not matched: [get bar]
FAIL
leatral commented 7 months ago

I meet this error too.

While use redis/v9/Client's Pipeline().exec(), it finally call pipelineReadCmds() in redis/v9/osscluster.go. In this method it only skip remain command while meet non redis err, but ProcessPipelineHook() in redismock/v9/mock.go don't have this judgement. Which caused this inconsistence.