Skip to content

Commit 4552921

Browse files
authored
[test-improver] Improve tests for mcp/tool_result (#5788)
# Test Improvements: `tool_result_test.go` ## File Analyzed - **Test File**: `internal/mcp/tool_result_test.go` - **Package**: `internal/mcp` - **Implementation**: `internal/mcp/tool_result.go` ## Improvements Made ### 1. Increased Coverage - ✅ Added test for audio content with missing `data` field - ✅ Added test for image content with unsupported data type (e.g., `int`) — covers `decodeContentData` `default:` branch - ✅ Added test for image content with `nil` data value - ✅ Added test for resource content where `json.Unmarshal` into `sdk.ResourceContents` fails - **Previous Coverage**: `decodeContentData` 85.7%, `convertContentItem` 96.4% - **New Coverage**: `decodeContentData` **100%**, `convertContentItem` **100%** ### 2. Better Testing Patterns - ✅ New tests use existing testify assertion patterns (`assert.Error`, `assert.ErrorContains`, `assert.Nil`) consistent with the file's style - ✅ Descriptive subtest names explain the invariant being asserted - ✅ Comments explain why each case is important (e.g., guarding against corrupted backend responses) ### 3. Cleaner & More Stable Tests - ✅ Each subtest is independent with no shared state - ✅ Tests use inline data, no external dependencies ## Test Execution All tests pass: ``` ok github.com/github/gh-aw-mcpg/internal/mcp 2.325s ``` Coverage improvement in `tool_result.go`: ``` tool_result.go:145: decodeContentData 85.7% → 100.0% tool_result.go:98: convertContentItem 96.4% → 100.0% ``` ## Why These Changes? `tool_result.go` is central to MCP response handling — it converts every backend tool result into the SDK format. The uncovered branches represent real error conditions (unsupported data types, unmarshalable resources) that could surface from misbehaving backends. Adding these tests ensures the error paths are exercised and their error messages are validated. --- *Generated by Test Improver Workflow* *Focuses on better patterns, increased coverage, and more stable tests* > [!WARNING] > <details> > <summary>Firewall blocked 1 domain</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/25947156108/agentic_workflow) · ● 2.5M · [◷](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, version: 1.0.40, model: claude-sonnet-4.6, id: 25947156108, workflow_id: test-improver, run: https://github.com/github/gh-aw-mcpg/actions/runs/25947156108 --> <!-- gh-aw-workflow-id: test-improver -->
2 parents 822fe77 + c543a5a commit 4552921

1 file changed

Lines changed: 55 additions & 0 deletions

File tree

internal/mcp/tool_result_test.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,44 @@ func TestDecodeContentData(t *testing.T) {
507507
assert.Nil(t, result)
508508
assert.ErrorContains(t, err, "failed to decode audio data")
509509
})
510+
511+
t.Run("missing audio data field returns error", func(t *testing.T) {
512+
ci := map[string]interface{}{
513+
"type": "audio",
514+
"mimeType": "audio/wav",
515+
// no "data" key
516+
}
517+
result, err := convertContentItem(ci)
518+
assert.Error(t, err, "missing audio data should produce an error")
519+
assert.Nil(t, result)
520+
assert.ErrorContains(t, err, "failed to decode audio data")
521+
})
522+
523+
t.Run("unsupported data type for image returns error", func(t *testing.T) {
524+
// decodeContentData must reject types that are neither []byte nor string.
525+
ci := map[string]interface{}{
526+
"type": "image",
527+
"mimeType": "image/png",
528+
"data": 42, // integer — not a valid data carrier
529+
}
530+
result, err := convertContentItem(ci)
531+
assert.Error(t, err, "unsupported data type should produce an error")
532+
assert.Nil(t, result)
533+
assert.ErrorContains(t, err, "failed to decode image data")
534+
})
535+
536+
t.Run("nil data value in image content returns error", func(t *testing.T) {
537+
// data key exists but is nil — decodeContentData must treat this as missing.
538+
ci := map[string]interface{}{
539+
"type": "image",
540+
"mimeType": "image/png",
541+
"data": nil,
542+
}
543+
result, err := convertContentItem(ci)
544+
assert.Error(t, err, "nil data value should produce an error")
545+
assert.Nil(t, result)
546+
assert.ErrorContains(t, err, "failed to decode image data")
547+
})
510548
}
511549

512550
// TestConvertContentItem_ResourceMarshalError verifies that an unmarshalable
@@ -523,6 +561,23 @@ func TestConvertContentItem_ResourceMarshalError(t *testing.T) {
523561
assert.ErrorContains(t, err, "failed to marshal resource")
524562
}
525563

564+
// TestConvertContentItem_ResourceUnmarshalError verifies that resource JSON that
565+
// cannot be unmarshaled into sdk.ResourceContents returns an appropriate error.
566+
func TestConvertContentItem_ResourceUnmarshalError(t *testing.T) {
567+
// Provide a resource value where a required string field contains an object,
568+
// which will marshal fine but fail to unmarshal into sdk.ResourceContents.
569+
ci := map[string]interface{}{
570+
"type": "resource",
571+
"resource": map[string]interface{}{
572+
"uri": map[string]interface{}{"nested": "object"}, // URI must be a string
573+
},
574+
}
575+
result, err := convertContentItem(ci)
576+
assert.Error(t, err)
577+
assert.Nil(t, result)
578+
assert.ErrorContains(t, err, "failed to parse resource")
579+
}
580+
526581
// TestConvertMapToCallToolResult_ContentItemCastFailure verifies that
527582
// malformed []interface{} content entries are rejected instead of being
528583
// silently skipped, to avoid data loss.

0 commit comments

Comments
 (0)