pashagolub / pgxmock

pgx mock driver for golang to test database interactions
Other
393 stars 49 forks source link

add WithNamedArgs arg matcher to make working with pgx.NamedArgs easier #165

Closed dropwhile closed 1 year ago

dropwhile commented 1 year ago

Example usage:

mock.ExpectExec("INSERT INTO users").
    WithNamedArgs(pgx.NamedArgs{"name": "john", "created": AnyArg()}).
    WillReturnResult(NewResult("INSERT", 1))

closes #164

dropwhile commented 1 year ago

Alternatively, the WithArgs could simply be reused (a single pgx.NamedArgs argument) instead of adding WithNamedArgs. Now that I think about it, that might be clearer, but I will leave up to reviewer's preference either way. This version does benefit from a bit of additional type checking on the WithNamedArgs parameter.

pashagolub commented 1 year ago

You mean we can use smth like this?

mock.ExpectExec("INSERT INTO users").
    WithArgs(pgx.NamedArgs{"name": "john", "created": AnyArg()}).
    WillReturnResult(NewResult("INSERT", 1))
dropwhile commented 1 year ago

You mean we can use smth like this?

mock.ExpectExec("INSERT INTO users").
    WithArgs(pgx.NamedArgs{"name": "john", "created": AnyArg()}).
    WillReturnResult(NewResult("INSERT", 1))

Yeah. After ruminating a bit, I think this version is more consistent, as the pgx.NamedArgs is still an argument to pgx.Query.

pashagolub commented 1 year ago

Agree. If we can use old one and just check for the proper interface, it's better

dropwhile commented 1 year ago

Updated PR with support for QueryRewriter.

Example usage:

    mock.ExpectExec("INSERT INTO users").
        WithArgs("john", AnyTime{}).
        WillReturnResult(NewResult("INSERT", 1))

    _, err = mock.Exec(context.Background(),
        "INSERT INTO users(name, created_at) VALUES (@name, @time)",
        pgx.NamedArgs{"name": "john", "time": time.Now()},
    )

Note that WithArgs stays the same as previous, matching against expected arguments, but now after the pgx.Argument query rewriting has been done. The users do need to know the output order of the query, but knowing that seems to be not too big of a hurdle, and matches existing conventions, while still supporting usage of NamedArgs at the sql call sites.

I don't think we can apply the same query rewriting to queryBasedExpectation.args, as I don't think ArgMatchers would rewrite properly.

This should also support end users using their own custom QueryRewriters too.

dropwhile commented 1 year ago

I don't think we can apply the same query rewriting to queryBasedExpectation.args, as I don't think ArgMatchers would rewrite properly.

Actually.. It seems to work.

dropwhile commented 1 year ago

Updated PR again.

Now both of these versions work:

    mock.ExpectExec("INSERT INTO users").
        WithArgs("john", AnyTime{}).
        WillReturnResult(NewResult("INSERT", 1))

    _, err = mock.Exec(context.Background(),
        "INSERT INTO users(name, created_at) VALUES (@name, @time)",
        pgx.NamedArgs{"name": "john", "time": time.Now()},
    )
    mock.ExpectExec("INSERT INTO users").
        WithArgs(pgx.NamedArgs{"name": "john", "time": AnyTime{}}).
        WillReturnResult(NewResult("INSERT", 1))

    _, err = mock.Exec(context.Background(),
        "INSERT INTO users(name, created_at) VALUES (@name, @time)",
        pgx.NamedArgs{"name": "john", "time": time.Now()},
    )
pashagolub commented 1 year ago

Since your PR was read-only, I've created a new one #166

Please, tell me how do you feel about it?

dropwhile commented 1 year ago

@pashagolub sounds good to me. closing this PR in favor of yours. 👍