Skip to content

Commit 7041e79

Browse files
NirajNandreNirajNandre
authored
fix(tests/postgres): restore list_schemas test and implement dynamic owner (#2521)
## Description This PR resolves the race conditions identified in Issue 2463 by fully implementing per-test resource isolation and dynamic metadata validation for` PostgreSQL` databases. ### Key Changes: **UUID-Based Isolation:** Prefixes schema names with a `uniqueID` to ensure that concurrent test runs in Cloud Build do not interfere with or accidentally delete each other's resources. **Dynamic Owner**: Replaced the hardcoded `postgres` owner with dynamic environment variables (e.g., PostgresUser, etc), ensuring tests pass in environments with non-default database users. **Restored Integration Test:** Un-commented the `invoke_list_schemas_with_owner_name` test case to verify the tool's filtering functionality and confirm the effectiveness of the isolation logic. **Error Log Correction**: Fixed a string formatting bug that produced `%!s(float64=0)` by updating the failure reporting verb to `%+v ` for structured map data. ## PR Checklist > Thank you for opening a Pull Request! Before submitting your PR, there are a > few things you can do to make sure it goes smoothly: - [ ] Make sure you reviewed [CONTRIBUTING.md](https://github.com/googleapis/genai-toolbox/blob/main/CONTRIBUTING.md) - [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/genai-toolbox/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [ ] Ensure the tests and linter pass - [ ] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) - [ ] Make sure to add `!` if this involve a breaking change 🛠️ Fixes #2463
1 parent 61d0ea4 commit 7041e79

5 files changed

Lines changed: 20 additions & 15 deletions

File tree

‎tests/alloydbomni/alloydb_omni_integration_test.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@ import (
2020
"net/url"
2121
"os"
2222
"regexp"
23+
"strings"
2324
"testing"
2425
"time"
2526

27+
"github.com/google/uuid"
2628
"github.com/googleapis/genai-toolbox/internal/testutils"
2729
"github.com/googleapis/genai-toolbox/tests"
2830
"github.com/jackc/pgx/v5/pgxpool"
@@ -113,6 +115,9 @@ func TestAlloyDBOmni(t *testing.T) {
113115
os.Setenv("ALLOYDB_OMNI_PASSWORD", AlloyDBPass)
114116
os.Setenv("ALLOYDB_OMNI_DATABASE", AlloyDBDatabase)
115117

118+
// Generate a unique ID
119+
uniqueID := strings.ReplaceAll(uuid.New().String(), "-", "")
120+
116121
args := []string{"--prebuilt", "alloydb-omni"}
117122

118123
pool, err := initPostgresConnectionPool(AlloyDBHost, AlloyDBPort, AlloyDBUser, AlloyDBPass, AlloyDBDatabase)
@@ -138,7 +143,7 @@ func TestAlloyDBOmni(t *testing.T) {
138143

139144
// Run Postgres prebuilt tool tests
140145
tests.RunPostgresListViewsTest(t, ctx, pool)
141-
tests.RunPostgresListSchemasTest(t, ctx, pool)
146+
tests.RunPostgresListSchemasTest(t, ctx, pool, AlloyDBUser, uniqueID)
142147
tests.RunPostgresListActiveQueriesTest(t, ctx, pool)
143148
tests.RunPostgresListAvailableExtensionsTest(t)
144149
tests.RunPostgresListInstalledExtensionsTest(t)

‎tests/alloydbpg/alloydb_pg_integration_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ func TestAlloyDBPgToolEndpoints(t *testing.T) {
195195
// Run Postgres prebuilt tool tests
196196
tests.RunPostgresListTablesTest(t, tableNameParam, tableNameAuth, AlloyDBPostgresUser)
197197
tests.RunPostgresListViewsTest(t, ctx, pool)
198-
tests.RunPostgresListSchemasTest(t, ctx, pool)
198+
tests.RunPostgresListSchemasTest(t, ctx, pool, AlloyDBPostgresUser, uniqueID)
199199
tests.RunPostgresListActiveQueriesTest(t, ctx, pool)
200200
tests.RunPostgresListAvailableExtensionsTest(t)
201201
tests.RunPostgresListInstalledExtensionsTest(t)

‎tests/cloudsqlpg/cloud_sql_pg_integration_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ func TestCloudSQLPgSimpleToolEndpoints(t *testing.T) {
179179
// Run Postgres prebuilt tool tests
180180
tests.RunPostgresListTablesTest(t, tableNameParam, tableNameAuth, CloudSQLPostgresUser)
181181
tests.RunPostgresListViewsTest(t, ctx, pool)
182-
tests.RunPostgresListSchemasTest(t, ctx, pool)
182+
tests.RunPostgresListSchemasTest(t, ctx, pool, CloudSQLPostgresUser, uniqueID)
183183
tests.RunPostgresListActiveQueriesTest(t, ctx, pool)
184184
tests.RunPostgresListAvailableExtensionsTest(t)
185185
tests.RunPostgresListInstalledExtensionsTest(t)

‎tests/postgres/postgres_integration_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ func TestPostgres(t *testing.T) {
158158
// Run Postgres prebuilt tool tests
159159
tests.RunPostgresListTablesTest(t, tableNameParam, tableNameAuth, PostgresUser)
160160
tests.RunPostgresListViewsTest(t, ctx, pool)
161-
tests.RunPostgresListSchemasTest(t, ctx, pool)
161+
tests.RunPostgresListSchemasTest(t, ctx, pool, PostgresUser, uniqueID)
162162
tests.RunPostgresListActiveQueriesTest(t, ctx, pool)
163163
tests.RunPostgresListAvailableExtensionsTest(t)
164164
tests.RunPostgresListInstalledExtensionsTest(t)

‎tests/tool.go‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1357,12 +1357,12 @@ func RunPostgresListViewsTest(t *testing.T, ctx context.Context, pool *pgxpool.P
13571357
}
13581358
}
13591359

1360-
func RunPostgresListSchemasTest(t *testing.T, ctx context.Context, pool *pgxpool.Pool) {
1361-
schemaName := "test_schema_" + strings.ReplaceAll(uuid.New().String(), "-", "")
1360+
func RunPostgresListSchemasTest(t *testing.T, ctx context.Context, pool *pgxpool.Pool, owner string, uniqueID string) {
1361+
schemaName := "test_schema_" + uniqueID
13621362
cleanup := setupPostgresSchemas(t, ctx, pool, schemaName)
13631363
defer cleanup()
13641364

1365-
wantSchema := map[string]any{"functions": float64(0), "grants": map[string]any{}, "owner": "postgres", "schema_name": schemaName, "tables": float64(0), "views": float64(0)}
1365+
wantSchema := map[string]any{"functions": float64(0), "grants": map[string]any{}, "owner": owner, "schema_name": schemaName, "tables": float64(0), "views": float64(0)}
13661366

13671367
invokeTcs := []struct {
13681368
name string
@@ -1377,13 +1377,13 @@ func RunPostgresListSchemasTest(t *testing.T, ctx context.Context, pool *pgxpool
13771377
wantStatusCode: http.StatusOK,
13781378
want: []map[string]any{wantSchema},
13791379
},
1380-
// {
1381-
// name: "invoke list_schemas with owner name",
1382-
// requestBody: bytes.NewBuffer([]byte(fmt.Sprintf(`{"owner": "%s"}`, "postgres"))),
1383-
// wantStatusCode: http.StatusOK,
1384-
// want: []map[string]any{wantSchema},
1385-
// compareSubset: true,
1386-
// },
1380+
{
1381+
name: "invoke list_schemas with owner name",
1382+
requestBody: bytes.NewBuffer([]byte(fmt.Sprintf(`{"owner": "%s"}`, owner))),
1383+
wantStatusCode: http.StatusOK,
1384+
want: []map[string]any{wantSchema},
1385+
compareSubset: true,
1386+
},
13871387
{
13881388
name: "invoke list_schemas with limit 1",
13891389
requestBody: bytes.NewBuffer([]byte(fmt.Sprintf(`{"schema_name": "%s","limit": 1}`, schemaName))),
@@ -1438,7 +1438,7 @@ func RunPostgresListSchemasTest(t *testing.T, ctx context.Context, pool *pgxpool
14381438
}
14391439
}
14401440
if !found {
1441-
t.Errorf("Expected schema '%s' not found in the list of all schemas.", wantSchema)
1441+
t.Errorf("Expected schema '%+v' not found in the list of all schemas.", wantSchema)
14421442
}
14431443
} else {
14441444
if diff := cmp.Diff(tc.want, got); diff != "" {

0 commit comments

Comments
 (0)