Skip to content

[test-improver] Improve tests for mcp tool_result package#4254

Merged
lpcox merged 1 commit intomainfrom
test-improver/mcp-tool-result-a945857cadb6ea2f
Apr 21, 2026
Merged

[test-improver] Improve tests for mcp tool_result package#4254
lpcox merged 1 commit intomainfrom
test-improver/mcp-tool-result-a945857cadb6ea2f

Conversation

@github-actions
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 Added: +38

Improvements Made

1. Increased Coverage

  • ✅ Added test for ConvertToCallToolResult when the content field is a non-array value (e.g. a JSON string) — triggers the json.Unmarshal error fallback that wraps raw bytes as a text content item

  • ✅ Added test documenting ParseToolArguments behavior with null JSON input — shows it returns a nil map (not empty map) without error

  • ConvertToCallToolResult coverage: 93.1% → 100%

  • ParseToolArguments coverage: maintained at 100%

  • NewErrorCallToolResult coverage: maintained at 100%

2. Cleaner & More Stable Tests

  • ✅ New tests follow the existing t.Run() subtest style in the file (not table-driven, consistent with surrounding code)
  • ✅ Use require/assert from testify for precise, idiomatic assertions

Test Execution

All internal/mcp tests pass:

ok  	github.com/github/gh-aw-mcpg/internal/mcp	0.744s	coverage: 82.0% of statements
github.com/github/gh-aw-mcpg/internal/mcp/tool_result.go:15:	ConvertToCallToolResult	100.0%
github.com/github/gh-aw-mcpg/internal/mcp/tool_result.go:131:	ParseToolArguments		100.0%
github.com/github/gh-aw-mcpg/internal/mcp/tool_result.go:148:	NewErrorCallToolResult	100.0%

Note: TestFetchAndFixSchema_NetworkError in internal/config is a pre-existing failure unrelated to this PR (network test making a real HTTP request that returns 403 in CI).

Why These Changes?

ConvertToCallToolResult had an untested fallback path: when the response JSON has a content field that is not an array (e.g. {"content": "some string"}), the typed struct unmarshal fails and the function falls back to wrapping the raw bytes as a text content item. This path was completely untested.

The ParseToolArguments null-JSON test documents a subtle but important behavior: passing null JSON yields a nil map (not an empty map), which callers should be aware of.


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:

  • invalidhostthatdoesnotexist12345.com

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

network:
  allowed:
    - defaults
    - "invalidhostthatdoesnotexist12345.com"

See Network Configuration for more information.

Generated by Test Improver · ● 7.6M ·

Add two new test cases to internal/mcp/tool_result_test.go:
1. Cover the json.Unmarshal error fallback in ConvertToCallToolResult when
   the 'content' field is a non-array value (e.g. a string), bringing
   ConvertToCallToolResult coverage from 93.1% to 100%.
2. Document ParseToolArguments behavior with null JSON input (yields nil map,
   not an empty map), covering the previously-untested null-JSON path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review April 21, 2026 14:19
Copilot AI review requested due to automatic review settings April 21, 2026 14:19
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Improves test coverage and documentation of edge-case behavior in the internal/mcp tool result utilities, specifically around ConvertToCallToolResult fallback parsing and ParseToolArguments handling of null JSON.

Changes:

  • Adds a test covering ConvertToCallToolResult’s fallback behavior when "content" is present but not an array.
  • Adds a test documenting that ParseToolArguments returns a nil map (without error) when arguments are JSON null.
Show a summary per file
File Description
internal/mcp/tool_result_test.go Adds subtests to cover previously untested parsing fallback and to document null-arguments behavior.

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: 0

@lpcox lpcox merged commit cebccbe into main Apr 21, 2026
21 checks passed
@lpcox lpcox deleted the test-improver/mcp-tool-result-a945857cadb6ea2f branch April 21, 2026 14:22
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.

2 participants