test: add comprehensive unit tests for MCP clients, profiler, and BM25 #10
Conversation
Add dependency injection seams to LocalMCPClient and RemoteMCPClient to enable unit testing without spawning real processes or network calls. Signed-off-by: assagman <[email protected]>
Cover constructor, connect, listTools, callTool, close, and caching behavior with mock factories to isolate transport layer. Signed-off-by: assagman <[email protected]>
Cover mark/measure, record, getStats, initialization tracking, server metrics, indexing metrics, export, reset, and timers. Signed-off-by: assagman <[email protected]>
Cover addTool, removeTool, has, getStats, indexToolsAsync, and addToolsAsync for BM25Index incremental update support. Signed-off-by: assagman <[email protected]>
Allow .opencode directory to be tracked if needed. Signed-off-by: assagman <[email protected]>
Summary of ChangesHello @assagman, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly boosts the robustness and maintainability of the codebase by adding comprehensive unit test coverage across several critical modules. It also introduces architectural improvements to the MCP client implementations, enabling more effective and isolated testing through dependency injection. These changes collectively ensure higher code quality and easier future development. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an excellent pull request that significantly improves the project's test coverage and maintainability. The refactoring of LocalMCPClient and RemoteMCPClient to use dependency injection is a great change that enables robust unit testing, as demonstrated by the new test files. The tests themselves are comprehensive, covering a wide range of success cases, error conditions, and edge cases for the MCP clients, profiler, and BM25 index. I have a few minor suggestions to further improve code clarity and robustness, but overall, this is a high-quality contribution.
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive unit test coverage for core MCP client modules (local and remote), the profiler, and BM25 search functionality. The changes also refactor MCP client code to support dependency injection for improved testability by introducing factory patterns for client and transport creation.
Changes:
- Refactored
LocalMCPClientandRemoteMCPClientto support dependency injection via factory patterns - Added 4 new comprehensive unit test files totaling ~1,340 lines of test code
- Cleaned up
.gitignoreto remove.opencode/plansentry
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/mcp-client/local.ts | Added DI interfaces and factory pattern support for testing |
| src/mcp-client/remote.ts | Added DI interfaces and factory pattern support for testing |
| test/unit/local-client.test.ts | Comprehensive unit tests for LocalMCPClient (339 lines) |
| test/unit/remote-client.test.ts | Comprehensive unit tests for RemoteMCPClient (484 lines) |
| test/unit/profiler.test.ts | Comprehensive unit tests for Profiler class (279 lines) |
| test/unit/bm25.test.ts | Extended tests for BM25 incremental/async operations (242 additional lines) |
| .gitignore | Removed .opencode/plans entry |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add LocalClientLike interface matching RemoteClientLike pattern - Simplify client property type from ReturnType<NonNullable<...>> - Remove unused beforeEach import from local-client.test.ts Addresses review feedback from Gemini and Copilot. Signed-off-by: assagman <[email protected]>
Addressed Review FeedbackCommit 2814561 addresses the following review comments:
|
Signed-off-by: assagman <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Reverts accidental removal of plan file exclusion and adds missing POSIX-compliant newline at end of file. Signed-off-by: assagman <[email protected]>
Summary
Adds comprehensive unit test coverage for core modules and refactors MCP client code to support dependency injection for improved testability.
Changes
local.tsandremote.tsfor mock injectionlocal-client.test.ts: 339 lines covering LocalMCPClientremote-client.test.ts: 484 lines covering RemoteMCPClientprofiler.test.ts: 279 lines covering profiler functionalitybm25.test.ts: 239 lines covering BM25 search + incremental/async ops.opencodefrom gitignoreBreaking Changes
None
Testing
vitest