Skip to content

[test-improver] Improve tests for mcp package - #7067

Merged
lpcox merged 3 commits into
mainfrom
test-improver/mcp-tool-result-712685ed8c624234
Jun 6, 2026
Merged

[test-improver] Improve tests for mcp package#7067
lpcox merged 3 commits into
mainfrom
test-improver/mcp-tool-result-712685ed8c624234

Conversation

@github-actions

@github-actions github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Test Improvements: tool_result_test.go

File Analyzed

  • Test File: internal/mcp/tool_result_test.go
  • Package: internal/mcp
  • Lines of Code: +93 lines added

Improvements Made

1. Increased Coverage — Close the convertMapToCallToolResult gap

The previously uncovered path was return nil, err at line 86–88 of tool_result.go — the error-propagation branch inside convertMapToCallToolResult's item-conversion loop. This branch was never reachable via the public ConvertToCallToolResult API because:

  • TestDecodeContentData and TestConvertContentItem_* called convertContentItem directly, bypassing the wrapping function.
  • TestConvertMapToCallToolResult_ContentItemCastFailure failed at the earlier type-assertion check (line 70–72), not inside the item loop.

New test: TestConvertToCallToolResult_ContentItemErrorPropagation

A table-driven test with 4 cases that go through the full ConvertToCallToolResult → convertMapToCallToolResult call chain to exercise line 86–88:

Case Content type Error trigger
Invalid image base64 via []interface{} []interface{} invalid base64 string in data field
Invalid audio base64 via []interface{} []interface{} invalid base64 string in data field
Invalid image base64 via []map[string]interface{} []map[string]interface{} invalid base64 string in data field
Valid item followed by invalid item []interface{} error propagates from second item in slice
  • Previous Coverage: convertMapToCallToolResult at 95.8%
  • New Coverage: convertMapToCallToolResult at 100.0%
  • Improvement: +4.2% on this function; all tool_result.go functions now at 100%

2. Cleaner & More Stable Tests

  • ✅ Added t.Parallel() to all 11 top-level test functions in the file — tests that are stateless (no shared mutable state, no t.Setenv()) now run concurrently, reducing total test time.
  • ✅ New test uses require.Error + require.Nil for fail-fast assertions at the subtest level.

Test Execution

All tests pass with improved coverage:

=== RUN   TestConvertToCallToolResult_ContentItemErrorPropagation
--- PASS: TestConvertToCallToolResult_ContentItemErrorPropagation (0.00s)
    --- PASS: .../invalid_image_base64_via_[]interface{}_content_slice (0.00s)
    --- PASS: .../invalid_audio_base64_via_[]interface{}_content_slice (0.00s)
    --- PASS: .../invalid_image_base64_via_[]map[string]interface{}_content_slice (0.00s)
    --- PASS: .../valid_item_followed_by_invalid_item_stops_at_first_error (0.00s)
PASS
ok  github.com/github/gh-aw-mcpg/internal/mcp  2.948s  coverage: 92.6% of statements

github.com/github/gh-aw-mcpg/internal/mcp/tool_result.go:53:  convertMapToCallToolResult  100.0%

Full suite (all packages):

ok  github.com/github/gh-aw-mcpg/internal/mcp        2.948s
ok  github.com/github/gh-aw-mcpg/test/integration   14.191s
... (all 27 packages pass)

Why These Changes?

tool_result.go is a critical conversion layer between raw backend JSON responses and the typed MCP SDK CallToolResult. The previously uncovered error path in convertMapToCallToolResult could silently mask content-item conversion failures — a bug here would corrupt tool responses. Making this path explicitly tested via the public API ensures the error propagation contract is enforced.

The t.Parallel() additions are purely mechanical quality improvements: the test functions are already stateless, so enabling parallel execution costs nothing and speeds up go test -v runs.


Generated by Test Improver Workflow
Focuses on better patterns, increased coverage, and more stable tests

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • index.crates.io

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "index.crates.io"

See Network Configuration for more information.

Generated by Test Improver · sonnet46 10.7M ·

… path

- Add TestConvertToCallToolResult_ContentItemErrorPropagation: table-driven
  test covering the previously-unreachable 'return nil, err' on line 86-88
  of convertMapToCallToolResult via the full ConvertToCallToolResult public API
- Exercises both []interface{} and []map[string]interface{} content slice types
- Brings convertMapToCallToolResult coverage from 95.8% to 100%
- Add t.Parallel() to all top-level test functions in the file for faster
  parallel test execution

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

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.

Pull request overview

This PR improves test coverage for the internal/mcp tool-result conversion layer by adding a new table-driven test that exercises an previously-uncovered error-propagation branch in convertMapToCallToolResult, and by parallelizing the package’s top-level tests to reduce runtime.

Changes:

  • Added a new TestConvertToCallToolResult_ContentItemErrorPropagation to drive failures through the public ConvertToCallToolResult API and validate error propagation from convertContentItem.
  • Added t.Parallel() to top-level tests in internal/mcp/tool_result_test.go to allow concurrent execution.
Show a summary per file
File Description
internal/mcp/tool_result_test.go Adds coverage for error propagation in content-item conversion and parallelizes top-level tests.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 1/1 changed files
  • Comments generated: 1

Comment thread internal/mcp/tool_result_test.go
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lpcox

lpcox commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator

Copilot AI commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

@copilot fix the failing ci check https://github.com/github/gh-aw-mcpg/actions/runs/27066716486/job/79888766757?pr=7067

Fixed in commit 00c1302 by applying gofmt to internal/mcp/tool_result_test.go, which resolves the failing lint check for that workflow job.

Copilot finished work on behalf of lpcox June 6, 2026 16:04
Copilot AI requested a review from lpcox June 6, 2026 16:04
@lpcox
lpcox merged commit c6a72e4 into main Jun 6, 2026
16 checks passed
@lpcox
lpcox deleted the test-improver/mcp-tool-result-712685ed8c624234 branch June 6, 2026 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants