DA-1055 investigate automated testing for tools mcp server #86
Conversation
Summary of ChangesHello @AayushTyagi1, 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 enhances the reliability and maintainability of the Couchbase MCP server by implementing a new set of automated integration tests. These tests are designed to validate the server's core functionalities and the behavior of its exposed tools against a live Couchbase cluster, providing a critical layer of quality assurance for 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 pull request introduces a comprehensive suite of integration tests for the MCP server, which is an excellent addition for ensuring the stability and correctness of the provided tools. The tests cover various tool categories, including KV, query, index, and server operations. The test setup in conftest.py is well-structured, and the tests are clear, covering important use cases and including cleanup for created test data. I have a couple of suggestions to improve the testing instructions and make one of the tests more robust.
There was a problem hiding this comment.
Pull request overview
This PR introduces comprehensive automated testing infrastructure for the MCP server, including unit tests for utility modules and integration tests for all server tools (KV operations, queries, indexes, and server management).
Key Changes
- Added pytest-based test suite with both unit and integration tests
- Implemented test fixtures and utilities for MCP session management
- Added testing configuration to pyproject.toml and documentation to README
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/conftest.py | Shared test fixtures, MCP session management, and configuration constants |
| tests/test_utils.py | Unit tests for utility functions in index_utils, constants, config, connection, and context modules |
| tests/test_mcp_integration.py | Integration tests for tool registration, descriptions, schemas, and basic server connectivity |
| tests/test_server_tools.py | Integration tests for server management tools (buckets, scopes, collections, health checks) |
| tests/test_kv_tools.py | Integration tests for key-value operations (get, upsert, delete documents) |
| tests/test_query_tools.py | Integration tests for query tools (schema inference, SQL++ queries) |
| tests/test_index_tools.py | Integration tests for index management tools (list indexes, advisor recommendations) |
| pyproject.toml | Added pytest dependencies and asyncio configuration |
| README.md | Added integration testing documentation section |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mock_ctx.request_context.lifespan_context.cluster = mock_cluster | ||
| return mock_cluster | ||
|
|
||
| with patch( | ||
| "utils.context.connect_to_couchbase_cluster", | ||
| side_effect=set_cluster_side_effect, | ||
| ): | ||
| # Since cluster is None, it will try to connect | ||
| # The function sets the cluster and then returns it | ||
| # We need to adjust the mock behavior | ||
| pass | ||
|
|
||
| # Test the path where cluster already exists | ||
| mock_ctx.request_context.lifespan_context.cluster = mock_cluster | ||
| result = get_cluster_connection(mock_ctx) |
There was a problem hiding this comment.
This test function sets up mocks but doesn't actually invoke get_cluster_connection to test the path where the cluster doesn't exist initially (lines 505-516 are setup but pass does nothing). The test only verifies the path where cluster already exists (lines 519-521). Consider calling get_cluster_connection(mock_ctx) before line 518 to test the connection creation path, or remove the unused setup code.
Testing