test(source/alloydbomni): create MCP integration tests#2964
Open
anubhav756 wants to merge 6 commits intoanubhav-parityfrom
Open
test(source/alloydbomni): create MCP integration tests#2964anubhav756 wants to merge 6 commits intoanubhav-parityfrom
anubhav756 wants to merge 6 commits intoanubhav-parityfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new integration test suite for AlloyDB Omni and a comprehensive set of reusable test helpers for MCP database tools, covering views, schemas, active queries, triggers, and more. The feedback focuses on improving test efficiency and maintainability by suggesting a reduction in test timeouts, the use of proper context propagation instead of context.Background(), and the refactoring of hardcoded SQL filter strings into constants.
tests/mcp_tool.go
Outdated
| // InitPostgresConnectionPool initializes a connection pool for Postgres. | ||
| func InitPostgresConnectionPool(host, port, user, pass, dbname string) (*pgxpool.Pool, error) { | ||
| url := buildPostgresURL(host, port, user, pass, dbname) | ||
| pool, err := pgxpool.New(context.Background(), url.String()) |
Contributor
There was a problem hiding this comment.
Using context.Background() in tests is generally discouraged. Consider passing the test context or a derived context to ensure proper cancellation and resource cleanup.
|
|
||
| // cleanupOldSchemas cleans up schemas that were created more than 1 hour ago | ||
| func cleanupOldSchemas(t *testing.T, ctx context.Context, pool *pgxpool.Pool) { | ||
| rows, err := pool.Query(ctx, "SELECT schema_name FROM information_schema.schemata WHERE schema_name LIKE 'test_proc_%'") |
Contributor
There was a problem hiding this comment.
The query uses a hardcoded string literal 'test_proc_%' for filtering. Consider using a constant or a more robust way to identify test schemas to avoid potential collisions or maintenance issues.
anubhav756
force-pushed
the
anubhav-mcp-alloydbomni
branch
3 times, most recently
from
01d4bfb to
2d7e90b
Compare
anubhav756
force-pushed
the
anubhav-mcp-alloydbomni
branch
2 times, most recently
from
1c411a8 to
77eb420
Compare
anubhav756
force-pushed
the
anubhav-mcp-alloydbainl
branch
from
4fe514a to
075e596
Compare
anubhav756
force-pushed
the
anubhav-mcp-alloydbomni
branch
from
77eb420 to
97ce3c1
Compare
anubhav756
force-pushed
the
anubhav-mcp-alloydbainl
branch
from
075e596 to
af9312a
Compare
anubhav756
force-pushed
the
anubhav-mcp-alloydbomni
branch
from
97ce3c1 to
bcecb75
Compare
anubhav756
force-pushed
the
anubhav-mcp-alloydbainl
branch
from
af9312a to
79969dc
Compare
anubhav756
force-pushed
the
anubhav-mcp-alloydbomni
branch
2 times, most recently
from
131d237 to
fef27e4
Compare
anubhav756
force-pushed
the
anubhav-mcp-alloydbainl
branch
from
79969dc to
e284d4f
Compare
anubhav756
force-pushed
the
anubhav-mcp-alloydbomni
branch
2 times, most recently
from
6d21e9c to
41b03ac
Compare
anubhav756
force-pushed
the
anubhav-mcp-alloydbomni
branch
from
816552c to
62dfc08
Compare
anubhav756
force-pushed
the
anubhav-mcp-cloudsqlpg
branch
from
2be51dc to
5561185
Compare
anubhav756
force-pushed
the
anubhav-mcp-alloydbomni
branch
from
62dfc08 to
ddb001a
Compare
anubhav756
force-pushed
the
anubhav-mcp-cloudsqlpg
branch
from
5561185 to
480b04b
Compare
anubhav756
force-pushed
the
anubhav-mcp-alloydbomni
branch
from
ddb001a to
6839d7d
Compare
anubhav756
force-pushed
the
anubhav-mcp-cloudsqlpg
branch
from
480b04b to
6cc7509
Compare
anubhav756
force-pushed
the
anubhav-mcp-alloydbomni
branch
from
6839d7d to
483ed1d
Compare
anubhav756
force-pushed
the
anubhav-mcp-cloudsqlpg
branch
from
e120bc5 to
e74fd80
Compare
anubhav756
force-pushed
the
anubhav-mcp-alloydbomni
branch
from
483ed1d to
8be4808
Compare
anubhav756
force-pushed
the
anubhav-mcp-cloudsqlpg
branch
from
e74fd80 to
3a4c79b
Compare
anubhav756
force-pushed
the
anubhav-mcp-alloydbomni
branch
2 times, most recently
from
1affb35 to
5cf000b
Compare
anubhav756
force-pushed
the
anubhav-mcp-cloudsqlpg
branch
2 times, most recently
from
29a4b31 to
64f7997
Compare
anubhav756
force-pushed
the
anubhav-mcp-alloydbomni
branch
from
5cf000b to
940ed19
Compare
anubhav756
force-pushed
the
anubhav-mcp-cloudsqlpg
branch
9 times, most recently
from
aad6cd9 to
9ecd5c2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds the mapped integration tests for AlloyDB Omni tools using the native MCP harness.