jpmschweitzerandClaude
eb3467d06a
97 findings to zero. Most were mechanical — 52 unsorted import blocks, 10 unsorted __all__, assorted pyupgrade and simplify hints. Two were not, and both were visible only because the lint made me look. `webber version` did not exist. src/cli/commands/version.py defines show_version(), main.py imported it, and the registration line was never written — the CLI exposed chat and explore only. The import carried `# noqa: F401`, which is what kept the omission quiet: someone marked the symptom as intentional instead of asking why it was unused. show_version is not redundant with the --version flag; it prints the resolved Ollama URL, model and debug state, which is the form worth having when something is misconfigured. Registered, and the suppression dropped because the import is now genuinely used. test_spawn_explore_agent asserted nothing. It built a mock RunContext, patched get_agent, and stopped at the comment "For now, verify the explore agent would be called correctly". It had been counted as a passing test. An AST sweep of all 238 test functions found it was the only one, which is worth knowing — the problem was contained, not systemic. It is now skipped with a reason, so it reports as unfinished rather than as passing. Reducing it rather than deleting its imports was the point: tidying the imports would have made a hollow test look clean. Two findings were false positives, and both are recorded rather than silently worked around: B023 flagged run_agent closing over full_prompt and ctx. Traced: agent_task is awaited at line 326 before `continue` reaches the next iteration, so neither name can be rebound while the closure is pending, and the exception path cancels and awaits too. Not a bug. Bound as defaults anyway, because that stays true if the await ever moves. I had called it a live bug before tracing it, which is the mistake Rule 5 exists for. RUF012 flagged `rules: list[ApprovalRule] = []` on ApprovalRuleSet. Its suggested fix — annotate ClassVar — would remove the field from the model. ApprovalRuleSet is a pydantic model and pydantic deep-copies defaults per instance; verified by constructing two and confirming their lists are distinct objects. Suppressed with that evidence in the comment. Ruff cannot see the pydantic base because BaseSchema is a local subclass of BaseModel. Also moved a stray `from src.shared.logging import ...` that had drifted below a function definition, and merged a nested if in the ollama provider. 215 passed, 23 skipped, unchanged except for the new skip. `webber version` exercised end to end. mypy is NOT addressed here and the gate still fails on it — 55 errors in 14 files, 35 of them no-any-return from pydantic_ai's untyped returns. That was hidden behind ruff, because the gate stops at the first failing stage. Co-Authored-By: Claude <noreply@anthropic.com>
244 lines
7.7 KiB
Python
244 lines
7.7 KiB
Python
"""
|
|
Tests for retry utilities.
|
|
"""
|
|
|
|
import httpx
|
|
import pytest
|
|
|
|
from src.shared.retry import (
|
|
calculate_backoff,
|
|
is_retryable_exception,
|
|
is_retryable_http_status,
|
|
retry_async,
|
|
with_retry,
|
|
)
|
|
|
|
|
|
class TestIsRetryableHttpStatus:
|
|
"""Tests for HTTP status code checking."""
|
|
|
|
def test_429_is_retryable(self):
|
|
"""429 Too Many Requests should be retryable."""
|
|
assert is_retryable_http_status(429) is True
|
|
|
|
def test_500_is_retryable(self):
|
|
"""500 Internal Server Error should be retryable."""
|
|
assert is_retryable_http_status(500) is True
|
|
|
|
def test_502_is_retryable(self):
|
|
"""502 Bad Gateway should be retryable."""
|
|
assert is_retryable_http_status(502) is True
|
|
|
|
def test_503_is_retryable(self):
|
|
"""503 Service Unavailable should be retryable."""
|
|
assert is_retryable_http_status(503) is True
|
|
|
|
def test_504_is_retryable(self):
|
|
"""504 Gateway Timeout should be retryable."""
|
|
assert is_retryable_http_status(504) is True
|
|
|
|
def test_400_not_retryable(self):
|
|
"""400 Bad Request should not be retryable."""
|
|
assert is_retryable_http_status(400) is False
|
|
|
|
def test_401_not_retryable(self):
|
|
"""401 Unauthorized should not be retryable."""
|
|
assert is_retryable_http_status(401) is False
|
|
|
|
def test_404_not_retryable(self):
|
|
"""404 Not Found should not be retryable."""
|
|
assert is_retryable_http_status(404) is False
|
|
|
|
def test_200_not_retryable(self):
|
|
"""200 OK should not be retryable."""
|
|
assert is_retryable_http_status(200) is False
|
|
|
|
|
|
class TestIsRetryableException:
|
|
"""Tests for exception checking."""
|
|
|
|
def test_timeout_exception_is_retryable(self):
|
|
"""Timeout exceptions should be retryable."""
|
|
exc = httpx.TimeoutException("timeout")
|
|
assert is_retryable_exception(exc) is True
|
|
|
|
def test_connect_error_is_retryable(self):
|
|
"""Connection errors should be retryable."""
|
|
exc = httpx.ConnectError("connection failed")
|
|
assert is_retryable_exception(exc) is True
|
|
|
|
def test_connection_error_is_retryable(self):
|
|
"""Python ConnectionError should be retryable."""
|
|
exc = ConnectionError("connection refused")
|
|
assert is_retryable_exception(exc) is True
|
|
|
|
def test_timeout_error_is_retryable(self):
|
|
"""Python TimeoutError should be retryable."""
|
|
exc = TimeoutError("timed out")
|
|
assert is_retryable_exception(exc) is True
|
|
|
|
def test_value_error_not_retryable(self):
|
|
"""ValueError should not be retryable."""
|
|
exc = ValueError("invalid value")
|
|
assert is_retryable_exception(exc) is False
|
|
|
|
def test_key_error_not_retryable(self):
|
|
"""KeyError should not be retryable."""
|
|
exc = KeyError("missing key")
|
|
assert is_retryable_exception(exc) is False
|
|
|
|
|
|
class TestCalculateBackoff:
|
|
"""Tests for backoff calculation."""
|
|
|
|
def test_first_attempt_base_delay(self):
|
|
"""First attempt should use base delay."""
|
|
delay = calculate_backoff(0, base_delay=1.0, jitter=False)
|
|
assert delay == 1.0
|
|
|
|
def test_second_attempt_doubles(self):
|
|
"""Second attempt should double the delay."""
|
|
delay = calculate_backoff(1, base_delay=1.0, jitter=False)
|
|
assert delay == 2.0
|
|
|
|
def test_third_attempt_quadruples(self):
|
|
"""Third attempt should quadruple the delay."""
|
|
delay = calculate_backoff(2, base_delay=1.0, jitter=False)
|
|
assert delay == 4.0
|
|
|
|
def test_max_delay_respected(self):
|
|
"""Delay should not exceed max_delay."""
|
|
delay = calculate_backoff(10, base_delay=1.0, max_delay=30.0, jitter=False)
|
|
assert delay == 30.0
|
|
|
|
def test_jitter_adds_randomness(self):
|
|
"""Jitter should add randomness to delay."""
|
|
delays = [calculate_backoff(1, base_delay=1.0, jitter=True) for _ in range(10)]
|
|
# With jitter, delays should vary (not all identical)
|
|
assert len(set(delays)) > 1
|
|
|
|
def test_jitter_within_bounds(self):
|
|
"""Jitter should keep delay within reasonable bounds."""
|
|
for _ in range(100):
|
|
delay = calculate_backoff(0, base_delay=2.0, jitter=True)
|
|
# Attempt 0 with base 2.0 = 2.0, with jitter should be 0.75-1.25x = 1.5-2.5
|
|
assert 1.5 <= delay <= 2.5
|
|
|
|
|
|
class TestWithRetryDecorator:
|
|
"""Tests for the @with_retry decorator."""
|
|
|
|
@pytest.mark.anyio
|
|
async def test_success_on_first_attempt(self):
|
|
"""Function should return on first successful attempt."""
|
|
call_count = 0
|
|
|
|
@with_retry(max_attempts=3)
|
|
async def successful_func():
|
|
nonlocal call_count
|
|
call_count += 1
|
|
return "success"
|
|
|
|
result = await successful_func()
|
|
assert result == "success"
|
|
assert call_count == 1
|
|
|
|
@pytest.mark.anyio
|
|
async def test_retry_on_timeout(self):
|
|
"""Should retry on timeout exception."""
|
|
call_count = 0
|
|
|
|
@with_retry(max_attempts=3, base_delay=0.01)
|
|
async def flaky_func():
|
|
nonlocal call_count
|
|
call_count += 1
|
|
if call_count < 3:
|
|
raise httpx.TimeoutException("timeout")
|
|
return "success"
|
|
|
|
result = await flaky_func()
|
|
assert result == "success"
|
|
assert call_count == 3
|
|
|
|
@pytest.mark.anyio
|
|
async def test_no_retry_on_value_error(self):
|
|
"""Should not retry on non-retryable exceptions."""
|
|
call_count = 0
|
|
|
|
@with_retry(max_attempts=3)
|
|
async def bad_func():
|
|
nonlocal call_count
|
|
call_count += 1
|
|
raise ValueError("bad value")
|
|
|
|
with pytest.raises(ValueError):
|
|
await bad_func()
|
|
assert call_count == 1
|
|
|
|
@pytest.mark.anyio
|
|
async def test_exhausted_retries(self):
|
|
"""Should raise last exception after all retries exhausted."""
|
|
call_count = 0
|
|
|
|
@with_retry(max_attempts=3, base_delay=0.01)
|
|
async def always_fails():
|
|
nonlocal call_count
|
|
call_count += 1
|
|
raise httpx.TimeoutException("always times out")
|
|
|
|
with pytest.raises(httpx.TimeoutException):
|
|
await always_fails()
|
|
assert call_count == 3
|
|
|
|
|
|
class TestRetryAsync:
|
|
"""Tests for the retry_async function."""
|
|
|
|
@pytest.mark.anyio
|
|
async def test_success_on_first_attempt(self):
|
|
"""Function should return on first successful attempt."""
|
|
async def successful_func():
|
|
return "success"
|
|
|
|
result = await retry_async(successful_func, max_attempts=3)
|
|
assert result == "success"
|
|
|
|
@pytest.mark.anyio
|
|
async def test_retry_on_connect_error(self):
|
|
"""Should retry on connection errors."""
|
|
call_count = 0
|
|
|
|
async def flaky_func():
|
|
nonlocal call_count
|
|
call_count += 1
|
|
if call_count < 2:
|
|
raise httpx.ConnectError("connection failed")
|
|
return "success"
|
|
|
|
result = await retry_async(flaky_func, max_attempts=3, base_delay=0.01)
|
|
assert result == "success"
|
|
assert call_count == 2
|
|
|
|
@pytest.mark.anyio
|
|
async def test_passes_args_and_kwargs(self):
|
|
"""Should pass arguments to the function."""
|
|
async def add(a, b, multiplier=1):
|
|
return (a + b) * multiplier
|
|
|
|
result = await retry_async(add, 2, 3, max_attempts=1, multiplier=2)
|
|
assert result == 10
|
|
|
|
@pytest.mark.anyio
|
|
async def test_no_retry_on_key_error(self):
|
|
"""Should not retry on non-retryable exceptions."""
|
|
call_count = 0
|
|
|
|
async def bad_func():
|
|
nonlocal call_count
|
|
call_count += 1
|
|
raise KeyError("missing")
|
|
|
|
with pytest.raises(KeyError):
|
|
await retry_async(bad_func, max_attempts=3)
|
|
assert call_count == 1
|