diff --git a/test/serena/test_mcp.py b/test/serena/test_mcp.py index 454b7b8..0a940d3 100644 --- a/test/serena/test_mcp.py +++ b/test/serena/test_mcp.py @@ -200,31 +200,94 @@ def test_make_tool_missing_apply() -> None: make_tool(tool) -def test_make_tool_missing_description() -> None: - """Test make_tool with a function that has no description in the docstring.""" +@pytest.mark.parametrize( + "docstring, expected_description", + [ + ( + """This is a test function. - class NoDescriptionTool(BaseMockTool): - def apply(self, param: str) -> str: + :param param: The parameter + :return: A result + """, + "This is a test function. Returns A result.", + ), + ( """ :param param: The parameter :return: A result + """, + "Returns A result.", + ), + ( """ + :param param: The parameter + """, + "", + ), + ("", ""), + ], +) +def test_make_tool_descriptions(docstring, expected_description) -> None: + """Test make_tool with various docstring formats.""" + + class TestTool(BaseMockTool): + def apply(self, param: str) -> str: return f"Result: {param}" def apply_ex(self, *args, **kwargs) -> str: return self.apply(**kwargs) - tool = NoDescriptionTool() + # Dynamically set the docstring + TestTool.apply.__doc__ = docstring + + tool = TestTool() mcp_tool = make_tool(tool) - assert mcp_tool.name == "no_description" - assert mcp_tool.description == "Returns A result." - assert mcp_tool.parameters["properties"]["param"]["description"] == "The parameter." + assert mcp_tool.name == "test" + assert mcp_tool.description == expected_description -def test_make_tool_all_tools() -> None: +def is_test_mock_class(tool_class: type) -> bool: + """Check if a class is a test mock class.""" + # Check if the class is defined in a test module + module_name = tool_class.__module__ + return ( + module_name.startswith(("test.", "tests.")) + or "test_" in module_name + or tool_class.__name__ + in [ + "BaseMockTool", + "BasicTool", + "BadTool", + "NoParamsTool", + "NoReturnTool", + "MissingParamTool", + "ComplexDocTool", + "FormatTool", + "NoDescriptionTool", + ] + ) + + +def get_real_tool_classes(): + """Get all non-test, non-abstract tool classes.""" + from serena.agent import iter_tool_classes + + for tool_class in iter_tool_classes(): + # Skip abstract base classes that can't be instantiated + if tool_class.__name__ == "Tool" or getattr(tool_class, "__abstractmethods__", set()): + continue + + # Skip test mock classes + if is_test_mock_class(tool_class): + continue + + yield tool_class + + +@pytest.mark.parametrize("tool_class", list(get_real_tool_classes())) +def test_make_tool_all_tools(tool_class) -> None: """Test that make_tool works for all tools in the codebase.""" - from serena.agent import iter_tool_classes, Tool # Create a mock agent for tool initialization class MockAgent: @@ -232,36 +295,15 @@ def test_make_tool_all_tools() -> None: self.project_config = None self.serena_config = None - # Get all tool classes - tool_classes = list(iter_tool_classes()) - - # Make sure we found some tools - assert len(tool_classes) > 0 - - # Test each tool class - for tool_class in tool_classes: - try: - # Skip abstract base classes that can't be instantiated - if tool_class.__name__ == "Tool" or getattr(tool_class, "__abstractmethods__", set()): - continue - - # Skip test mock classes that don't properly implement __init__ - if tool_class.__name__ in ["BaseMockTool", "BasicTool", "BadTool", "NoParamsTool", "NoReturnTool", - "MissingParamTool", "ComplexDocTool", "FormatTool", "NoDescriptionTool"]: - continue - - # Create an instance of the tool - tool_instance = tool_class(MockAgent()) - - # Try to create an MCP tool from it - mcp_tool = make_tool(tool_instance) - - # Basic validation - assert isinstance(mcp_tool, MCPTool) - assert mcp_tool.name == tool_class.get_name() - - # The description should be a string (either from docstring or default) - assert isinstance(mcp_tool.description, str) - - except Exception as e: - pytest.fail(f"Failed to create MCP tool for {tool_class.__name__}: {e}") + # Create an instance of the tool + tool_instance = tool_class(MockAgent()) + + # Try to create an MCP tool from it + mcp_tool = make_tool(tool_instance) + + # Basic validation + assert isinstance(mcp_tool, MCPTool) + assert mcp_tool.name == tool_class.get_name() + + # The description should be a string (either from docstring or default) + assert isinstance(mcp_tool.description, str) diff --git a/test_make_tool.py b/verify_fix.py similarity index 74% rename from test_make_tool.py rename to verify_fix.py index 2933bd5..d0a2833 100644 --- a/test_make_tool.py +++ b/verify_fix.py @@ -1,7 +1,7 @@ -"""Simple test for the make_tool function.""" +"""Simple script to verify our fix for the make_tool function.""" import docstring_parser -from typing import Any, Callable +from typing import Any, Optional def make_tool_simplified(func_doc: str) -> str: @@ -32,6 +32,7 @@ def test_with_description(): """ result = make_tool_simplified(doc) assert result == "This is a test function. Returns A greeting message." + print("✅ Test with description passed") def test_without_description(): @@ -43,6 +44,7 @@ def test_without_description(): """ result = make_tool_simplified(doc) assert result == "Returns A greeting message." + print("✅ Test without description passed") def test_empty_docstring(): @@ -50,10 +52,24 @@ def test_empty_docstring(): doc = "" result = make_tool_simplified(doc) assert result == "" + print("✅ Test with empty docstring passed") + + +def test_no_return(): + """Test with a docstring that has no return description.""" + doc = """This is a test function. + + :param name: The person's name + :param age: The person's age + """ + result = make_tool_simplified(doc) + assert result == "This is a test function." + print("✅ Test with no return passed") if __name__ == "__main__": test_with_description() test_without_description() test_empty_docstring() + test_no_return() print("All tests passed!") \ No newline at end of file