Transactions and rollbacks after commits

[alexeyco]

Is your feature request related to a problem? Please describe. When we need to mock transactions made with BeginFunc, we should mock rollback after commit:

        conn.ExpectBegin()
        conn.ExpectExec("DELETE FROM table WHERE id = $1").
            WithArgs(123).
            WillReturnResult(pgxmock.NewResult("DELETE", 1))
        conn.ExpectCommit()
        conn.ExpectRollback()

If we don't do this, we will receive should be nil, "all expectations were already fulfilled, call to Rollback transaction was not expected" given. Is it correct way to avoid this error?

[pashagolub]

Hi,

would you please provide the source code we're actually trying to test?

Thanks

[alexeyco]

Ok, her is a simple example.

repo.go:

package repo

import (
    "context"

    "github.com/jackc/pgx/v4"
)

type Conn interface {
    BeginFunc(context.Context, func(pgx.Tx) error) error
}

type Repo interface {
    Delete(ctx context.Context, id int64) error
}

type repo struct {
    conn Conn
}

func (r *repo) Delete(ctx context.Context, id int64) error {
    return r.conn.BeginFunc(ctx, func(tx pgx.Tx) error {
        _, err := tx.Exec(ctx, "DELETE FROM entities WHERE id = $1", id)
        return err
    })
}

func New(conn Conn) Repo {
    return &repo{
        conn: conn,
    }
}

repo_test.go:

package repo_test

import (
    "context"
    "errors"
    "testing"

    "repo"
    "github.com/pashagolub/pgxmock"
    "github.com/stretchr/testify/assert"
)

func mockConnection(t *testing.T) pgxmock.PgxConnIface {
    t.Helper()

    conn, err := pgxmock.NewConn(pgxmock.QueryMatcherOption(pgxmock.QueryMatcherEqual))
    assert.NoError(t, err)

    conn.MatchExpectationsInOrder(true)

    return conn
}

func TestRepo_Delete(t *testing.T) {
    t.Parallel()

    var expectedId int64 = 123

    t.Run("Ok", func(t *testing.T) {
        t.Parallel()

        conn := mockConnection(t)

        conn.ExpectBegin()
        conn.ExpectExec("DELETE FROM entities WHERE id = $1").
            WithArgs(expectedId).
            WillReturnResult(pgxmock.NewResult("DELETE", 1))
        conn.ExpectCommit()
        conn.ExpectRollback() // Without this one tests will fail

        err := repo.New(conn).
            Delete(context.Background(), expectedId)

        assert.NoError(t, err)
    })

    t.Run("ErrorCauseExecError", func(t *testing.T) {
        t.Parallel()

        expectedErr := errors.New("i am error")

        conn := mockConnection(t)

        conn.ExpectBegin()
        conn.ExpectExec("DELETE FROM entities WHERE id = $1").
            WithArgs(expectedId).
            WillReturnError(expectedErr)
        conn.ExpectRollback()
        conn.ExpectRollback() // Without this one tests will fail

        err := repo.New(conn).
            Delete(context.Background(), expectedId)

        assert.Error(t, err)
        assert.Equal(t, expectedErr, err)
    })
}

[pashagolub]

After a little investigation:

func (tx *dbTx) BeginFunc(ctx context.Context, f func(Tx) error) (err error) {
    if tx.closed {
        return ErrTxClosed
    }

    var savepoint Tx
    savepoint, err = tx.Begin(ctx)
    if err != nil {
        return err
    }
    defer func() {
        rollbackErr := savepoint.Rollback(ctx)
        if !(rollbackErr == nil || errors.Is(rollbackErr, ErrTxClosed)) {
            err = rollbackErr
        }
    }()

    fErr := f(savepoint)
    if fErr != nil {
        _ = savepoint.Rollback(ctx) // ignore rollback error as there is already an error to return
        return fErr
    }

    return savepoint.Commit(ctx)
}

We can see a deferred function which does Rollback() in any case. I assume that was the architectural choice of @jackc. So my answer would be:

• if you use BeginFunc() keep in mind there is hidden Rollback()

[alexeyco]

Got it, thanks!