refactor(tools): use MCPManager interface in NewMCPTool constructor
- Change NewMCPTool to accept MCPManager interface instead of concrete *mcp.Manager - Remove unused mcpPkg import from mcp_tool.go - Remove newMCPToolForTest helper function as NewMCPTool now accepts interface - Update all tests to use NewMCPTool directly with MockMCPManager - Improves testability and follows dependency inversion principle
This commit is contained in:
parent
96712e372c
commit
67a5865059
2 changed files with 13 additions and 23 deletions
|
|
@ -6,7 +6,6 @@ import (
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
"github.com/modelcontextprotocol/go-sdk/mcp"
|
"github.com/modelcontextprotocol/go-sdk/mcp"
|
||||||
mcpPkg "github.com/sipeed/picoclaw/pkg/mcp"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
// MCPManager defines the interface for MCP manager operations
|
// MCPManager defines the interface for MCP manager operations
|
||||||
|
|
@ -23,7 +22,7 @@ type MCPTool struct {
|
||||||
}
|
}
|
||||||
|
|
||||||
// NewMCPTool creates a new MCP tool wrapper
|
// NewMCPTool creates a new MCP tool wrapper
|
||||||
func NewMCPTool(manager *mcpPkg.Manager, serverName string, tool *mcp.Tool) *MCPTool {
|
func NewMCPTool(manager MCPManager, serverName string, tool *mcp.Tool) *MCPTool {
|
||||||
return &MCPTool{
|
return &MCPTool{
|
||||||
manager: manager,
|
manager: manager,
|
||||||
serverName: serverName,
|
serverName: serverName,
|
||||||
|
|
|
||||||
|
|
@ -26,15 +26,6 @@ func (m *MockMCPManager) CallTool(ctx context.Context, serverName, toolName stri
|
||||||
}, nil
|
}, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// newMCPToolForTest creates an MCP tool for testing with mock manager
|
|
||||||
func newMCPToolForTest(manager MCPManager, serverName string, tool *mcp.Tool) *MCPTool {
|
|
||||||
return &MCPTool{
|
|
||||||
manager: manager,
|
|
||||||
serverName: serverName,
|
|
||||||
tool: tool,
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestNewMCPTool verifies MCP tool creation
|
// TestNewMCPTool verifies MCP tool creation
|
||||||
func TestNewMCPTool(t *testing.T) {
|
func TestNewMCPTool(t *testing.T) {
|
||||||
manager := &MockMCPManager{}
|
manager := &MockMCPManager{}
|
||||||
|
|
@ -48,11 +39,11 @@ func TestNewMCPTool(t *testing.T) {
|
||||||
"type": "string",
|
"type": "string",
|
||||||
"description": "Test input",
|
"description": "Test input",
|
||||||
},
|
},
|
||||||
|
},
|
||||||
},
|
},
|
||||||
},
|
}
|
||||||
}
|
|
||||||
|
|
||||||
mcpTool := newMCPToolForTest(manager, "test_server", tool)
|
mcpTool := NewMCPTool(manager, "test_server", tool)
|
||||||
|
|
||||||
if mcpTool == nil {
|
if mcpTool == nil {
|
||||||
t.Fatal("NewMCPTool should not return nil")
|
t.Fatal("NewMCPTool should not return nil")
|
||||||
|
|
@ -95,7 +86,7 @@ func TestMCPTool_Name(t *testing.T) {
|
||||||
t.Run(tt.name, func(t *testing.T) {
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
manager := &MockMCPManager{}
|
manager := &MockMCPManager{}
|
||||||
tool := &mcp.Tool{Name: tt.toolName}
|
tool := &mcp.Tool{Name: tt.toolName}
|
||||||
mcpTool := newMCPToolForTest(manager, tt.serverName, tool)
|
mcpTool := NewMCPTool(manager, tt.serverName, tool)
|
||||||
|
|
||||||
result := mcpTool.Name()
|
result := mcpTool.Name()
|
||||||
if result != tt.expected {
|
if result != tt.expected {
|
||||||
|
|
@ -134,7 +125,7 @@ func TestMCPTool_Description(t *testing.T) {
|
||||||
Name: "test_tool",
|
Name: "test_tool",
|
||||||
Description: tt.toolDescription,
|
Description: tt.toolDescription,
|
||||||
}
|
}
|
||||||
mcpTool := newMCPToolForTest(manager, tt.serverName, tool)
|
mcpTool := NewMCPTool(manager, tt.serverName, tool)
|
||||||
|
|
||||||
result := mcpTool.Description()
|
result := mcpTool.Description()
|
||||||
|
|
||||||
|
|
@ -182,7 +173,7 @@ func TestMCPTool_Parameters(t *testing.T) {
|
||||||
Name: "test_tool",
|
Name: "test_tool",
|
||||||
InputSchema: tt.inputSchema,
|
InputSchema: tt.inputSchema,
|
||||||
}
|
}
|
||||||
mcpTool := newMCPToolForTest(manager, "test_server", tool)
|
mcpTool := NewMCPTool(manager, "test_server", tool)
|
||||||
|
|
||||||
params := mcpTool.Parameters()
|
params := mcpTool.Parameters()
|
||||||
|
|
||||||
|
|
@ -222,7 +213,7 @@ func TestMCPTool_Execute_Success(t *testing.T) {
|
||||||
Name: "search_repos",
|
Name: "search_repos",
|
||||||
Description: "Search GitHub repositories",
|
Description: "Search GitHub repositories",
|
||||||
}
|
}
|
||||||
mcpTool := newMCPToolForTest(manager, "github", tool)
|
mcpTool := NewMCPTool(manager, "github", tool)
|
||||||
|
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
args := map[string]interface{}{
|
args := map[string]interface{}{
|
||||||
|
|
@ -251,7 +242,7 @@ func TestMCPTool_Execute_ManagerError(t *testing.T) {
|
||||||
}
|
}
|
||||||
|
|
||||||
tool := &mcp.Tool{Name: "test_tool"}
|
tool := &mcp.Tool{Name: "test_tool"}
|
||||||
mcpTool := newMCPToolForTest(manager, "test_server", tool)
|
mcpTool := NewMCPTool(manager, "test_server", tool)
|
||||||
|
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
result := mcpTool.Execute(ctx, map[string]interface{}{})
|
result := mcpTool.Execute(ctx, map[string]interface{}{})
|
||||||
|
|
@ -284,7 +275,7 @@ func TestMCPTool_Execute_ServerError(t *testing.T) {
|
||||||
}
|
}
|
||||||
|
|
||||||
tool := &mcp.Tool{Name: "test_tool"}
|
tool := &mcp.Tool{Name: "test_tool"}
|
||||||
mcpTool := newMCPToolForTest(manager, "test_server", tool)
|
mcpTool := NewMCPTool(manager, "test_server", tool)
|
||||||
|
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
result := mcpTool.Execute(ctx, map[string]interface{}{})
|
result := mcpTool.Execute(ctx, map[string]interface{}{})
|
||||||
|
|
@ -319,7 +310,7 @@ func TestMCPTool_Execute_MultipleContent(t *testing.T) {
|
||||||
}
|
}
|
||||||
|
|
||||||
tool := &mcp.Tool{Name: "multi_output"}
|
tool := &mcp.Tool{Name: "multi_output"}
|
||||||
mcpTool := newMCPToolForTest(manager, "test_server", tool)
|
mcpTool := NewMCPTool(manager, "test_server", tool)
|
||||||
|
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
result := mcpTool.Execute(ctx, map[string]interface{}{})
|
result := mcpTool.Execute(ctx, map[string]interface{}{})
|
||||||
|
|
@ -407,7 +398,7 @@ func TestExtractContentText_EmptyContent(t *testing.T) {
|
||||||
func TestMCPTool_InterfaceCompliance(t *testing.T) {
|
func TestMCPTool_InterfaceCompliance(t *testing.T) {
|
||||||
manager := &MockMCPManager{}
|
manager := &MockMCPManager{}
|
||||||
tool := &mcp.Tool{Name: "test"}
|
tool := &mcp.Tool{Name: "test"}
|
||||||
mcpTool := newMCPToolForTest(manager, "test_server", tool)
|
mcpTool := NewMCPTool(manager, "test_server", tool)
|
||||||
|
|
||||||
// Verify it implements Tool interface
|
// Verify it implements Tool interface
|
||||||
var _ Tool = mcpTool
|
var _ Tool = mcpTool
|
||||||
|
|
@ -431,7 +422,7 @@ func TestMCPTool_Parameters_MapSchema(t *testing.T) {
|
||||||
Name: "test_tool",
|
Name: "test_tool",
|
||||||
InputSchema: schema,
|
InputSchema: schema,
|
||||||
}
|
}
|
||||||
mcpTool := newMCPToolForTest(manager, "test_server", tool)
|
mcpTool := NewMCPTool(manager, "test_server", tool)
|
||||||
|
|
||||||
params := mcpTool.Parameters()
|
params := mcpTool.Parameters()
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue