From 4ee8b12ce2ef76af35fe83315aba49504278eb62 Mon Sep 17 00:00:00 2001 From: Charlie Tonneslan Date: Fri, 29 May 2026 08:29:04 -0400 Subject: [PATCH] fix: ExpectBegin TxOptions check uses && instead of || (closes #342) The Begin path required both Isolation AND ReadOnly to differ before the "options do not match" error fired, so any single-field mismatch silently passed. The existing TestContextBeginWithTxOptions even depended on that buggy behavior (Isolation matched, ReadOnly didn't, expectation still considered fulfilled). Switched the conjunction to a disjunction and fixed the existing test to use matching options. Added two new mismatch tests (one for Isolation-only differs, one for ReadOnly-only differs) that fail on master and pass with the fix. Signed-off-by: Charlie Tonneslan --- sqlmock.go | 4 ++-- sqlmock_go18_test.go | 48 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/sqlmock.go b/sqlmock.go index a4f8f35..6380491 100644 --- a/sqlmock.go +++ b/sqlmock.go @@ -254,8 +254,8 @@ func (c *sqlmock) begin(opts driver.TxOptions) (*ExpectedBegin, error) { } defer expected.Unlock() if expected.txOpts != nil && - expected.txOpts.Isolation != opts.Isolation && - expected.txOpts.ReadOnly != opts.ReadOnly { + (expected.txOpts.Isolation != opts.Isolation || + expected.txOpts.ReadOnly != opts.ReadOnly) { return nil, fmt.Errorf("expected transaction options do not match: %+v, got: %+v", expected.txOpts, opts) } diff --git a/sqlmock_go18_test.go b/sqlmock_go18_test.go index ddc7306..d95a8b9 100644 --- a/sqlmock_go18_test.go +++ b/sqlmock_go18_test.go @@ -380,7 +380,7 @@ func TestContextBeginWithTxOptions(t *testing.T) { cancel() }() - _, err = db.BeginTx(ctx, &sql.TxOptions{Isolation: sql.LevelReadCommitted, ReadOnly: false}) + _, err = db.BeginTx(ctx, &sql.TxOptions{Isolation: sql.LevelReadCommitted, ReadOnly: true}) if err != nil { t.Errorf("error was not expected, but got: %v", err) } @@ -420,6 +420,52 @@ func TestContextBeginWithTxOptionsMismatch(t *testing.T) { } } +func TestContextBeginWithTxOptionsReadOnlyMismatch(t *testing.T) { + t.Parallel() + db, mock, err := New() + if err != nil { + t.Errorf("an error '%s' was not expected when opening a stub database connection", err) + } + defer db.Close() + + mock.ExpectBegin().WithTxOptions(sql.TxOptions{ + Isolation: sql.LevelReadCommitted, + ReadOnly: true, + }) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Isolation matches, but ReadOnly does not. Should still error. + _, err = db.BeginTx(ctx, &sql.TxOptions{Isolation: sql.LevelReadCommitted, ReadOnly: false}) + if err == nil { + t.Error("error was expected on ReadOnly mismatch, but there was none") + } +} + +func TestContextBeginWithTxOptionsIsolationMismatch(t *testing.T) { + t.Parallel() + db, mock, err := New() + if err != nil { + t.Errorf("an error '%s' was not expected when opening a stub database connection", err) + } + defer db.Close() + + mock.ExpectBegin().WithTxOptions(sql.TxOptions{ + Isolation: sql.LevelReadCommitted, + ReadOnly: true, + }) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // ReadOnly matches, but Isolation does not. Should still error. + _, err = db.BeginTx(ctx, &sql.TxOptions{Isolation: sql.LevelSerializable, ReadOnly: true}) + if err == nil { + t.Error("error was expected on Isolation mismatch, but there was none") + } +} + func TestContextPrepareCancel(t *testing.T) { t.Parallel() db, mock, err := New()