-
Notifications
You must be signed in to change notification settings - Fork 2
feat: add comprehensive MCP integration tests and testing documentation #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add high-level integration tests for complete MCP workflows including: - End-to-end connector validation workflows (validate -> resolve -> test) - Error handling and edge case scenarios with invalid manifests - Performance testing with multiple rapid tool calls - Concurrent tool execution testing - Authentication configuration testing - Add comprehensive TESTING.md guide with FastMCP 2.0 CLI tools and best practices - Add simple API manifest fixture for additional test scenarios - Update README to reference testing guide - All tests pass (19 passed, 1 skipped) with no regressions - FastMCP CLI tools verified working (inspect command tested successfully) Co-Authored-By: AJ Steers <[email protected]>
Original prompt from AJ Steers
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. Testing This VersionYou can test this version of the MCP Server using the following: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/builder-mcp.git@devin/1754079194-mcp-integration-tests#egg=airbyte-builder-mcp' --help Helpful Resources
PR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds comprehensive MCP integration tests and detailed testing documentation to improve the robustness and testability of the Builder MCP server. The changes introduce high-level workflow tests, error handling validation, performance testing, and a complete testing guide using FastMCP 2.0 best practices.
- Adds 8 new integration tests covering complete MCP workflows, error handling, and concurrency scenarios
- Introduces comprehensive testing documentation with FastMCP CLI usage examples and debugging guides
- Provides additional test fixtures for broader coverage of API manifest scenarios
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
tests/test_integration.py | Adds high-level integration tests, error handling tests, and performance/concurrency testing |
tests/resources/simple_api_manifest.yaml | New test fixture providing a simple API manifest for testing workflows |
TESTING.md | Comprehensive testing guide with FastMCP CLI tools, debugging practices, and testing patterns |
README.md | Adds reference to the new testing documentation |
Comments suppressed due to low confidence (1)
tests/test_integration.py:173
- This test only verifies that errors is a list but doesn't validate the actual behavior of authentication config validation. The test should assert meaningful validation results rather than just checking the type.
assert isinstance(result.errors, list) # Should return a proper validation result
PyTest Results (Full)0 tests 0 ✅ 0s ⏱️ Results for commit 727f2d4. ♻️ This comment has been updated with latest results. |
- Fix unused loop variable 'i' -> '_' in performance test - Remove unused 'expected_tools' variable in server discovery test - Apply ruff auto-formatting fixes for import ordering and whitespace - All ruff checks now pass locally Co-Authored-By: AJ Steers <[email protected]>
/poe lock
❌ Poe command |
- Cast validation error to string to prevent AttributeError - Reduce performance test timeout from 30s to 15s for better issue detection Co-Authored-By: AJ Steers <[email protected]>
I updated from main to resolve the unrelated ci failure. |
- Add poe mcp-serve-local, mcp-serve-http, mcp-serve-sse commands - Remove duplicate poe task definitions from pyproject.toml - Update TESTING.md to use correct poe commands (test vs pytest) - Audit test_integration.py: remove signal suppression, add parametrization, create helpers - Address Aaron's GitHub PR comments for better MCP testing workflow Co-Authored-By: AJ Steers <[email protected]>
- Fix whitespace in blank lines in test_integration.py - Address CI failures for Ruff Format Check and Ruff Lint Check Co-Authored-By: AJ Steers <[email protected]>
- Clarify that fastmcp inspect generates comprehensive JSON reports - Remove references to non-existent --tools and --health flags - Add examples for using --output argument passthrough - Add section on testing specific tools using jq to parse JSON output - Verify poe inspect command works correctly with argument passthrough Co-Authored-By: AJ Steers <[email protected]>
feat: add comprehensive MCP integration tests and testing documentation
Summary
This PR adds comprehensive high-level MCP integration tests and detailed testing documentation using FastMCP 2.0 best practices and CLI tools, as requested by @aaronsteers.
Key additions:
TESTING.md
) with FastMCP CLI tools usageTest coverage expanded from 12 to 20 tests with all tests passing (19 passed, 1 skipped) and no regressions.
Review & Testing Checklist for Human
uv run fastmcp inspect builder_mcp/server.py:app
to ensure CLI tools function correctlyuv run pytest tests/ -v
Recommended test plan:
uv run pytest tests/ -v
uv run fastmcp inspect builder_mcp/server.py:app
uv run builder-mcp
(Ctrl+C to stop)Diagram
Notes
fastmcp inspect
command tested successfully)@pytest.mark.requires_creds
to avoid running in fast CILink to Devin session: https://app.devin.ai/sessions/b5cab87e17364e3faf2cc329cd2455d3
Requested by: @aaronsteers