test(alloydbpg,cloudsqlpg): achieve full parity with legacy tests for custom tools#3014
test(alloydbpg,cloudsqlpg): achieve full parity with legacy tests for custom tools#3014anubhav756 wants to merge 4 commits intoanubhav-mcp-cloudsqlpgfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request expands the integration test suite for AlloyDB and CloudSQL Postgres by adding comprehensive test cases for custom tools, execute-sql functionality, and template parameters. The changes introduce new helper functions in tests/mcp_tool.go to facilitate these tests. I have provided feedback regarding the hardcoding of table names in the new helper function and suggested a cleaner way to define SQL statements to avoid unnecessary string manipulation.
| } | ||
|
|
||
| // RunMCPExecuteSqlToolInvokeTest runs execute-sql tool invoke test cases via MCP. | ||
| func RunMCPExecuteSqlToolInvokeTest(t *testing.T, ctx context.Context, createTableStatement, select1Want string, options ...ExecuteSqlOption) { |
There was a problem hiding this comment.
The RunMCPExecuteSqlToolInvokeTest function hardcodes the table name t in several test cases (lines 456, 462, 500). This makes the helper less flexible and can cause race conditions or conflicts if multiple tests run in parallel against the same database. Consider adding a tableName parameter to the function and using it to construct the SQL queries, similar to how RunMCPToolInvokeWithTemplateParameters is implemented on line 549.
| // RunMCPExecuteSqlToolInvokeTest runs execute-sql tool invoke test cases via MCP. | ||
| func RunMCPExecuteSqlToolInvokeTest(t *testing.T, ctx context.Context, createTableStatement, select1Want string, options ...ExecuteSqlOption) { | ||
| configs := &ExecuteSqlTestConfig{ | ||
| select1Statement: `"SELECT 1"`, |
There was a problem hiding this comment.
The select1Statement is defined with escaped double quotes, which are then trimmed in every test case (e.g., line 444). This adds unnecessary complexity. It would be cleaner to define the SQL statement without quotes and remove the strings.Trim calls.
| select1Statement: `"SELECT 1"`, | |
| select1Statement: "SELECT 1", |
anubhav756
force-pushed
the
anubhav-parity
branch
from
4b23dcb to
269939b
Compare
anubhav756
force-pushed
the
anubhav-mcp-cloudsqlpg
branch
from
cf13215 to
63b7b9e
Compare
anubhav756
force-pushed
the
anubhav-parity
branch
from
269939b to
68680a6
Compare
anubhav756
force-pushed
the
anubhav-mcp-cloudsqlpg
branch
from
63b7b9e to
85a2173
Compare
anubhav756
force-pushed
the
anubhav-parity
branch
from
68680a6 to
27d2f73
Compare
anubhav756
force-pushed
the
anubhav-parity
branch
from
9059e86 to
2e66927
Compare
1 participant
Description
This PR completes the migration of AlloyDB and Cloud SQL Postgres integration tests to the MCP framework by achieving full feature parity with the legacy tests, specifically for custom tool configurations.
hanges
alloydbpgandcloudsqlpgMCP tests to set up necessary tables and register custom tools (param tools, auth tools, semantic search) via dynamic configuration files, mirroring the legacy test setup.RunMCPExecuteSqlToolInvokeTestandRunMCPToolInvokeWithTemplateParametersintests/mcp_tool.goto allow testing advanced custom tools over the JSON-RPC/mcpendpoint.