-
Notifications
You must be signed in to change notification settings - Fork 67
Description
Fix Misleading Integration Tests
Problem Description
The current integration tests are misleading because they return mock data regardless of whether the actual functionality works. This gives false confidence about the implementation and allowed bugs to go undetected.
Current Behavior
In tests/helpers/server_testing.py (lines 281-288):
# For other resources, return mock data
return {
"result": {
"contents": [
{"uri": uri, "mimeType": "text/plain", "text": f"Mock data for {uri}"}
]
}
}In tests/test_integration_e2e.py (line 277):
assert "Mock data" in text # Our mock implementation returns thisProblem: Tests always pass because they check for mock data, not actual functionality.
Expected Behavior
Integration tests should:
- Call actual resource handlers
- Verify real data is returned
- Test error conditions with actual error messages
- Validate the complete request/response cycle
Impact
This issue has hidden real bugs:
- Resource URI parsing bug went undetected
- Tests showed resources working when they actually failed
- False confidence in implementation completeness
Before Implementation
-
Analyze Current Test Structure
- Review all test files in
tests/directory - Understand test categories (unit, integration, e2e)
- Check which tests actually use mocks vs real calls
- Identify the purpose of existing mocks
- Review all test files in
-
Validate Mock Usage
- Determine if mocks serve legitimate purposes
- Check if real Odoo server is available during tests
- Understand test isolation requirements
- Review CI/CD test environment constraints
-
Assess Test Performance
- Measure current test execution time
- Check if real API calls would slow tests significantly
- Consider test reliability with external dependencies
- Evaluate impact on development workflow
Proposed Investigation & Solution
1. Analysis Phase
INVESTIGATION STEPS:
1. Map all test files and their mock usage
2. Identify critical vs convenience mocks
3. Test current mock behavior vs real behavior
4. Determine optimal testing strategy
2. Potential Solution Approach
CONCEPTUAL APPROACH (verify implementation first):
1. Replace misleading mocks with real handler calls
2. Keep performance/isolation mocks where appropriate
3. Add integration tests with real Odoo server
4. Ensure proper test data setup/teardown
# In server_testing.py
async def read_resource(self, uri: str) -> ResourceResponse:
"""Read a resource using actual handlers."""
try:
# Parse URI
parsed_uri = parse_uri(uri)
# Call actual resource handler based on operation
if parsed_uri.operation == OdooOperation.RECORD:
content = await self.resources._handle_record_retrieval(
parsed_uri.model,
str(parsed_uri.record_id)
)
elif parsed_uri.operation == OdooOperation.SEARCH:
content = await self.resources._handle_search(
parsed_uri.model,
parsed_uri.domain,
parsed_uri.fields,
parsed_uri.limit,
parsed_uri.offset,
parsed_uri.order
)
# ... handle other operations
return ResourceResponse(
contents=[
ResourceContent(
uri=uri,
mimeType="text/plain",
text=content
)
]
)
except Exception as e:
raise McpError(str(e))2. Update Test Assertions
Replace mock data checks with real data validation:
# Instead of:
assert "Mock data" in text
# Use:
assert "res.partner" in text
assert "name" in text
assert "email" in text
# Or check for actual record data3. Add Proper Test Fixtures
Create test data that can be verified:
@pytest.fixture
async def test_partner(odoo_connection):
"""Create a test partner for resource tests."""
partner_id = await odoo_connection.create("res.partner", {
"name": "Test Integration Partner",
"email": "[email protected]",
"phone": "+1-555-0001"
})
yield partner_id
# Cleanup
await odoo_connection.unlink("res.partner", [partner_id])
async def test_resource_record_retrieval(mcp_server, test_partner):
"""Test actual record retrieval through resources."""
uri = f"odoo://res.partner/record/{test_partner}"
response = await mcp_server.read_resource(uri)
text = response.contents[0].text
# Verify actual data
assert "Test Integration Partner" in text
assert "[email protected]" in text
assert "+1-555-0001" in text4. Test Error Conditions
Add tests for real error scenarios:
async def test_resource_not_found(mcp_server):
"""Test resource not found error."""
uri = "odoo://res.partner/record/999999"
with pytest.raises(McpError) as exc_info:
await mcp_server.read_resource(uri)
assert "not found" in str(exc_info.value).lower()
# NOT "Mock data" or generic errors
async def test_resource_invalid_operation(mcp_server):
"""Test invalid operation error."""
uri = "odoo://res.partner/invalid_op"
with pytest.raises(McpError) as exc_info:
await mcp_server.read_resource(uri)
assert "unknown operation" in str(exc_info.value).lower()5. Add Protocol-Level Tests
Test through the actual MCP protocol:
async def test_mcp_protocol_resources(mcp_client):
"""Test resources through MCP client protocol."""
# List resources
resources = await mcp_client.list_resources()
# Should be empty due to FastMCP limitation
assert len(resources) == 0
# But resource templates should work
result = await mcp_client.call_tool("list_resource_templates", {})
assert len(result["templates"]) == 5
# Try to read a resource
with pytest.raises(McpError) as exc_info:
await mcp_client.read_resource("odoo://res.partner/record/10")
# Should fail with actual error, not mock success
assert "unknown resource" in str(exc_info.value).lower()Test Categories to Fix
- Unit Tests - These are fine, they test individual components
- Integration Tests - Need to use real handlers, not mocks
- E2E Tests - Need to test actual MCP protocol flow
- Resource Tests - Must test actual resource operations
Success Criteria
- No mock data in integration tests
- Tests call actual resource handlers
- Tests verify real Odoo data
- Error tests check actual error messages
- All tests that currently pass due to mocks are fixed
- New bugs are caught by updated tests
- Test coverage remains >90%
Additional Testing Improvements
-
Add performance benchmarks
@pytest.mark.benchmark async def test_resource_performance(benchmark, mcp_server): result = await benchmark( mcp_server.read_resource, "odoo://res.partner/record/10" ) assert benchmark.stats['mean'] < 0.1 # 100ms
-
Add contract tests
- Verify response format matches MCP spec
- Validate all required fields are present
-
Add stress tests
- Multiple concurrent resource requests
- Large data sets
- Token limit validation
Risk
Without fixing these tests, future bugs may go undetected, leading to production issues and poor user experience.