Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions tests/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,17 @@ rather than assume the e2e's always-present case.
## Test Patterns

- Tests expecting tool **success**: use `mcp.call_tool_success()` inside `MCPAssertions` context
- Tests expecting tool **failure**: use `safe_call_tool()` directly (catches `ToolError`, returns parsed dict)
- Service availability checks should use `safe_call_tool` to probe, not `call_tool_success`
- Tests expecting tool **failure**: use `mcp.call_tool_failure()` inside `MCPAssertions`
context. Prefer passing `expected_error` — it is what turns the call into a real
assertion, matching that substring against the extracted error message. The bare
success check is `if data.get("success")`, so a result dict that omits the key is
accepted as a failure on its own. About half the current call sites omit
`expected_error` and assert on the returned dict themselves instead; that is equally
fine. Omitting both is what leaves the failure unverified.
Comment thread
kingpanther13 marked this conversation as resolved.
Outdated
- `safe_call_tool()` is for calls whose outcome the test does **not** assert: `finally`
cleanup (so a cleanup failure cannot mask the real assertion) and service-availability
probes. It swallows `ToolError` and returns a parsed dict, so using it for an expected
failure means nothing verifies the call failed at all.
Comment thread
kingpanther13 marked this conversation as resolved.

## E2E Test Patterns

Expand Down
7 changes: 5 additions & 2 deletions tests/src/e2e/utilities/assertions.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,11 @@ async def safe_call_tool(
) -> dict[str, Any]:
"""Call an MCP tool and return parsed result, handling ToolError exceptions.

This is useful for tests that expect tools to fail and want to inspect
the error response without catching exceptions manually.
For a call whose outcome the test does NOT assert: ``finally``-block cleanup
(so a cleanup failure cannot mask the real assertion) and service-availability
probes. It swallows ``ToolError`` and returns a parsed dict either way, so a
test that asserts a failure should use ``MCPAssertions.call_tool_failure()``
with ``expected_error`` instead -- see tests/AGENTS.md "Test Patterns".

Args:
mcp_client: The MCP client instance
Expand Down
Loading