test: Implemented tests for the MCP server lifecycle
This commit is contained in:
@@ -0,0 +1,129 @@
|
||||
# Iteration 5 — Test Implementation Notes
|
||||
|
||||
## Plan file addressed
|
||||
|
||||
`docs/testing/plans/05-mcp-lifecycle.md`
|
||||
|
||||
## Tests created
|
||||
|
||||
### src/config/mcp_factory.rs (12 new tests)
|
||||
|
||||
| Test name | What it verifies |
|
||||
|---|---|
|
||||
| `key_from_stdio_spec_captures_command_args_env` | McpServerKey extracts command, args, env from stdio spec |
|
||||
| `key_from_stdio_spec_sorts_args_and_env` | Args and env are sorted for deterministic key hashing |
|
||||
| `key_from_stdio_spec_defaults_empty_when_none` | None args/env default to empty vecs |
|
||||
| `key_from_remote_http_spec` | Http transport key captures url and transport type |
|
||||
| `key_from_remote_sse_spec_with_sorted_headers` | SSE headers sorted for deterministic keys |
|
||||
| `key_equality_same_spec_produces_equal_keys` | Same spec → equal keys (sharing contract) |
|
||||
| `key_inequality_different_names` | Different server names → different keys |
|
||||
| `key_inequality_different_commands` | Different commands → different keys (isolation contract) |
|
||||
| `key_env_bool_and_int_coerce_to_string` | JsonField::Bool/Int coerced to String in key |
|
||||
| `factory_try_get_active_returns_none_when_empty` | Empty factory returns None |
|
||||
| `factory_try_get_active_returns_none_for_unknown_key` | Unknown key returns None |
|
||||
| `factory_default_has_empty_active_map` | Default factory has empty internal map |
|
||||
|
||||
### src/config/tool_scope.rs (6 new tests)
|
||||
|
||||
| Test name | What it verifies |
|
||||
|---|---|
|
||||
| `mcp_runtime_new_is_empty` | New McpRuntime has no servers |
|
||||
| `mcp_runtime_default_is_empty` | Default McpRuntime is empty |
|
||||
| `mcp_runtime_get_returns_none_for_missing_server` | get() on nonexistent server returns None |
|
||||
| `tool_scope_default_has_empty_mcp_runtime` | Default ToolScope has empty MCP runtime |
|
||||
| `tool_scope_default_has_empty_functions` | Default ToolScope has no functions |
|
||||
| `tool_scope_default_tracker_has_no_loops` | Default ToolScope tracker detects no loops |
|
||||
|
||||
### src/mcp/mod.rs (30 new tests)
|
||||
|
||||
| Test name | What it verifies |
|
||||
|---|---|
|
||||
| `validate_stdio_with_command_succeeds` | Valid stdio spec passes |
|
||||
| `validate_stdio_missing_command_fails` | Stdio without command is rejected |
|
||||
| `validate_stdio_with_url_fails` | Stdio with url (remote field) is rejected |
|
||||
| `validate_stdio_with_headers_fails` | Stdio with headers (remote field) is rejected |
|
||||
| `validate_http_with_url_succeeds` | Valid http spec passes |
|
||||
| `validate_http_missing_url_fails` | Http without url is rejected |
|
||||
| `validate_http_with_command_fails` | Http with command (stdio field) is rejected |
|
||||
| `validate_http_with_args_fails` | Http with args (stdio field) is rejected |
|
||||
| `validate_http_with_cwd_fails` | Http with cwd (stdio field) is rejected |
|
||||
| `validate_sse_with_url_succeeds` | Valid SSE spec passes |
|
||||
| `validate_sse_missing_url_fails` | SSE without url is rejected |
|
||||
| `is_remote_true_for_http_and_sse` | Http and SSE are remote transports |
|
||||
| `is_remote_false_for_stdio` | Stdio is not remote |
|
||||
| `deserialize_stdio_server_from_json` | Full stdio spec from JSON |
|
||||
| `deserialize_http_server_from_json` | Http spec with headers from JSON |
|
||||
| `deserialize_env_with_mixed_types` | Env with String, Bool, Int values |
|
||||
| `deserialize_multiple_servers` | Multiple server entries parsed |
|
||||
| `deserialize_empty_servers_map` | Empty mcpServers map parsed |
|
||||
| `deserialize_server_with_cwd` | cwd field parsed correctly |
|
||||
| `resolve_all_returns_all_configured_servers` | "all" resolves to all config keys |
|
||||
| `resolve_comma_separated_returns_matching_servers` | Comma-separated list filters correctly |
|
||||
| `resolve_single_server_name` | Single name resolved |
|
||||
| `resolve_none_returns_empty` | None enabled → empty list |
|
||||
| `resolve_no_config_returns_empty` | No config → empty list |
|
||||
| `resolve_nonexistent_server_filtered_out` | Unknown names silently filtered |
|
||||
| `resolve_all_nonexistent_returns_empty` | All unknown → empty list |
|
||||
| `resolve_trims_whitespace` | Whitespace in comma list trimmed |
|
||||
| `registry_default_is_empty` | Default registry: empty, no config, no log |
|
||||
| `registry_with_config_reports_config` | Config accessor works |
|
||||
| `meta_function_prefixes_are_correct` | mcp_invoke/search/describe prefixes |
|
||||
|
||||
### src/config/request_context.rs (6 new tests)
|
||||
|
||||
| Test name | What it verifies |
|
||||
|---|---|
|
||||
| `rebuild_tool_scope_mcp_disabled_skips_servers` | mcp_server_support=false → empty runtime |
|
||||
| `rebuild_tool_scope_no_enabled_servers_yields_empty_runtime` | None enabled → empty runtime |
|
||||
| `rebuild_tool_scope_no_mcp_config_yields_empty_runtime` | No mcp_config → empty runtime |
|
||||
| `rebuild_tool_scope_preserves_tool_tracker` | Tracker survives rebuild |
|
||||
| `rebuild_tool_scope_repl_mode_appends_user_interaction_functions` | REPL adds user__ functions |
|
||||
| `rebuild_tool_scope_cmd_mode_no_user_interaction_functions` | CMD skips user__ functions |
|
||||
|
||||
**Total: 54 new tests (176 total in suite)**
|
||||
|
||||
## Bugs discovered
|
||||
|
||||
None.
|
||||
|
||||
## Observations for future iterations
|
||||
|
||||
1. **ConnectedServer untestable without subprocess**: `ConnectedServer`
|
||||
(= `RunningService<RoleClient, ()>`) cannot be constructed without
|
||||
a real MCP server subprocess. This blocks unit testing for:
|
||||
- McpFactory.acquire() full flow (spawn + insert + Weak sharing)
|
||||
- McpRuntime.insert/get with real handles
|
||||
- McpRuntime.search/describe/invoke (need live tool catalog)
|
||||
- All scope transition tests (role/session/agent MCP start/stop)
|
||||
|
||||
These require integration tests with a mock MCP server binary
|
||||
(e.g., a simple echo server). Recommended for a dedicated
|
||||
integration test iteration.
|
||||
|
||||
2. **McpServerKey sorting guarantees sharing correctness**: The
|
||||
sorting of args, env, and headers in McpServerKey::from_spec
|
||||
is critical — without it, HashMap key equality would be
|
||||
non-deterministic. Tests verify this explicitly.
|
||||
|
||||
3. **rebuild_tool_scope has 3 guard clauses that prevent server
|
||||
acquisition**: mcp_server_support=false, mcp_config=None,
|
||||
enabled_mcp_servers=None. All three paths tested.
|
||||
|
||||
4. **REPL vs CMD mode differs in user interaction functions**: The
|
||||
`rebuild_tool_scope` method conditionally appends `user__*`
|
||||
functions only in REPL mode. Tested both paths.
|
||||
|
||||
5. **McpServer::validate enforces strict transport/field separation**:
|
||||
Stdio servers cannot have url/headers, remote servers cannot have
|
||||
command/args/cwd. This prevents misconfiguration. All cross-field
|
||||
conflict cases tested.
|
||||
|
||||
6. **McpRegistry.resolve_server_ids is private** but tested via
|
||||
`#[cfg(test)]` in the same module. It's the core of server ID
|
||||
resolution for "all", comma-separated, and empty cases.
|
||||
|
||||
## Next iteration
|
||||
|
||||
Plan file 06: Tool Evaluation — eval_tool_calls, ToolCall dispatch,
|
||||
tool handlers, MCP tool invocation chain (mcp__search, mcp__describe,
|
||||
mcp__invoke).
|
||||
Reference in New Issue
Block a user