Skip to content

Commit d9d7f6c

Browse files
DanielPoeppelmannjulian-rischanakin87
authored
test: added new integrations/mcp tests to harden and increase coverage (#3155)
* check invoke vs ainvoke return the same * remove cov.xml * MCPToolset tool returns parsed dict * test(mcp): cover lazy toolset warm-up lifecycle * test(mcp): validate lazy toolset config on warm-up * test(mcp): validate lazy tool config on warm-up * test(mcp): cover lazy tool lookup failures * test(mcp): cover structured output parsing paths * test(mcp): verify client cleanup clears stale state * fix tests --------- Co-authored-by: Julian Risch <julian.risch@deepset.ai> Co-authored-by: anakin87 <stefanofiorucci@gmail.com>
1 parent b3b0020 commit d9d7f6c

File tree

3 files changed

+288
-2
lines changed

3 files changed

+288
-2
lines changed

integrations/mcp/tests/mcp_servers_fixtures.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
from mcp import types
12
from mcp.server.fastmcp import FastMCP
23

34
################################################
@@ -55,3 +56,16 @@ def state_subtract(a: int, b: int) -> dict:
5556
def echo(text: str) -> str:
5657
"""Echo the input text."""
5758
return text
59+
60+
61+
################################################
62+
# Image MCP Server
63+
################################################
64+
65+
image_mcp = FastMCP("Image")
66+
67+
68+
@image_mcp.tool()
69+
def image_tool() -> list[types.ImageContent]:
70+
"""Return image content without any text blocks."""
71+
return [types.ImageContent(type="image", data="ZmFrZQ==", mimeType="image/png")]

integrations/mcp/tests/test_mcp_tool.py

Lines changed: 169 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import io
22
import json
33
import os
4+
from types import SimpleNamespace
45
from unittest.mock import AsyncMock, MagicMock, patch
56

67
import pytest
@@ -13,12 +14,13 @@
1314

1415
from haystack_integrations.tools.mcp import (
1516
MCPTool,
17+
MCPToolNotFoundError,
1618
StdioServerInfo,
1719
)
1820
from haystack_integrations.tools.mcp.mcp_tool import StdioClient, _extract_first_text_element
1921

2022
from .mcp_memory_transport import InMemoryServerInfo
21-
from .mcp_servers_fixtures import calculator_mcp, echo_mcp
23+
from .mcp_servers_fixtures import calculator_mcp, echo_mcp, image_mcp, state_calculator_mcp
2224

2325

2426
@tool
@@ -104,6 +106,41 @@ def test_mcp_tool_invoke(self, mcp_add_tool, mcp_echo_tool):
104106
echo_result = json.loads(echo_result)
105107
assert echo_result["content"][0]["text"] == "Hello MCP!"
106108

109+
def test_mcp_tool_outputs_to_state_falls_back_to_full_response_for_non_text_content(self, mcp_tool_cleanup):
110+
"""Test that non-text MCP content returns the full parsed response when state output is enabled."""
111+
server_info = InMemoryServerInfo(server=image_mcp._mcp_server)
112+
tool = MCPTool(
113+
name="image_tool",
114+
server_info=server_info,
115+
eager_connect=True,
116+
outputs_to_state={"image_payload": {}},
117+
)
118+
mcp_tool_cleanup(tool)
119+
120+
result = tool.invoke()
121+
122+
assert isinstance(result, dict)
123+
assert len(result["content"]) == 1
124+
assert result["content"][0]["type"] == "image"
125+
assert result["content"][0]["data"] == "ZmFrZQ=="
126+
assert result["content"][0]["mimeType"] == "image/png"
127+
assert result["isError"] is False
128+
129+
def test_mcp_tool_outputs_to_state_returns_raw_text_when_text_is_not_json(self, mcp_tool_cleanup):
130+
"""Test that plain text content is returned as-is when state output parsing cannot decode JSON."""
131+
server_info = InMemoryServerInfo(server=echo_mcp._mcp_server)
132+
tool = MCPTool(
133+
name="echo",
134+
server_info=server_info,
135+
eager_connect=True,
136+
outputs_to_state={"echo_payload": {}},
137+
)
138+
mcp_tool_cleanup(tool)
139+
140+
result = tool.invoke(text="Hello MCP!")
141+
142+
assert result == "Hello MCP!"
143+
107144
def test_mcp_tool_error_handling(self, mcp_error_tool):
108145
"""Test error handling with the in-memory server."""
109146
with pytest.raises(ToolInvocationError) as exc_info:
@@ -114,6 +151,47 @@ def test_mcp_tool_error_handling(self, mcp_error_tool):
114151
# The first part of the message comes from ToolInvocationError's formatting
115152
assert "Failed to invoke Tool `divide_by_zero`" in error_message
116153

154+
def test_mcp_tool_lazy_missing_tool_raises_with_available_tools(self, mcp_tool_cleanup):
155+
"""Test that lazy warm-up surfaces missing-tool errors with the available tool names."""
156+
server_info = InMemoryServerInfo(server=calculator_mcp._mcp_server)
157+
tool = MCPTool(name="multiply", server_info=server_info, eager_connect=False)
158+
mcp_tool_cleanup(tool)
159+
160+
mock_worker = MagicMock()
161+
mock_worker.tools.return_value = [
162+
SimpleNamespace(name="add"),
163+
SimpleNamespace(name="subtract"),
164+
SimpleNamespace(name="divide_by_zero"),
165+
]
166+
167+
with (
168+
patch("haystack_integrations.tools.mcp.mcp_tool._MCPClientSessionManager", return_value=mock_worker),
169+
pytest.raises(MCPToolNotFoundError) as exc_info,
170+
):
171+
tool.warm_up()
172+
173+
assert exc_info.value.tool_name == "multiply"
174+
assert set(exc_info.value.available_tools) == {"add", "subtract", "divide_by_zero"}
175+
176+
def test_mcp_tool_lazy_no_tools_server_raises_tool_not_found(self, mcp_tool_cleanup):
177+
"""Test that lazy warm-up fails cleanly when the server exposes no tools."""
178+
server_info = InMemoryServerInfo(server=calculator_mcp._mcp_server)
179+
tool = MCPTool(name="anything", server_info=server_info, eager_connect=False)
180+
mcp_tool_cleanup(tool)
181+
182+
mock_worker = MagicMock()
183+
mock_worker.tools.return_value = []
184+
185+
with (
186+
patch("haystack_integrations.tools.mcp.mcp_tool._MCPClientSessionManager", return_value=mock_worker),
187+
pytest.raises(MCPToolNotFoundError) as exc_info,
188+
):
189+
tool.warm_up()
190+
191+
assert str(exc_info.value) == "No tools available on server"
192+
assert exc_info.value.tool_name == "anything"
193+
assert exc_info.value.available_tools == []
194+
117195
def test_mcp_tool_serde(self, mcp_tool_cleanup):
118196
"""Test serialization and deserialization of MCPTool with in-memory server."""
119197
server_info = InMemoryServerInfo(server=calculator_mcp._mcp_server)
@@ -186,6 +264,22 @@ def test_mcp_tool_state_mapping_parameters(self, mcp_tool_cleanup):
186264
assert "b" in tool.parameters["properties"]
187265
assert "b" in tool.parameters["required"]
188266

267+
def test_mcp_tool_eager_state_mapping_removes_inputs_from_schema(self, mcp_tool_cleanup):
268+
"""Test that eager MCPTool initialization removes state-injected params from its public schema."""
269+
server_info = InMemoryServerInfo(server=calculator_mcp._mcp_server)
270+
tool = MCPTool(
271+
name="add",
272+
server_info=server_info,
273+
eager_connect=True,
274+
inputs_from_state={"state_a": "a"},
275+
)
276+
mcp_tool_cleanup(tool)
277+
278+
assert "a" not in tool.parameters["properties"]
279+
assert "a" not in tool.parameters.get("required", [])
280+
assert "b" in tool.parameters["properties"]
281+
assert "b" in tool.parameters["required"]
282+
189283
def test_mcp_tool_serde_with_state_mapping(self, mcp_tool_cleanup):
190284
"""Test serialization and deserialization of MCPTool with state-mapping parameters."""
191285
server_info = InMemoryServerInfo(server=calculator_mcp._mcp_server)
@@ -219,6 +313,62 @@ def test_mcp_tool_serde_with_state_mapping(self, mcp_tool_cleanup):
219313
assert new_tool._inputs_from_state == {"state_a": "a"}
220314
assert new_tool._outputs_to_state == {"result": {"source": "output"}}
221315

316+
@pytest.mark.skipif(
317+
not hasattr(__import__("haystack.tools", fromlist=["Tool"]).Tool, "_get_valid_inputs"),
318+
reason="Requires Haystack >= 2.22.0 for inputs_from_state validation",
319+
)
320+
def test_mcp_tool_lazy_invalid_parameter_raises_on_warm_up(self, mcp_tool_cleanup):
321+
"""Test that lazy MCPTool defers invalid inputs_from_state validation until warm_up()."""
322+
server_info = InMemoryServerInfo(server=calculator_mcp._mcp_server)
323+
tool = MCPTool(
324+
name="add",
325+
server_info=server_info,
326+
eager_connect=False,
327+
inputs_from_state={"state_key": "non_existent_param"},
328+
)
329+
mcp_tool_cleanup(tool)
330+
331+
assert tool.parameters == {"type": "object", "properties": {}, "additionalProperties": True}
332+
333+
with pytest.raises(ValueError, match="unknown parameter"):
334+
tool.warm_up()
335+
336+
def test_mcp_tool_invoke_auto_warms_up_once(self, mcp_tool_cleanup):
337+
"""Test that lazy MCPTool initializes on first invoke and reuses that connection."""
338+
server_info = InMemoryServerInfo(server=calculator_mcp._mcp_server)
339+
tool = MCPTool(name="add", server_info=server_info, eager_connect=False)
340+
mcp_tool_cleanup(tool)
341+
342+
assert tool.parameters == {"type": "object", "properties": {}, "additionalProperties": True}
343+
344+
with patch.object(tool, "_connect_and_initialize", wraps=tool._connect_and_initialize) as mock_connect:
345+
first_result = json.loads(tool.invoke(a=20, b=22))
346+
second_result = json.loads(tool.invoke(a=1, b=2))
347+
348+
assert first_result["content"][0]["text"] == "42"
349+
assert second_result["content"][0]["text"] == "3"
350+
assert "a" in tool.parameters["properties"]
351+
assert "b" in tool.parameters["properties"]
352+
assert mock_connect.call_count == 1
353+
354+
@pytest.mark.asyncio
355+
async def test_mcp_tool_ainvoke_matches_invoke_with_outputs_to_state(self, mcp_tool_cleanup):
356+
"""Test that sync and async invocation paths return the same parsed state output."""
357+
server_info = InMemoryServerInfo(server=state_calculator_mcp._mcp_server)
358+
tool = MCPTool(
359+
name="state_add",
360+
server_info=server_info,
361+
eager_connect=True,
362+
outputs_to_state={"result": {"source": "result"}},
363+
)
364+
mcp_tool_cleanup(tool)
365+
366+
sync_result = tool.invoke(a=20, b=22)
367+
async_result = await tool.ainvoke(a=20, b=22)
368+
369+
assert sync_result == {"result": 42}
370+
assert async_result == sync_result
371+
222372
@pytest.mark.asyncio
223373
@pytest.mark.parametrize(
224374
"fileno_side_effect,fileno_return_value,notebook_environment",
@@ -255,6 +405,24 @@ async def test_stdio_client_stderr_handling(self, fileno_side_effect, fileno_ret
255405
else:
256406
assert errlog is mock_stderr
257407

408+
@pytest.mark.asyncio
409+
async def test_mcp_client_aclose_clears_references_even_when_cleanup_fails(self, caplog):
410+
"""Test that client cleanup always clears connection state, even if exit_stack cleanup raises."""
411+
client = StdioClient(command="echo")
412+
client.session = MagicMock()
413+
client.stdio = MagicMock()
414+
client.write = MagicMock()
415+
client.exit_stack = MagicMock()
416+
client.exit_stack.aclose = AsyncMock(side_effect=RuntimeError("cleanup failed"))
417+
418+
with caplog.at_level("WARNING"):
419+
await client.aclose()
420+
421+
assert any("Error during MCP client cleanup: cleanup failed" in record.message for record in caplog.records)
422+
assert client.session is None
423+
assert client.stdio is None
424+
assert client.write is None
425+
258426
@pytest.mark.skipif(not os.environ.get("OPENAI_API_KEY"), reason="OPENAI_API_KEY not set")
259427
@pytest.mark.integration
260428
def test_pipeline_warmup_with_mcp_tool(self):

integrations/mcp/tests/test_mcp_toolset.py

Lines changed: 105 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030

3131
# Import in-memory transport and fixtures
3232
from .mcp_memory_transport import InMemoryServerInfo
33-
from .mcp_servers_fixtures import calculator_mcp, echo_mcp
33+
from .mcp_servers_fixtures import calculator_mcp, echo_mcp, image_mcp, state_calculator_mcp
3434

3535
logger = logging.getLogger(__name__)
3636

@@ -152,6 +152,16 @@ async def test_echo_toolset(self, echo_toolset):
152152
assert echo_tool.name == "echo"
153153
assert "Echo the input text." in echo_tool.description
154154

155+
async def test_toolset_invoke_returns_raw_json_string_without_outputs_to_state(self, echo_toolset):
156+
"""Test that toolset-created tools keep the raw MCP JSON when no state output parsing is configured."""
157+
echo_tool = echo_toolset.tools[0]
158+
159+
result = echo_tool.invoke(text="Hello MCP!")
160+
parsed = json.loads(result)
161+
162+
assert parsed["content"][0]["text"] == "Hello MCP!"
163+
assert parsed["isError"] is False
164+
155165
async def test_toolset_with_filtered_tools(self, calculator_toolset_with_tool_filter):
156166
"""Test if the MCPToolset correctly filters tools based on tool_names parameter."""
157167
toolset = calculator_toolset_with_tool_filter
@@ -172,6 +182,24 @@ async def test_toolset_with_filtered_tools(self, calculator_toolset_with_tool_fi
172182
assert tool.name == "add"
173183
assert "Add two integers." in tool.description
174184

185+
async def test_toolset_warm_up_replaces_placeholder_and_is_idempotent(self, mcp_tool_cleanup):
186+
"""Test lazy toolsets swap the placeholder tool for real tools exactly once."""
187+
server_info = InMemoryServerInfo(server=calculator_mcp._mcp_server)
188+
toolset = MCPToolset(server_info=server_info, eager_connect=False)
189+
mcp_tool_cleanup(toolset)
190+
191+
assert len(toolset.tools) == 1
192+
assert toolset.tools[0].name.startswith("mcp_not_connected_placeholder_")
193+
194+
toolset.warm_up()
195+
warmed_tool_names = [tool.name for tool in toolset.tools]
196+
197+
assert set(warmed_tool_names) == {"add", "subtract", "divide_by_zero"}
198+
assert not any(name.startswith("mcp_not_connected_placeholder_") for name in warmed_tool_names)
199+
200+
toolset.warm_up()
201+
assert [tool.name for tool in toolset.tools] == warmed_tool_names
202+
175203
async def test_toolset_serde(self, calculator_toolset):
176204
"""Test serialization and deserialization of MCPToolset."""
177205
toolset = calculator_toolset
@@ -292,6 +320,59 @@ async def test_toolset_with_state_config(self, calculator_toolset_with_state_con
292320
assert add_tool.outputs_to_string is not None
293321
assert subtract_tool.outputs_to_string is None
294322

323+
async def test_toolset_invoke_returns_parsed_dict_when_outputs_to_state_configured(self, mcp_tool_cleanup):
324+
"""Test that toolset-created tools parse MCP text content into dicts for state updates."""
325+
server_info = InMemoryServerInfo(server=state_calculator_mcp._mcp_server)
326+
toolset = MCPToolset(
327+
server_info=server_info,
328+
tool_names=["state_add"],
329+
eager_connect=True,
330+
outputs_to_state={"state_add": {"result": {"source": "result"}}},
331+
)
332+
mcp_tool_cleanup(toolset)
333+
334+
add_tool = toolset.tools[0]
335+
result = add_tool.invoke(a=20, b=22)
336+
337+
assert result == {"result": 42}
338+
339+
async def test_toolset_returns_full_response_for_non_text_content_with_outputs_to_state(self, mcp_tool_cleanup):
340+
"""Test that toolset-created tools preserve full MCP payloads when there is no text content to parse."""
341+
server_info = InMemoryServerInfo(server=image_mcp._mcp_server)
342+
toolset = MCPToolset(
343+
server_info=server_info,
344+
tool_names=["image_tool"],
345+
eager_connect=True,
346+
outputs_to_state={"image_tool": {"image_payload": {}}},
347+
)
348+
mcp_tool_cleanup(toolset)
349+
350+
image_tool = toolset.tools[0]
351+
result = image_tool.invoke()
352+
353+
assert isinstance(result, dict)
354+
assert len(result["content"]) == 1
355+
assert result["content"][0]["type"] == "image"
356+
assert result["content"][0]["data"] == "ZmFrZQ=="
357+
assert result["content"][0]["mimeType"] == "image/png"
358+
assert result["isError"] is False
359+
360+
async def test_toolset_returns_raw_text_when_outputs_to_state_content_is_not_json(self, mcp_tool_cleanup):
361+
"""Test that toolset-created tools preserve plain text when JSON decoding is not possible."""
362+
server_info = InMemoryServerInfo(server=echo_mcp._mcp_server)
363+
toolset = MCPToolset(
364+
server_info=server_info,
365+
tool_names=["echo"],
366+
eager_connect=True,
367+
outputs_to_state={"echo": {"echo_payload": {}}},
368+
)
369+
mcp_tool_cleanup(toolset)
370+
371+
echo_tool = toolset.tools[0]
372+
result = echo_tool.invoke(text="Hello MCP!")
373+
374+
assert result == "Hello MCP!"
375+
295376
async def test_toolset_state_config_serde(self, calculator_toolset_with_state_config, mcp_tool_cleanup):
296377
"""Test serialization and deserialization of MCPToolset with state configuration."""
297378
toolset = calculator_toolset_with_state_config
@@ -373,6 +454,29 @@ async def test_toolset_state_config_invalid_parameter_raises_error(self):
373454
},
374455
)
375456

457+
@pytest.mark.skipif(
458+
not hasattr(__import__("haystack.tools", fromlist=["Tool"]).Tool, "_get_valid_inputs"),
459+
reason="Requires Haystack >= 2.22.0 for inputs_from_state validation",
460+
)
461+
async def test_toolset_lazy_invalid_parameter_raises_on_warm_up(self, mcp_tool_cleanup):
462+
"""Test that lazy toolsets defer invalid inputs_from_state validation until warm_up()."""
463+
server_info = InMemoryServerInfo(server=calculator_mcp._mcp_server)
464+
toolset = MCPToolset(
465+
server_info=server_info,
466+
tool_names=["add"],
467+
eager_connect=False,
468+
inputs_from_state={
469+
"add": {"state_key": "non_existent_param"},
470+
},
471+
)
472+
mcp_tool_cleanup(toolset)
473+
474+
assert len(toolset.tools) == 1
475+
assert toolset.tools[0].name.startswith("mcp_not_connected_placeholder_")
476+
477+
with pytest.raises(ValueError, match="unknown parameter"):
478+
toolset.warm_up()
479+
376480
async def test_toolset_no_state_config(self, calculator_toolset):
377481
"""Test that tools have no state config when none is provided."""
378482
toolset = calculator_toolset

0 commit comments

Comments
 (0)