Commit 163cd0d
test(plugins): assert error propagation in plugin error fallback tests
Merge #6309
Three plugin "fallback to runner" tests are meant to verify that when the
`on_*_error` callback is disabled, the underlying error is not swallowed and
propagates out through the runner. As written, they do not actually enforce that:
- `test_async_on_tool_error_fallback_to_runner` and
`test_live_on_tool_error_fallback_to_runner`
(`tests/unittests/flows/llm_flows/test_plugin_tool_callbacks.py`) are shaped as:
```python
try:
await invoke_tool_with_plugin(mock_error_tool, mock_plugin)
except Exception as e:
assert e == mock_error
```
The only assertion lives inside the `except`. If the production code ever stops
raising (exactly the regression these tests exist to catch), the `try` body
completes normally, the `except` never runs, the assertion is skipped, and the
test still passes green.
- `test_on_model_error_callback_fallback_to_runner`
(`tests/unittests/flows/llm_flows/test_plugin_model_callbacks.py`) is worse: its
`try` only constructs `InMemoryRunner(agent, plugins=[...])`, which never runs the
agent, so no error path is exercised at all and the test can never fail.
All three pass today over correct code, so this is latent false safety, not a live
bug: they would not catch the swallow-the-error regression they are named for.
**Solution:**
Rewrite the three test bodies so a no-raise regression fails them, following
conventions already used in this repo. No production code changes.
- Tool tests: wrap the call in `pytest.raises(ClientError)` and assert the captured
error, so the test fails if nothing is raised:
```python
with pytest.raises(ClientError) as exc_info:
await invoke_tool_with_plugin(mock_error_tool, mock_plugin)
assert exc_info.value == mock_error
```
- Model test: actually run the agent, then assert the unhandled model error surfaces
as a terminal error event. On the synchronous runner an unhandled model error is
converted into an error event rather than raised to the caller, so `pytest.raises`
is not the right shape here; this mirrors the existing error-event assertions in
`tests/unittests/flows/llm_flows/test_base_llm_flow.py`:
```python
runner = testing_utils.InMemoryRunner(agent, plugins=[mock_plugin])
events = runner.run('test')
error_events = [e for e in events if e.error_code]
assert len(error_events) == 1
assert error_events[0].error_code == 'ClientError'
```
Closes #6309
Co-authored-by: Jason Zhang <jasoncz@google.com>
PiperOrigin-RevId: 9478616081 parent 7c11f58 commit 163cd0d
2 files changed
Lines changed: 11 additions & 11 deletions
Lines changed: 7 additions & 5 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
171 | 171 | | |
172 | 172 | | |
173 | 173 | | |
174 | | - | |
| 174 | + | |
175 | 175 | | |
176 | 176 | | |
177 | 177 | | |
178 | 178 | | |
179 | 179 | | |
180 | 180 | | |
181 | 181 | | |
182 | | - | |
183 | | - | |
184 | | - | |
185 | | - | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
186 | 188 | | |
187 | 189 | | |
188 | 190 | | |
| |||
Lines changed: 4 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
180 | 180 | | |
181 | 181 | | |
182 | 182 | | |
183 | | - | |
| 183 | + | |
184 | 184 | | |
185 | | - | |
186 | | - | |
| 185 | + | |
187 | 186 | | |
188 | 187 | | |
189 | 188 | | |
| |||
258 | 257 | | |
259 | 258 | | |
260 | 259 | | |
261 | | - | |
| 260 | + | |
262 | 261 | | |
263 | | - | |
264 | | - | |
| 262 | + | |
265 | 263 | | |
266 | 264 | | |
267 | 265 | | |
| |||
0 commit comments