Skip to content

test(source/alloydbainl): create MCP integration tests#2921

Open
anubhav756 wants to merge 4 commits intoanubhav-feat-native-mcp-alloydbfrom
anubhav-mcp-alloydbainl
Open

test(source/alloydbainl): create MCP integration tests#2921
anubhav756 wants to merge 4 commits intoanubhav-feat-native-mcp-alloydbfrom
anubhav-mcp-alloydbainl

Conversation

@anubhav756
Copy link
Copy Markdown
Contributor

This PR adds the mapped integration tests for AlloyDB AINL tools using the native MCP harness.

@anubhav756 anubhav756 self-assigned this Apr 1, 2026
@anubhav756 anubhav756 requested review from a team as code owners April 1, 2026 16:50
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the AlloyDB AI NL integration tests by moving shared configuration and environment variable logic into a new file, alloydb_ai_nl_mcp_test.go, which focuses on testing tool functionality via the Model Context Protocol (MCP). The new tests cover tool listing and various invocation scenarios, including authenticated and unauthenticated requests. Feedback includes a recommendation to add safety checks for slice indexing to prevent runtime panics and a requirement to update tool names to snake_case to comply with the project's style guide.

if mcpResp.Result.IsError {
t.Fatalf("expected success result, got error: %v", mcpResp.Result)
}
got := mcpResp.Result.Content[0].Text
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Accessing mcpResp.Result.Content[0] without checking the length of the Content slice could lead to a runtime panic if the tool returns an empty result. It is safer to verify that at least one content item is present before accessing it.

Suggested change
got := mcpResp.Result.Content[0].Text
if len(mcpResp.Result.Content) == 0 {
t.Fatalf("expected at least one content item, got none")
}
got := mcpResp.Result.Content[0].Text

},
},
"tools": map[string]any{
"my-simple-tool": map[string]any{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The tool name my-simple-tool uses kebab-case, which violates the repository style guide. According to the guide, tool names should use snake_case (e.g., my_simple_tool). This also applies to my-auth-tool and my-auth-required-tool defined in this configuration, as well as their usage in the test cases and other test files in this package. Additionally, since these are MCP configuration values, please ensure the code includes a comment citing and linking to the MCP semantic conventions to avoid confusion, as per repository rules.

References
  1. Tool names must use snake_case and should not include the product name. (link)
  2. When using configuration values that adhere to a specific standard like MCP, include a comment citing and linking to that standard to avoid confusion.

@anubhav756 anubhav756 force-pushed the anubhav-feat-native-mcp-alloydb branch from 69bb59b to d81edf8 Compare April 1, 2026 17:03
@anubhav756 anubhav756 force-pushed the anubhav-mcp-alloydbainl branch from fa85277 to 023e098 Compare April 2, 2026 08:42
@anubhav756 anubhav756 force-pushed the anubhav-feat-native-mcp-alloydb branch from d81edf8 to 4c21e3f Compare April 2, 2026 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant