[test-improver] Improve tests for mcp schema package#1867
Merged
Conversation
Add internal/mcp/schema_test.go with comprehensive unit tests for the
NormalizeInputSchema function in schema.go. Previously this function
had no test file in the mcp package itself; coverage only existed
through integration tests in the server package.
New tests cover all 8 branches of the normalization logic:
- nil schema → default empty object schema
- empty schema (no type, no properties) → empty object schema
- no type with properties → add "type": "object" (copy)
- non-string type value → returned as-is
- non-"object" type string (e.g. "string", "array") → returned as-is
- object type with properties → returned as-is
- object type with only additionalProperties → returned as-is
- object type missing both properties and additionalProperties → add empty properties (copy)
Additional tests verify:
- Extra fields ("required", "description") are preserved when a copy is made
- Original schema is never mutated in copy-producing branches
- Returned map is a distinct reference from the original
- Various toolName variants do not affect normalization logic
- Deeply nested schemas are returned without modification
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
lpcox
marked this pull request as ready for review
March 13, 2026 18:30
Contributor
There was a problem hiding this comment.
Pull request overview
Adds direct unit-test coverage for internal/mcp.NormalizeInputSchema, complementing existing integration-style coverage in internal/server and ensuring go test -cover ./internal/mcp/... exercises schema.go.
Changes:
- Introduces
internal/mcp/schema_test.gowith table-driven tests covering all normalization branches. - Adds focused tests for field preservation and non-mutation guarantees in copy-producing branches.
- Exercises
toolNameinput variants and a deeply nested schema “no-op” case.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| } | ||
|
|
||
| // TestNormalizeInputSchema_PreservesExtraFields verifies that extra fields like | ||
| // "required", "description", and "$schema" are preserved in copy paths. |
Comment on lines
+237
to
+259
| schema := map[string]interface{}{ | ||
| "type": "object", | ||
| } | ||
| expected := map[string]interface{}{ | ||
| "type": "object", | ||
| "properties": map[string]interface{}{}, | ||
| } | ||
|
|
||
| toolNames := []string{ | ||
| "simple", | ||
| "", | ||
| "server___tool-name", | ||
| "tool with spaces", | ||
| "tool/with/slashes", | ||
| "very-long-tool-name-that-exceeds-normal-length-limits-for-mcp-tool-names", | ||
| } | ||
|
|
||
| for _, name := range toolNames { | ||
| t.Run("tool name: "+name, func(t *testing.T) { | ||
| result := NormalizeInputSchema(schema, name) | ||
| assert.Equal(t, expected, result, "normalization result should not depend on toolName") | ||
| }) | ||
| } |
lpcox
added a commit
that referenced
this pull request
Mar 13, 2026
Removes `internal/mcp/types_test.go` which was superseded by `schema_test.go` (added in 50b1f31 via PR #1867). The duplicate `TestNormalizeInputSchema_NilSchema` declaration caused a `go vet` error that broke `make agent-finished`. All 13 test cases from `types_test.go` are covered by the more comprehensive `schema_test.go` (`AllBranches`, `PreservesExtraFields`, `DoesNotMutateOriginal`, `ToolNameVariants`, `DeepNestedSchema`). `make agent-finished` passes after this change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Test Improvements:
schema_test.goFile Analyzed
internal/mcp/schema_test.gointernal/mcpinternal/mcp/schema.go—NormalizeInputSchemafunctionWhy This File Was Selected
internal/mcp/schema.gocontained theNormalizeInputSchemafunction with 8 distinct code branches but had no unit test file in themcppackage itself. The only tests were integration-style tests ininternal/server/schema_normalization_test.go(a different package), which means:go test -cover ./internal/mcp/...would show 0% coverage forschema.goImprovements Made
1. Direct Unit Test Coverage in the Correct Package
internal/mcp/schema_test.goin themcppackage for direct coverageassertfor clean, idiomatic assertionsnamefields for all branch cases2. Complete Branch Coverage
All 8 branches of
NormalizeInputSchemaare now exercised with direct unit tests:nilschema{}"type": "object"via copytypevalue"object"type string"string","array", etc.)propertiesadditionalProperties"properties": {}via copy3. New Edge Cases Not Covered Elsewhere
"required","description"survive copy-producing branchestoolNamevariants: empty string, special chars, long names — all normalized identically4. Cleaner Test Structure
TestNormalizeInputSchema_NilSchemafor the most fundamental caseTestNormalizeInputSchema_AllBranchesfor the complete table-driven setTest Structure
Coverage Impact
internal/mcp(schema.go)The server-package integration tests remain unchanged and continue to provide cross-package validation.
Generated by Test Improver Workflow
Focuses on better patterns, increased coverage, and more stable tests