Improve tests for make_tool function

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)

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!")