pashagolub / pgxmock

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

[!] add support for `pgx.Batch`, closes #152 #179

Closed makubit closed 9 months ago

makubit commented 11 months ago

This PR introduces functionality to properly test SendBatch() function. In the scope of this PR, there was created new helper structs:BatchElement, Batch, and BatchResults.

An example, how to use batches in tests:

// defining the batch that will be passed to the SendBatch() function
    expectedBatch := mock.NewBatch().
        AddBatchElements(
            NewBatchElement("SELECT *", 1),
            NewBatchElement("SELECT *"),
        )

// using rows implementation to properly simulate Query() and QueryRow() functions in BatchResults interface
    rows := NewRows([]string{"id", "name", "email"}).
        AddRow("some-id-1", "some-name-1", "some-email-1").
        AddRow("some-id-2", "some-name-2", "some-email-2")

// mock for pgx.BatchResults
    batchResultsMock := NewBatchResults().WillReturnRows(rows).AddCommandTag(pgconn.NewCommandTag("SELECT 2"))

// real batch call, that we mocked above
    batch := new(pgx.Batch)
    batch.Queue("SELECT * FROM TABLE", 1)
    batch.Queue("SELECT * FROM TABLE")

// our expectation accepts Batch mock and BatchResults mock (in additional builder function) as parameters
    mock.ExpectSendBatch(expectedBatch).
        WillReturnBatchResults(batchResultsMock)

// the call of the function, here we can use br.AnyFunctionCall() implemented in pgx.BatchResults interface
    br := mock.SendBatch(context.Background(), batch)
    a.NotNil(br)
    r, err := br.Query()

I would appreciate any comments and feedback. ~Magda

pashagolub commented 11 months ago

@makubit would you please check the golint warnings and fix them? Not urgent, just to know where we are with your PR :)

makubit commented 11 months ago

Yes, I will look at the warnings later this week, I was pretty busy recently, so I didn't have much time to look into that.

pashagolub commented 11 months ago

No problem at all! Take your time! I'm not planning to spend much time until the new year anyway :)

makubit commented 10 months ago

@pashagolub PR should be fine now, I resolved the linter issues. Starting this week I have more spare time, so I should be more responsive in case there are some other comments/issues.

pashagolub commented 10 months ago

Also raised jackc/pgx#1878 discussion about making Batch and QueuedQuery properties public

pashagolub commented 10 months ago

Something I cannot implement with the new functionality. Would you please help me to build a test case for the code snippet from the pgx manual:

conn, err := pgx.Connect(ctx, os.Getenv("PGX_TEST_DATABASE"))
if err != nil {
    fmt.Printf("Unable to establish connection: %v", err)
    return
}

batch := &pgx.Batch{}
batch.Queue("select 1 + 1").QueryRow(func(row pgx.Row) error {
    var n int32
    err := row.Scan(&n)
    if err != nil {
        return err
    }

    fmt.Println(n)

    return err
})

batch.Queue("select 1 + 2").QueryRow(func(row pgx.Row) error {
    var n int32
    err := row.Scan(&n)
    if err != nil {
        return err
    }

    fmt.Println(n)

    return err
})

batch.Queue("select 2 + 3").QueryRow(func(row pgx.Row) error {
    var n int32
    err := row.Scan(&n)
    if err != nil {
        return err
    }

    fmt.Println(n)

    return err
})

err = conn.SendBatch(ctx, batch).Close()
if err != nil {
    fmt.Printf("SendBatch error: %v", err)
    return
}

The trick here is every QueuedQuery returns at least one row. But in your code we have only one result set for all queued queries. Am I correct?

makubit commented 9 months ago

Hi, yes, you are correct, I didn't notice that. I will work on some better solutions, so that QueryRow() can have more sophisticated results :)

pashagolub commented 9 months ago

No problem!

I didn't notice that.

Me neither. But before you'll spend more time on this, I have an idea, that this feature should look like ExpectedPrepare. In fact the current implementation of ExpectedPrepare should be deleted, it's just a leftover of how lib/sql works.

So in a few words, we just can replace implementation of ExpectedPrepare with ExpectedBatch plus some renaming and tweaks.

I understand it basically means to throw your code away, sorry about that. I will completely understand if you won't work on this implementation. On the other hand I listed this feature for this year GSoC. Maybe it's worth to work on it during the GSoC program? If you're eligible to be a participant I'd love to accept your proposal. If you're not eligible for participating but eligible to be a mentor, I'd love to onboard you as a GSoC mentor. :)