Add mcptest package for in-process MCP testing
#149
Conversation
WalkthroughThis update introduces a new package for testing MCP servers, providing a Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
mcptest/mcptest_test.go (1)
69-85: Helper function could handle more content types.The current implementation only handles
TextContentand returns an error for other content types. While this works for the current test, it might be limiting for future tests that need to handle different content types.Consider making this function more flexible by handling other content types or providing a way to customize content handling based on the test's needs.
func resultToString(result *mcp.CallToolResult) (string, error) { var b strings.Builder for _, content := range result.Content { - text, ok := content.(mcp.TextContent) - if !ok { - return "", fmt.Errorf("unsupported content type: %T", content) - } - b.WriteString(text.Text) + switch c := content.(type) { + case mcp.TextContent: + b.WriteString(c.Text) + case mcp.ImageContent: + b.WriteString("[Image Content]") + default: + return "", fmt.Errorf("unsupported content type: %T", content) + } } if result.IsError { return "", fmt.Errorf("%s", b.String()) } return b.String(), nil }mcptest/mcptest.go (1)
77-97: Consider adding synchronization for server readiness.The
Startmethod launches the server in a goroutine but doesn't provide a way to know when the server is actually ready to process requests. While it works for simple tests, more complex scenarios might need to ensure the server is fully initialized.Consider adding a mechanism, such as a ready channel, to signal when the server is fully initialized and ready to accept requests:
func (s *Server) Start() { + ready := make(chan struct{}) // Start the MCP server in a goroutine go func() { mcpServer := server.NewMCPServer(s.name, "1.0.0") mcpServer.AddTools(s.tools...) logger := log.New(&s.logBuffer, "", 0) stdioServer := server.NewStdioServer(mcpServer) stdioServer.SetErrorLogger(logger) + // Signal that we're ready to accept connections + close(ready) if err := stdioServer.Listen(s.ctx, s.serverReader, s.serverWriter); err != nil { logger.Println("StdioServer.Listen failed:", err) } }() s.client = client.NewStdioMCPClientWithIO(s.clientReader, s.clientWriter, io.NopCloser(&s.logBuffer)) + // Wait for the server to be ready + <-ready }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
client/stdio.go(3 hunks)mcptest/mcptest.go(1 hunks)mcptest/mcptest_test.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
mcptest/mcptest.go (5)
server/server.go (2)
ServerTool(48-51)ToolHandlerFunc(42-42)client/stdio.go (2)
StdioMCPClient(22-35)NewStdioMCPClientWithIO(40-59)mcp/tools.go (1)
Tool(69-78)server/stdio.go (1)
NewStdioServer(82-91)client/client.go (1)
MCPClient(13-111)
client/stdio.go (1)
client/types.go (1)
RPCResponse(5-8)
mcptest/mcptest_test.go (4)
mcptest/mcptest.go (1)
NewServer(35-41)server/server.go (1)
ServerTool(48-51)mcp/types.go (3)
InitializeRequest(266-275)Params(104-104)Content(660-662)mcp/utils.go (1)
NewToolResultText(208-217)
🔇 Additional comments (9)
client/stdio.go (3)
37-59: Well-implemented function for in-process client testing.This new function allows creating a
StdioMCPClientusing existing I/O streams instead of launching a subprocess, which is perfect for testing purposes. The implementation correctly initializes all necessary fields and sets up the response reading goroutine.
95-95: Good refactoring of client creation.Using the newly created
NewStdioMCPClientWithIOfunction here simplifies the code and reduces duplication. This change aligns well with the DRY principle.
109-111: Properly handles null command case.This addition correctly handles the case where the client was created with
NewStdioMCPClientWithIO(wherecmdis nil), preventing potential nil pointer dereferences.mcptest/mcptest_test.go (2)
14-57: Comprehensive test for the new MCP server testing framework.The test effectively demonstrates the usage of the
mcptestpackage by creating a server with a custom tool, initializing a client connection, and validating the tool's response. This serves as both a functional test and documentation example.
59-67: Well-implemented handler with appropriate default value.The handler function correctly extracts the name parameter from the request arguments and provides a sensible default value of "World" when the name is missing or not a string.
mcptest/mcptest.go (4)
16-32: Well-structured Server type with all necessary components.The
Serverstruct encapsulates all the components needed for testing MCP servers, including context management, I/O pipes, and client handling. This design makes it easy to use in tests.
34-41: Convenient helper function for standard test setup.This function provides a clean API for the common case of creating and starting a server with specified tools, making tests more concise and readable.
43-62: Good separation of server creation and starting.The
NewUnstartedServerfunction allows adding tools before starting the server, providing flexibility for more complex test setups. The TODO comment about usingt.Context()in Go 1.24 shows good forward thinking.
99-110: Thorough cleanup in Close method.The
Closemethod properly handles cleanup of resources by closing the client and canceling the context. It also checks if they are nil before attempting to close them, preventing potential nil pointer dereferences.
This function allows creating a `*transport.Stdio` using provided `io.Reader` and `io.Writer`. This allows creating an MCP client to a server running in the same process, which significanly simplifies testing.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
client/transport/stdio.go(3 hunks)mcptest/mcptest.go(1 hunks)mcptest/mcptest_test.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- mcptest/mcptest_test.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
client/transport/stdio.go (2)
client/transport/interface.go (1)
JSONRPCResponse(36-45)testdata/mockstdio_server.go (1)
JSONRPCResponse(18-26)
🪛 golangci-lint (1.64.8)
mcptest/mcptest.go
39-39: Error return value of server.Start is not checked
(errcheck)
🔇 Additional comments (9)
client/transport/stdio.go (4)
36-48: Well-designed constructor for testing scenarios!This new
NewIOconstructor follows good dependency injection practices by accepting existing I/O streams instead of spawning a subprocess. This approach makes testing more reliable and controlled by eliminating process management complexity.
71-84: Good refactoring to separate process management from transport initialization.The refactored
Startmethod now properly delegates subprocess handling tostartProcwhile handling the response reading goroutine separately. This separation of concerns improves maintainability and allows the transport to work without a subprocess when needed.
86-124: Clean extraction of subprocess logic.The new
startProcmethod nicely encapsulates the subprocess spawning logic, with an appropriate early return when no command is specified. This supports the goal of enabling in-process testing without external processes.
136-142: Properly handles nil subprocess case.The modified
Closemethod now correctly checks ifc.cmdis nil before callingWait(), which is necessary when using the transport with in-memory I/O streams viaNewIO.mcptest/mcptest.go (5)
17-33: Well-designed Server struct with complete resource management.The
Serverstruct effectively encapsulates all components needed for in-process MCP testing: context management, I/O pipes, logging, and transport interface. This comprehensive design supports isolated testing environments.
44-63: Good separation between server creation and startup.The
NewUnstartedServerfunction provides flexibility by allowing tools to be added before starting the server. The comment about usingt.Context()in Go 1.24 shows good forward-thinking for future improvements.
78-100: Clean server startup implementation with proper resource management.The
Startmethod effectively:
- Launches the server asynchronously in a goroutine
- Sets up logging to an internal buffer
- Establishes transport over pipes using the new
transport.NewIOconstructorThis implementation properly integrates with the transport layer changes and maintains error reporting through the logger.
102-114: Thorough cleanup with nil checks.The
Closemethod properly releases all resources by closing the transport and canceling the context. The nil checks ensure safe cleanup even if the server wasn't fully initialized or was already closed.
115-118: Simple client accessor simplifies test writing.This method provides a convenient way to obtain a connected client without having to manage transport setup manually, making test code more concise and focusing on the actual test logic rather than connection setup.
|
Rebased on |
The new `mcptest` package provides functions for setting up an in-process MCP server and an MCP client connected to it. This allows testing MCP implementations end-to-end without spawning any processes. The `mcptest` package itself has a unit test that demonstrates how to use the package.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
mcptest/mcptest.go (3)
54-55: Consider using t.Cleanup() until Go 1.24While waiting for Go 1.24's
t.Context(), you could leveraget.Cleanup()to automatically close the server when the test completes:// Use t.Context() once we switch to go >= 1.24 ctx := context.TODO() + +// Register cleanup to ensure proper resource management +t.Cleanup(func() { + if server.cancel != nil { + server.cancel() + } +})This ensures resources are released even if a test panics or forgets to call Close().
85-98: Consider a mechanism to communicate server startup errorsThe goroutine logs errors from
stdioServer.Listen()but has no way to communicate these errors back to the caller. This could make debugging test failures difficult since crucial errors might only appear in logs.Consider using a channel to propagate errors back to the caller:
func (s *Server) Start() error { + errChan := make(chan error, 1) // Start the MCP server in a goroutine go func() { mcpServer := server.NewMCPServer(s.name, "1.0.0") mcpServer.AddTools(s.tools...) logger := log.New(&s.logBuffer, "", 0) stdioServer := server.NewStdioServer(mcpServer) stdioServer.SetErrorLogger(logger) if err := stdioServer.Listen(s.ctx, s.serverReader, s.serverWriter); err != nil { logger.Println("StdioServer.Listen failed:", err) + select { + case errChan <- err: + default: + } } }() s.transport = transport.NewIO(s.clientReader, s.clientWriter, io.NopCloser(&s.logBuffer)) - return s.transport.Start(s.ctx) + if err := s.transport.Start(s.ctx); err != nil { + return err + } + + // Check if server setup failed + select { + case err := <-errChan: + return err + default: + return nil + } }
118-121: Access to server logs could be beneficialThe
Serverstruct captures logs inlogBuffer, but there's no method to access these logs. Consider adding aLogs()method to retrieve server logs for debugging test failures:// Client returns an MCP client connected to the server. func (s *Server) Client() client.MCPClient { return client.NewClient(s.transport) } +// Logs returns the captured server logs as a string. +func (s *Server) Logs() string { + return s.logBuffer.String() +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
mcptest/mcptest.go(1 hunks)mcptest/mcptest_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- mcptest/mcptest_test.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
mcptest/mcptest.go (7)
server/server.go (2)
ServerTool(48-51)ToolHandlerFunc(42-42)client/transport/interface.go (1)
Interface(11-27)mcp/tools.go (1)
Tool(69-80)server/stdio.go (1)
NewStdioServer(82-91)client/transport/stdio.go (1)
NewIO(39-48)client/client.go (2)
Client(16-24)NewClient(34-38)client/interface.go (1)
MCPClient(11-109)
🔇 Additional comments (2)
mcptest/mcptest.go (2)
36-45: Fixed error handling in NewServerThe function now correctly checks and returns errors from
server.Start(), addressing the issue flagged in the previous review. This is a good improvement that prevents silent failures during test setup.
1-122: Overall implementation looks solidThe
mcptestpackage successfully implements in-process MCP testing without spawning external processes. It provides a clean API for creating servers, adding tools, and connecting clients. The use of paired pipes for client-server communication is an elegant approach.The implementation aligns well with the PR objectives of simplifying testing of MCP implementations and enabling full request/response cycle testing in a controlled environment.
This allows to omit the initialization code in tests, making them less verbose.
The new name is more descriptive.
|
@ezynda3 Could you help me understand how to move this PR forward? Thank you! |
This PR introduces a new
mcptestpackage that simplifies end-to-end testing of MCP implementations without spawning external processes. The package provides utilities for creating an in-process MCP server and connecting it to a client within the same test, making functional tests more reliable and easier to write.Key features of
mcptestExample usage
👉 Look at
mcptest/mcptest_test.gofor an unabridged exampleImplementation notes
NewStdioMCPClientWithIOfunction that allows creating anStdioMCPClientusing providedio.Readerandio.Writerinterfaces instead of spawning a subprocessThis PR significantly improves testability for MCP implementations and should make writing functional tests much more straightforward.
Summary by CodeRabbit
New Features
Tests