Skip to content

Commit bb21d4b

Browse files
authored
[test] Add tests for mcp.Connection.callTool (#10937)
# Test Coverage Improvement: `callTool` ## Function Analyzed - **Package**: `internal/mcp` - **Function**: `Connection.callTool` (in `connection_methods.go`) - **Previous Coverage**: 20.0% - **New Coverage**: 100.0% - **Complexity**: Medium (nil-defaulting branch, SDK dispatch, error propagation) ## Why This Function? `callTool` is the gateway's core `tools/call` dispatch path — it normalizes nil `arguments` to an empty map (a hard MCP protocol requirement) before invoking the backend SDK session's `CallTool`. It had only 20% statement coverage: existing tests exercised argument marshaling/unmarshaling at the HTTP JSON layer and nil-session error paths, but never exercised the real SDK call path (the `if p.Arguments == nil` branch and the successful `getSDKSession().CallTool(...)` call were both untested). ## Tests Added - ✅ `TestCallTool_NilArgumentsDefaultsToEmptyMap` — verifies omitted `arguments` becomes an empty map at the backend - ✅ `TestCallTool_ArgumentsForwarded` — verifies provided arguments are forwarded unmodified - ✅ `TestCallTool_BackendHandlerError` — verifies a Go error from the backend tool handler surfaces as an error - ✅ `TestCallTool_UnknownToolName` — verifies an unknown tool name produces an error All four tests spin up a **real** `sdk.NewStreamableHTTPHandler`-backed `httptest.Server` (rather than mocking JSON-RPC at the HTTP layer), so they exercise the actual `callSDKMethod → callParamMethod → sdk.ClientSession.CallTool` dispatch path end-to-end. ## Coverage Report ``` Before: internal/mcp package 97.6%, callTool 20.0% After: internal/mcp package 98.4%, callTool 100.0% Improvement: +0.8% package, +80% for the target function ``` ## Test Execution ``` === RUN TestCallTool_NilArgumentsDefaultsToEmptyMap --- PASS: TestCallTool_NilArgumentsDefaultsToEmptyMap (0.00s) === RUN TestCallTool_ArgumentsForwarded --- PASS: TestCallTool_ArgumentsForwarded (0.00s) === RUN TestCallTool_BackendHandlerError --- PASS: TestCallTool_BackendHandlerError (0.00s) === RUN TestCallTool_UnknownToolName --- PASS: TestCallTool_UnknownToolName (0.00s) PASS ok github.com/github/gh-aw-mcpg/internal/mcp 0.028s ``` `gofmt` and `go vet` are clean on the new file. --- *Generated by Test Coverage Improver* *Next run will target the next most complex under-tested function* > Generated by [Test Coverage Improver](https://github.com/github/gh-aw-mcpg/actions/runs/31324128354) · auto · 131.6 AIC · ⊞ 10.1K · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw-mcpg+%22gh-aw-workflow-id%3A+test-coverage-improver%22&type=pullrequests) <!-- gh-aw-agentic-workflow: Test Coverage Improver, engine: copilot, model: auto, id: 31324128354, workflow_id: test-coverage-improver, run: https://github.com/github/gh-aw-mcpg/actions/runs/31324128354 --> <!-- gh-aw-workflow-id: test-coverage-improver --> <!-- gh-aw-workflow-call-id: github/gh-aw-mcpg/test-coverage-improver -->
2 parents c033de6 + 7a853e6 commit bb21d4b

1 file changed

Lines changed: 153 additions & 0 deletions

File tree

internal/mcp/call_tool_test.go

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
package mcp
2+
3+
import (
4+
"context"
5+
"net/http"
6+
"net/http/httptest"
7+
"sync"
8+
"testing"
9+
10+
sdk "github.com/modelcontextprotocol/go-sdk/mcp"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
)
14+
15+
// newCallToolTestBackend spins up a real SDK streamable HTTP MCP server with a
16+
// single tool ("echo_tool") that records the arguments it receives via
17+
// req.Params.Arguments (the SDK-native path, not the gateway's own JSON
18+
// parsing). This lets tests exercise Connection.callTool's real SDK dispatch
19+
// (via callSDKMethod → callParamMethod → sdk.ClientSession.CallTool) end to
20+
// end, rather than mocking at the HTTP JSON-RPC layer.
21+
func newCallToolTestBackend(t *testing.T) (*httptest.Server, *sync.Map) {
22+
t.Helper()
23+
received := &sync.Map{} // toolName -> map[string]any arguments
24+
25+
impl := &sdk.Implementation{Name: "call-tool-test-backend", Version: "1.0.0"}
26+
mcpServer := sdk.NewServer(impl, nil)
27+
mcpServer.AddTool(&sdk.Tool{
28+
Name: "echo_tool",
29+
Description: "Echoes back the arguments it receives",
30+
InputSchema: map[string]interface{}{"type": "object"},
31+
}, func(_ context.Context, req *sdk.CallToolRequest) (*sdk.CallToolResult, error) {
32+
args, err := ParseToolArguments(req)
33+
if err != nil {
34+
return &sdk.CallToolResult{
35+
IsError: true,
36+
Content: []sdk.Content{&sdk.TextContent{Text: err.Error()}},
37+
}, nil
38+
}
39+
received.Store("echo_tool", args)
40+
return &sdk.CallToolResult{
41+
Content: []sdk.Content{&sdk.TextContent{Text: "ok"}},
42+
}, nil
43+
})
44+
mcpServer.AddTool(&sdk.Tool{
45+
Name: "failing_tool",
46+
Description: "Always returns a Go error from the handler",
47+
InputSchema: map[string]interface{}{"type": "object"},
48+
}, func(_ context.Context, _ *sdk.CallToolRequest) (*sdk.CallToolResult, error) {
49+
return nil, assert.AnError
50+
})
51+
52+
handler := sdk.NewStreamableHTTPHandler(func(_ *http.Request) *sdk.Server {
53+
return mcpServer
54+
}, &sdk.StreamableHTTPOptions{Stateless: false})
55+
56+
mux := http.NewServeMux()
57+
mux.Handle("/mcp", handler)
58+
mux.Handle("/mcp/", handler)
59+
60+
return httptest.NewServer(mux), received
61+
}
62+
63+
// TestCallTool_NilArgumentsDefaultsToEmptyMap verifies that callTool (invoked
64+
// via the real SDK dispatch path) defaults a nil Arguments map to an empty
65+
// map before forwarding the call to the backend, per the MCP protocol
66+
// requirement that "arguments" always be present.
67+
func TestCallTool_NilArgumentsDefaultsToEmptyMap(t *testing.T) {
68+
srv, received := newCallToolTestBackend(t)
69+
defer srv.Close()
70+
71+
conn, err := NewHTTPConnection(context.Background(), "test-server", srv.URL+"/mcp", nil, nil, "", 0, 0)
72+
require.NoError(t, err)
73+
defer conn.Close()
74+
75+
// Explicitly omit "arguments" so p.Arguments unmarshals to nil.
76+
params := map[string]interface{}{"name": "echo_tool"}
77+
resp, err := conn.SendRequestWithServerID(context.Background(), "tools/call", params, "test-server")
78+
require.NoError(t, err)
79+
require.NotNil(t, resp)
80+
require.Nil(t, resp.Error)
81+
82+
val, ok := received.Load("echo_tool")
83+
require.True(t, ok, "backend should have recorded the call")
84+
args, ok := val.(map[string]any)
85+
require.True(t, ok)
86+
assert.Empty(t, args, "nil arguments should be normalized to an empty map")
87+
}
88+
89+
// TestCallTool_ArgumentsForwarded verifies that when arguments are provided,
90+
// callTool forwards them unmodified to the backend through the real SDK
91+
// dispatch path.
92+
func TestCallTool_ArgumentsForwarded(t *testing.T) {
93+
srv, received := newCallToolTestBackend(t)
94+
defer srv.Close()
95+
96+
conn, err := NewHTTPConnection(context.Background(), "test-server", srv.URL+"/mcp", nil, nil, "", 0, 0)
97+
require.NoError(t, err)
98+
defer conn.Close()
99+
100+
params := map[string]interface{}{
101+
"name": "echo_tool",
102+
"arguments": map[string]interface{}{
103+
"query": "hello",
104+
"count": float64(3),
105+
},
106+
}
107+
resp, err := conn.SendRequestWithServerID(context.Background(), "tools/call", params, "test-server")
108+
require.NoError(t, err)
109+
require.NotNil(t, resp)
110+
require.Nil(t, resp.Error)
111+
112+
val, ok := received.Load("echo_tool")
113+
require.True(t, ok)
114+
args, ok := val.(map[string]any)
115+
require.True(t, ok)
116+
assert.Equal(t, "hello", args["query"])
117+
assert.Equal(t, float64(3), args["count"])
118+
}
119+
120+
// TestCallTool_BackendHandlerError verifies that when the backend tool
121+
// handler returns a Go error (not a CallToolResult with IsError), callTool
122+
// surfaces that as an error through the real SDK dispatch path.
123+
func TestCallTool_BackendHandlerError(t *testing.T) {
124+
srv, _ := newCallToolTestBackend(t)
125+
defer srv.Close()
126+
127+
conn, err := NewHTTPConnection(context.Background(), "test-server", srv.URL+"/mcp", nil, nil, "", 0, 0)
128+
require.NoError(t, err)
129+
defer conn.Close()
130+
131+
params := map[string]interface{}{"name": "failing_tool"}
132+
resp, err := conn.SendRequestWithServerID(context.Background(), "tools/call", params, "test-server")
133+
require.Error(t, err)
134+
assert.Nil(t, resp)
135+
}
136+
137+
// TestCallTool_UnknownToolName verifies that calling a tool name unknown to
138+
// the backend surfaces an error via the real SDK dispatch path (exercises the
139+
// non-nil-session branch differently from the "unsupported method" cases
140+
// covered elsewhere).
141+
func TestCallTool_UnknownToolName(t *testing.T) {
142+
srv, _ := newCallToolTestBackend(t)
143+
defer srv.Close()
144+
145+
conn, err := NewHTTPConnection(context.Background(), "test-server", srv.URL+"/mcp", nil, nil, "", 0, 0)
146+
require.NoError(t, err)
147+
defer conn.Close()
148+
149+
params := map[string]interface{}{"name": "does_not_exist"}
150+
resp, err := conn.SendRequestWithServerID(context.Background(), "tools/call", params, "test-server")
151+
require.Error(t, err)
152+
assert.Nil(t, resp)
153+
}

0 commit comments

Comments
 (0)