Skip to content

Commit 3f229a2

Browse files
kingpanther13claude
andcommitted
docs(tests): correct the e2e expected-failure helper guidance
tests/AGENTS.md told tests expecting a tool failure to use safe_call_tool() directly. The harness disagrees: MCPAssertions .call_tool_failure() is used at 61 call sites across 13 e2e files and does strictly more -- it catches the ToolError, asserts the call actually failed rather than silently succeeding, and matches expected_error against the extracted message. safe_call_tool() swallows ToolError and returns a parsed dict, so using it for an expected failure leaves nothing verifying the call failed. Its real roles are finally-block cleanup and availability probing, which the entry now says. CodeRabbit cites the old line as a coding-guidelines violation whenever call_tool_failure appears, most recently on #2246. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015c4gkE9mcKu4RiuQzR5eff
1 parent 45824cd commit 3f229a2

1 file changed

Lines changed: 8 additions & 2 deletions

File tree

tests/AGENTS.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,14 @@ rather than assume the e2e's always-present case.
4949
## Test Patterns
5050

5151
- Tests expecting tool **success**: use `mcp.call_tool_success()` inside `MCPAssertions` context
52-
- Tests expecting tool **failure**: use `safe_call_tool()` directly (catches `ToolError`, returns parsed dict)
53-
- Service availability checks should use `safe_call_tool` to probe, not `call_tool_success`
52+
- Tests expecting tool **failure**: use `mcp.call_tool_failure()` inside `MCPAssertions` context.
53+
It catches the `ToolError`, asserts the call actually failed rather than silently
54+
succeeding, and — when `expected_error` is given — matches it against the extracted
55+
error message.
56+
- `safe_call_tool()` is for calls whose outcome the test does **not** assert: `finally`
57+
cleanup (so a cleanup failure cannot mask the real assertion) and service-availability
58+
probes. It swallows `ToolError` and returns a parsed dict, so using it for an expected
59+
failure means nothing verifies the call failed at all.
5460

5561
## E2E Test Patterns
5662

0 commit comments

Comments
 (0)