From f4b766b433a401499e7bfd7bff8ca1a723205d40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Thu, 11 Jun 2026 14:16:52 +0200 Subject: [PATCH 1/2] test(stmt): add tests to verify query parameter mismatch behaviour --- stmt_test.go | 38 ++++++++++++++++++++++++++++++++++++ stmt_with_mockserver_test.go | 17 ++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/stmt_test.go b/stmt_test.go index d725c737..c92d4c07 100644 --- a/stmt_test.go +++ b/stmt_test.go @@ -17,9 +17,47 @@ package spannerdriver import ( "database/sql/driver" "reflect" + "strings" "testing" + + "cloud.google.com/go/spanner/admin/database/apiv1/databasepb" + "github.com/googleapis/go-sql-spanner/connectionstate" + "github.com/googleapis/go-sql-spanner/parser" ) +func TestPrepareSpannerStmt(t *testing.T) { + state := createInitialConnectionState(connectionstate.TypeNonTransactional, nil) + p, err := parser.NewStatementParser(databasepb.DatabaseDialect_GOOGLE_STANDARD_SQL, 1000) + if err != nil { + t.Fatal(err) + } + + // Case 1: Query parameter name matches exactly. + { + stmt, err := prepareSpannerStmt(state, p, "SELECT * FROM Singers WHERE SingerId = @id", []driver.NamedValue{ + {Name: "id", Value: int64(1)}, + }) + if err != nil { + t.Errorf("Unexpected error for matching parameter name: %v", err) + } + if got, want := stmt.Params["id"], int64(1); got != want { + t.Errorf("Params[\"id\"] = %v, want %v", got, want) + } + } + + // Case 2: Query parameter name mismatch. + { + _, err := prepareSpannerStmt(state, p, "SELECT * FROM Singers WHERE SingerId = @singer_id", []driver.NamedValue{ + {Name: "id", Value: int64(1)}, + }) + if err == nil { + t.Error("Expected error for mismatched parameter name, got nil") + } else if !strings.Contains(err.Error(), "missing value for query parameter @singer_id") { + t.Errorf("Expected 'missing value for query parameter @singer_id' error, got %v", err) + } + } +} + func TestConvertParam(t *testing.T) { check := func(in, want driver.Value) { t.Helper() diff --git a/stmt_with_mockserver_test.go b/stmt_with_mockserver_test.go index d9ade319..4e92487c 100644 --- a/stmt_with_mockserver_test.go +++ b/stmt_with_mockserver_test.go @@ -943,3 +943,20 @@ func executeParamTest(t *testing.T, test paramTest, server *testutil.MockedSpann } } } + +func TestQueryMismatchedParameterName(t *testing.T) { + t.Parallel() + + db, _, teardown := setupTestDBConnection(t) + defer teardown() + ctx := context.Background() + + // SELECT with @my_id parameter but passing named parameter "id" instead + _, err := db.QueryContext(ctx, "select value from test where id=@my_id", sql.Named("id", 123)) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "missing value for query parameter @my_id") { + t.Fatalf("expected missing value error, got: %v", err) + } +} From 9ec7947c10dfa23bac6918da002b3d1ba09548af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Fri, 12 Jun 2026 13:36:07 +0200 Subject: [PATCH 2/2] chore: address review comments --- GEMINI.md | 1 + stmt_test.go | 19 +++++++++---------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/GEMINI.md b/GEMINI.md index 5784027e..25fb353c 100644 --- a/GEMINI.md +++ b/GEMINI.md @@ -60,3 +60,4 @@ Any pull request modifying or extending the driver's features must include: - **Mock Server Tests**: Located in files such as `driver_with_mockserver_test.go`, `conn_with_mockserver_test.go`, and `stmt_with_mockserver_test.go`. Use these (or add new test files) to mock Spanner gRPC API responses (e.g. BeginTransaction, Commit, ExecuteSql) and verify that the driver translates options, tags, and states correctly. - **Emulator Tests**: Validate integration behavior against the Cloud Spanner Emulator (`integration_test.go` and examples). Make sure the test configurations can run locally with `auto_config_emulator=true`. - **Wrapper Tests**: If you modified `spannerlib`, ensure you trigger or run unit/integration tests for the respective wrappers (`python-spanner-lib-wrapper-unit-tests.yml`, `ruby-wrapper-tests.yml`, etc.). +- **Subtests & Early Failure**: Prefer using Go subtests (`t.Run`) to isolate distinct test cases. When an intermediate setup step fails (e.g., helper function error, unexpected nil, client initialization failure), use `t.Fatalf` or `t.Fatal` to fail and abort that specific test/subtest immediately. Avoid using `t.Errorf` or `t.Error` if subsequent assertions rely on values or state from the failed step, as this results in redundant and confusing secondary error outputs. diff --git a/stmt_test.go b/stmt_test.go index c92d4c07..17404e02 100644 --- a/stmt_test.go +++ b/stmt_test.go @@ -32,30 +32,29 @@ func TestPrepareSpannerStmt(t *testing.T) { t.Fatal(err) } - // Case 1: Query parameter name matches exactly. - { + t.Run("QueryParameterNameMatchesExactly", func(t *testing.T) { stmt, err := prepareSpannerStmt(state, p, "SELECT * FROM Singers WHERE SingerId = @id", []driver.NamedValue{ {Name: "id", Value: int64(1)}, }) if err != nil { - t.Errorf("Unexpected error for matching parameter name: %v", err) + t.Fatalf("Unexpected error for matching parameter name: %v", err) } if got, want := stmt.Params["id"], int64(1); got != want { t.Errorf("Params[\"id\"] = %v, want %v", got, want) } - } + }) - // Case 2: Query parameter name mismatch. - { + t.Run("QueryParameterNameMismatch", func(t *testing.T) { _, err := prepareSpannerStmt(state, p, "SELECT * FROM Singers WHERE SingerId = @singer_id", []driver.NamedValue{ {Name: "id", Value: int64(1)}, }) if err == nil { - t.Error("Expected error for mismatched parameter name, got nil") - } else if !strings.Contains(err.Error(), "missing value for query parameter @singer_id") { - t.Errorf("Expected 'missing value for query parameter @singer_id' error, got %v", err) + t.Fatal("Expected error for mismatched parameter name, got nil") } - } + if !strings.Contains(err.Error(), "missing value for query parameter @singer_id") { + t.Fatalf("Expected 'missing value for query parameter @singer_id' error, got %v", err) + } + }) } func TestConvertParam(t *testing.T) {