Skip to content

Commit fb03e85

Browse files
strawgateclaude
andauthored
Fix high-severity test quality issues (#3854)
- Delete entirely commented-out test_run_server.py (99 lines dead code) - Fix test_pydantic_model_with_stringified_json_no_strict: replace try/except-both-branches-pass with clear pytest.raises assertion - Fix test_path_traversal_blocked: remove dead assertions after pytest.raises (lines after raise never execute) Error handling middleware test fixes are in a separate PR (#3858) which also fixes the underlying RetryMiddleware bug. 🤖 Generated with Claude Code Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent cae9333 commit fb03e85

3 files changed

Lines changed: 12 additions & 131 deletions

File tree

tests/server/providers/test_skills_provider.py

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -653,14 +653,8 @@ async def test_path_traversal_blocked(self, tmp_path: Path):
653653
mcp.add_provider(SkillsDirectoryProvider(roots=skills_dir))
654654

655655
async with Client(mcp) as client:
656-
# Path traversal attempts should fail (either normalized away or blocked)
657-
# The important thing is that SECRET DATA is never returned
656+
# Path traversal attempts should fail — the secret must never be returned
658657
with pytest.raises(Exception):
659-
result = await client.read_resource(
658+
await client.read_resource(
660659
AnyUrl("skill://test-skill/../../../secret.txt")
661660
)
662-
# If we somehow got here, ensure we didn't get the secret
663-
if result:
664-
for content in result:
665-
if hasattr(content, "text"):
666-
assert "SECRET DATA" not in content.text

tests/server/test_input_validation.py

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from pydantic import BaseModel
1414

1515
from fastmcp import Client, FastMCP
16+
from fastmcp.exceptions import ToolError
1617

1718

1819
class UserProfile(BaseModel):
@@ -130,8 +131,13 @@ def create_user(profile: UserProfile) -> str:
130131
assert "Alice" in result.content[0].text
131132
assert "30" in result.content[0].text
132133

133-
async def test_pydantic_model_with_stringified_json_no_strict(self):
134-
"""Test if stringified JSON is accepted for Pydantic models without strict validation."""
134+
async def test_stringified_json_not_auto_parsed_for_pydantic_models(self):
135+
"""Stringified JSON is rejected for Pydantic model parameters.
136+
137+
Some LLM clients send stringified JSON (a JSON string containing a
138+
JSON object) instead of a proper JSON object. FastMCP does not
139+
auto-parse these; callers get a validation error.
140+
"""
135141
mcp = FastMCP("TestServer", strict_input_validation=False)
136142

137143
@mcp.tool
@@ -140,33 +146,12 @@ def create_user(profile: UserProfile) -> str:
140146
return f"Created user {profile.name}, age {profile.age}"
141147

142148
async with Client(mcp) as client:
143-
# Some LLM clients send stringified JSON instead of actual JSON
144149
stringified = json.dumps(
145150
{"name": "Bob", "age": 25, "email": "bob@example.com"}
146151
)
147152

148-
# This test verifies whether we handle stringified JSON
149-
error_msg = ""
150-
try:
151-
result = await client.call_tool("create_user", {"profile": stringified})
152-
# If this succeeds, we're handling stringified JSON
153-
assert isinstance(result.content[0], TextContent)
154-
assert "Bob" in result.content[0].text
155-
stringified_json_works = True
156-
except Exception as e:
157-
# If this fails, we're not handling stringified JSON
158-
stringified_json_works = False
159-
error_msg = str(e)
160-
161-
# Document the behavior - we want to know if this works or not
162-
if stringified_json_works:
163-
# This is the desired behavior
164-
pass
165-
else:
166-
# This means stringified JSON doesn't work - document it
167-
assert (
168-
"validation" in error_msg.lower() or "invalid" in error_msg.lower()
169-
)
153+
with pytest.raises(ToolError, match="validation"):
154+
await client.call_tool("create_user", {"profile": stringified})
170155

171156
async def test_pydantic_model_with_coercion(self):
172157
"""Pydantic models should benefit from coercion without strict validation."""

tests/server/test_run_server.py

Lines changed: 0 additions & 98 deletions
This file was deleted.

0 commit comments

Comments
 (0)