Skip to content

Commit b0670d0

Browse files
authored
[test-improver] Improve tests for mcp/tool_result package (#4011)
## File Analyzed - **Test File**: `internal/mcp/tool_result_test.go` - **Package**: `internal/mcp` - **Lines Added**: +60 ## Improvements Made ### 1. New Coverage: `NewErrorCallToolResult` The `NewErrorCallToolResult` function had **0% coverage** — it was entirely untested. Added `TestNewErrorCallToolResult` with three subtests: - ✅ Non-nil error returns `IsError=true` result with the error message as `TextContent` - ✅ Nil error input substitutes `"unknown error"` message (covers the nil-guard branch) - ✅ Error messages with special characters (e.g. embedded JSON) are preserved verbatim ### 2. New Coverage: `ConvertToCallToolResult` marshal error path Added `TestConvertToCallToolResult_MarshalError` to cover the early-return error path when the data argument cannot be JSON-marshaled (e.g., passing a Go channel). This path was previously unreachable by any test. ### 3. Coverage Improvement | Function | Before | After | |---|---|---| | `NewErrorCallToolResult` | **0%** | **100%** | | `ConvertToCallToolResult` | 89.7% | 93.1% | | `internal/mcp` package total | 81.0% | 81.6% | ## Test Execution All new and existing tests pass: ``` === RUN TestNewErrorCallToolResult === RUN TestNewErrorCallToolResult/non-nil_error_produces_IsError_result_with_error_message_as_text === RUN TestNewErrorCallToolResult/nil_error_substitutes_unknown_error_message === RUN TestNewErrorCallToolResult/error_message_with_special_characters_is_preserved --- PASS: TestNewErrorCallToolResult (0.00s) === RUN TestConvertToCallToolResult_MarshalError --- PASS: TestConvertToCallToolResult_MarshalError (0.00s) PASS ok github.com/github/gh-aw-mcpg/internal/mcp 0.011s ``` ## Why These Changes? `NewErrorCallToolResult` is a utility used when a tool call fails and the gateway needs to produce a structured error response back to the MCP client. Despite being a public, used function, it had zero test coverage. The nil-error guard and the function's three return values (result, nil, err) are now all exercised. The `ConvertToCallToolResult` marshal error path guards against programming errors where an un-serializable type is accidentally passed in; this defensive path was untested. --- *Generated by Test Improver Workflow* *Focuses on better patterns, increased coverage, and more stable tests* > [!WARNING] > <details> > <summary><strong>⚠️ Firewall blocked 1 domain</strong></summary> > > The following domain was blocked by the firewall during workflow execution: > > - `invalidhostthatdoesnotexist12345.com` > > To allow these domains, add them to the `network.allowed` list in your workflow frontmatter: > > ```yaml > network: > allowed: > - defaults > - "invalidhostthatdoesnotexist12345.com" > ``` > > See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information. > > </details> > Generated by [Test Improver](https://github.com/github/gh-aw-mcpg/actions/runs/24563044421/agentic_workflow) · ● 3.6M · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw-mcpg+%22gh-aw-workflow-id%3A+test-improver%22&type=pullrequests) <!-- gh-aw-agentic-workflow: Test Improver, engine: copilot, model: auto, id: 24563044421, workflow_id: test-improver, run: https://github.com/github/gh-aw-mcpg/actions/runs/24563044421 --> <!-- gh-aw-workflow-id: test-improver -->
2 parents b9c4059 + 7a758a2 commit b0670d0

File tree

1 file changed

+60
-0
lines changed

1 file changed

+60
-0
lines changed

internal/mcp/tool_result_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package mcp
22

33
import (
44
"encoding/json"
5+
"errors"
56
"testing"
67

78
sdk "github.com/modelcontextprotocol/go-sdk/mcp"
@@ -315,3 +316,62 @@ func TestParseToolArguments(t *testing.T) {
315316
assert.Empty(t, args)
316317
})
317318
}
319+
320+
// TestNewErrorCallToolResult tests construction of error CallToolResult values.
321+
func TestNewErrorCallToolResult(t *testing.T) {
322+
t.Run("non-nil error produces IsError result with error message as text", func(t *testing.T) {
323+
inputErr := errors.New("tool execution failed")
324+
325+
result, second, returnedErr := NewErrorCallToolResult(inputErr)
326+
327+
require.NotNil(t, result)
328+
assert.True(t, result.IsError)
329+
assert.Nil(t, second)
330+
assert.ErrorIs(t, returnedErr, inputErr)
331+
332+
require.Len(t, result.Content, 1)
333+
text, ok := result.Content[0].(*sdk.TextContent)
334+
require.True(t, ok, "Expected TextContent")
335+
assert.Equal(t, "tool execution failed", text.Text)
336+
})
337+
338+
t.Run("nil error substitutes unknown error message", func(t *testing.T) {
339+
result, second, returnedErr := NewErrorCallToolResult(nil)
340+
341+
require.NotNil(t, result)
342+
assert.True(t, result.IsError)
343+
assert.Nil(t, second)
344+
require.Error(t, returnedErr)
345+
assert.Equal(t, "unknown error", returnedErr.Error())
346+
347+
require.Len(t, result.Content, 1)
348+
text, ok := result.Content[0].(*sdk.TextContent)
349+
require.True(t, ok, "Expected TextContent")
350+
assert.Equal(t, "unknown error", text.Text)
351+
})
352+
353+
t.Run("error message with special characters is preserved", func(t *testing.T) {
354+
inputErr := errors.New(`backend error: {"code":500,"message":"internal server error"}`)
355+
356+
result, _, returnedErr := NewErrorCallToolResult(inputErr)
357+
358+
require.NotNil(t, result)
359+
assert.True(t, result.IsError)
360+
assert.ErrorIs(t, returnedErr, inputErr)
361+
362+
require.Len(t, result.Content, 1)
363+
text, ok := result.Content[0].(*sdk.TextContent)
364+
require.True(t, ok)
365+
assert.Equal(t, inputErr.Error(), text.Text)
366+
})
367+
}
368+
369+
// TestConvertToCallToolResult_MarshalError tests the error path when data cannot be marshaled.
370+
func TestConvertToCallToolResult_MarshalError(t *testing.T) {
371+
// Channels cannot be marshaled to JSON; json.Marshal returns an error.
372+
result, err := ConvertToCallToolResult(make(chan int))
373+
374+
assert.Error(t, err)
375+
assert.Nil(t, result)
376+
assert.Contains(t, err.Error(), "failed to marshal backend result")
377+
}

0 commit comments

Comments
 (0)