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>
This commit is contained in:
Bill Easton 2026-04-12 12:14:52 -05:00 committed by GitHub
commit fb03e85592
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 12 additions and 131 deletions

View file

@ -653,14 +653,8 @@ class TestPathTraversalPrevention:
mcp.add_provider(SkillsDirectoryProvider(roots=skills_dir))
async with Client(mcp) as client:
# Path traversal attempts should fail (either normalized away or blocked)
# The important thing is that SECRET DATA is never returned
# Path traversal attempts should fail — the secret must never be returned
with pytest.raises(Exception):
result = await client.read_resource(
await client.read_resource(
AnyUrl("skill://test-skill/../../../secret.txt")
)
# If we somehow got here, ensure we didn't get the secret
if result:
for content in result:
if hasattr(content, "text"):
assert "SECRET DATA" not in content.text

View file

@ -13,6 +13,7 @@ from mcp.types import TextContent
from pydantic import BaseModel
from fastmcp import Client, FastMCP
from fastmcp.exceptions import ToolError
class UserProfile(BaseModel):
@ -130,8 +131,13 @@ class TestPydanticModelArguments:
assert "Alice" in result.content[0].text
assert "30" in result.content[0].text
async def test_pydantic_model_with_stringified_json_no_strict(self):
"""Test if stringified JSON is accepted for Pydantic models without strict validation."""
async def test_stringified_json_not_auto_parsed_for_pydantic_models(self):
"""Stringified JSON is rejected for Pydantic model parameters.
Some LLM clients send stringified JSON (a JSON string containing a
JSON object) instead of a proper JSON object. FastMCP does not
auto-parse these; callers get a validation error.
"""
mcp = FastMCP("TestServer", strict_input_validation=False)
@mcp.tool
@ -140,33 +146,12 @@ class TestPydanticModelArguments:
return f"Created user {profile.name}, age {profile.age}"
async with Client(mcp) as client:
# Some LLM clients send stringified JSON instead of actual JSON
stringified = json.dumps(
{"name": "Bob", "age": 25, "email": "bob@example.com"}
)
# This test verifies whether we handle stringified JSON
error_msg = ""
try:
result = await client.call_tool("create_user", {"profile": stringified})
# If this succeeds, we're handling stringified JSON
assert isinstance(result.content[0], TextContent)
assert "Bob" in result.content[0].text
stringified_json_works = True
except Exception as e:
# If this fails, we're not handling stringified JSON
stringified_json_works = False
error_msg = str(e)
# Document the behavior - we want to know if this works or not
if stringified_json_works:
# This is the desired behavior
pass
else:
# This means stringified JSON doesn't work - document it
assert (
"validation" in error_msg.lower() or "invalid" in error_msg.lower()
)
with pytest.raises(ToolError, match="validation"):
await client.call_tool("create_user", {"profile": stringified})
async def test_pydantic_model_with_coercion(self):
"""Pydantic models should benefit from coercion without strict validation."""

View file

@ -1,98 +0,0 @@
# from pathlib import Path
# from typing import TYPE_CHECKING, Any
# import pytest
# import fastmcp
# from fastmcp import FastMCP
# if TYPE_CHECKING:
# pass
# USERS = [
# {"id": "1", "name": "Alice", "active": True},
# {"id": "2", "name": "Bob", "active": True},
# {"id": "3", "name": "Charlie", "active": False},
# ]
# @pytest.fixture
# def fastmcp_server():
# server = FastMCP("TestServer")
# # --- Tools ---
# @server.tool
# def greet(name: str) -> str:
# """Greet someone by name."""
# return f"Hello, {name}!"
# @server.tool
# def add(a: int, b: int) -> int:
# """Add two numbers together."""
# return a + b
# @server.tool
# def error_tool():
# """This tool always raises an error."""
# raise ValueError("This is a test error")
# # --- Resources ---
# @server.resource(uri="resource://wave")
# def wave() -> str:
# return "👋"
# @server.resource(uri="data://users")
# async def get_users() -> list[dict[str, Any]]:
# return USERS
# @server.resource(uri="data://user/{user_id}")
# async def get_user(user_id: str) -> dict[str, Any] | None:
# return next((user for user in USERS if user["id"] == user_id), None)
# # --- Prompts ---
# @server.prompt
# def welcome(name: str) -> str:
# return f"Welcome to FastMCP, {name}!"
# return server
# @pytest.fixture
# async def stdio_client():
# # Find the stdio.py script path
# base_dir = Path(__file__).parent
# stdio_script = base_dir / "test_servers" / "stdio.py"
# if not stdio_script.exists():
# raise FileNotFoundError(f"Could not find stdio.py script at {stdio_script}")
# client = fastmcp.Client(
# transport=fastmcp.client.transports.StdioTransport(
# command="python",
# args=[str(stdio_script)],
# )
# )
# async with client:
# print("READY")
# yield client
# print("DONE")
# class TestRunServerStdio:
# async def test_run_server_stdio(
# self, fastmcp_server: FastMCP, stdio_client: fastmcp.Client
# ):
# print("TEST")
# tools = await stdio_client.list_tools()
# print("TEST 2")
# assert tools == 1
# class TestRunServerSSE:
#
# async def test_run_server_sse(self, fastmcp_server: FastMCP):
# pass