forked from Rockachopa/Timmy-time-dashboard
test: remove hardcoded sleeps, add pytest-timeout (#69)
- Replace fixed time.sleep() calls with intelligent polling or WebDriverWait - Add pytest-timeout dependency and --timeout=30 to prevent hangs - Fixes test flakiness and improves test suite speed Co-authored-by: Alexander Payne <apayne@MM.local>
This commit is contained in:
committed by
GitHub
parent
bf0e388d2a
commit
51140fb7f0
@@ -20,14 +20,14 @@ from infrastructure.router.cascade import (
|
||||
|
||||
class TestProviderMetrics:
|
||||
"""Test provider metrics tracking."""
|
||||
|
||||
|
||||
def test_empty_metrics(self):
|
||||
"""Test metrics with no requests."""
|
||||
metrics = ProviderMetrics()
|
||||
assert metrics.total_requests == 0
|
||||
assert metrics.avg_latency_ms == 0.0
|
||||
assert metrics.error_rate == 0.0
|
||||
|
||||
|
||||
def test_avg_latency_calculation(self):
|
||||
"""Test average latency calculation."""
|
||||
metrics = ProviderMetrics(
|
||||
@@ -35,7 +35,7 @@ class TestProviderMetrics:
|
||||
total_latency_ms=1000.0, # 4 requests, 1000ms total
|
||||
)
|
||||
assert metrics.avg_latency_ms == 250.0
|
||||
|
||||
|
||||
def test_error_rate_calculation(self):
|
||||
"""Test error rate calculation."""
|
||||
metrics = ProviderMetrics(
|
||||
@@ -48,7 +48,7 @@ class TestProviderMetrics:
|
||||
|
||||
class TestProvider:
|
||||
"""Test Provider dataclass."""
|
||||
|
||||
|
||||
def test_get_default_model(self):
|
||||
"""Test getting default model."""
|
||||
provider = Provider(
|
||||
@@ -62,7 +62,7 @@ class TestProvider:
|
||||
],
|
||||
)
|
||||
assert provider.get_default_model() == "llama3"
|
||||
|
||||
|
||||
def test_get_default_model_no_default(self):
|
||||
"""Test getting first model when no default set."""
|
||||
provider = Provider(
|
||||
@@ -76,7 +76,7 @@ class TestProvider:
|
||||
],
|
||||
)
|
||||
assert provider.get_default_model() == "llama3"
|
||||
|
||||
|
||||
def test_get_default_model_empty(self):
|
||||
"""Test with no models."""
|
||||
provider = Provider(
|
||||
@@ -91,7 +91,7 @@ class TestProvider:
|
||||
|
||||
class TestRouterConfig:
|
||||
"""Test router configuration."""
|
||||
|
||||
|
||||
def test_default_config(self):
|
||||
"""Test default configuration values."""
|
||||
config = RouterConfig()
|
||||
@@ -103,13 +103,13 @@ class TestRouterConfig:
|
||||
|
||||
class TestCascadeRouterInit:
|
||||
"""Test CascadeRouter initialization."""
|
||||
|
||||
|
||||
def test_init_without_config(self, tmp_path):
|
||||
"""Test initialization without config file."""
|
||||
router = CascadeRouter(config_path=tmp_path / "nonexistent.yaml")
|
||||
assert len(router.providers) == 0
|
||||
assert router.config.timeout_seconds == 30
|
||||
|
||||
|
||||
def test_init_with_config(self, tmp_path):
|
||||
"""Test initialization with config file."""
|
||||
config = {
|
||||
@@ -129,16 +129,16 @@ class TestCascadeRouterInit:
|
||||
}
|
||||
config_path = tmp_path / "providers.yaml"
|
||||
config_path.write_text(yaml.dump(config))
|
||||
|
||||
|
||||
router = CascadeRouter(config_path=config_path)
|
||||
assert router.config.timeout_seconds == 60
|
||||
assert router.config.max_retries_per_provider == 3
|
||||
assert len(router.providers) == 0 # Provider is disabled
|
||||
|
||||
|
||||
def test_env_var_expansion(self, tmp_path, monkeypatch):
|
||||
"""Test environment variable expansion in config."""
|
||||
monkeypatch.setenv("TEST_API_KEY", "secret123")
|
||||
|
||||
|
||||
config = {
|
||||
"cascade": {},
|
||||
"providers": [
|
||||
@@ -153,7 +153,7 @@ class TestCascadeRouterInit:
|
||||
}
|
||||
config_path = tmp_path / "providers.yaml"
|
||||
config_path.write_text(yaml.dump(config))
|
||||
|
||||
|
||||
router = CascadeRouter(config_path=config_path)
|
||||
assert len(router.providers) == 1
|
||||
assert router.providers[0].api_key == "secret123"
|
||||
@@ -161,80 +161,82 @@ class TestCascadeRouterInit:
|
||||
|
||||
class TestCascadeRouterMetrics:
|
||||
"""Test metrics tracking."""
|
||||
|
||||
|
||||
def test_record_success(self):
|
||||
"""Test recording successful request."""
|
||||
provider = Provider(name="test", type="ollama", enabled=True, priority=1)
|
||||
|
||||
|
||||
router = CascadeRouter(config_path=Path("/nonexistent"))
|
||||
router._record_success(provider, 150.0)
|
||||
|
||||
|
||||
assert provider.metrics.total_requests == 1
|
||||
assert provider.metrics.successful_requests == 1
|
||||
assert provider.metrics.total_latency_ms == 150.0
|
||||
assert provider.metrics.consecutive_failures == 0
|
||||
|
||||
|
||||
def test_record_failure(self):
|
||||
"""Test recording failed request."""
|
||||
provider = Provider(name="test", type="ollama", enabled=True, priority=1)
|
||||
|
||||
|
||||
router = CascadeRouter(config_path=Path("/nonexistent"))
|
||||
router._record_failure(provider)
|
||||
|
||||
|
||||
assert provider.metrics.total_requests == 1
|
||||
assert provider.metrics.failed_requests == 1
|
||||
assert provider.metrics.consecutive_failures == 1
|
||||
|
||||
|
||||
def test_circuit_breaker_opens(self):
|
||||
"""Test circuit breaker opens after failures."""
|
||||
provider = Provider(name="test", type="ollama", enabled=True, priority=1)
|
||||
|
||||
|
||||
router = CascadeRouter(config_path=Path("/nonexistent"))
|
||||
router.config.circuit_breaker_failure_threshold = 3
|
||||
|
||||
|
||||
# Record 3 failures
|
||||
for _ in range(3):
|
||||
router._record_failure(provider)
|
||||
|
||||
|
||||
assert provider.circuit_state == CircuitState.OPEN
|
||||
assert provider.status == ProviderStatus.UNHEALTHY
|
||||
assert provider.circuit_opened_at is not None
|
||||
|
||||
|
||||
def test_circuit_breaker_can_close(self):
|
||||
"""Test circuit breaker can transition to closed."""
|
||||
provider = Provider(name="test", type="ollama", enabled=True, priority=1)
|
||||
|
||||
|
||||
router = CascadeRouter(config_path=Path("/nonexistent"))
|
||||
router.config.circuit_breaker_failure_threshold = 3
|
||||
router.config.circuit_breaker_recovery_timeout = 1
|
||||
|
||||
router.config.circuit_breaker_recovery_timeout = 0.1
|
||||
|
||||
# Open the circuit
|
||||
for _ in range(3):
|
||||
router._record_failure(provider)
|
||||
|
||||
|
||||
assert provider.circuit_state == CircuitState.OPEN
|
||||
|
||||
# Wait for recovery timeout
|
||||
time.sleep(1.1)
|
||||
|
||||
|
||||
# Wait for recovery timeout (reduced for faster tests)
|
||||
import time
|
||||
|
||||
time.sleep(0.2)
|
||||
|
||||
# Check if can close
|
||||
assert router._can_close_circuit(provider) is True
|
||||
|
||||
|
||||
def test_half_open_to_closed(self):
|
||||
"""Test circuit breaker closes after successful test calls."""
|
||||
provider = Provider(name="test", type="ollama", enabled=True, priority=1)
|
||||
|
||||
|
||||
router = CascadeRouter(config_path=Path("/nonexistent"))
|
||||
router.config.circuit_breaker_half_open_max_calls = 2
|
||||
|
||||
|
||||
# Manually set to half-open
|
||||
provider.circuit_state = CircuitState.HALF_OPEN
|
||||
provider.half_open_calls = 0
|
||||
|
||||
|
||||
# Record successful calls
|
||||
router._record_success(provider, 100.0)
|
||||
assert provider.circuit_state == CircuitState.HALF_OPEN # Still half-open
|
||||
|
||||
|
||||
router._record_success(provider, 100.0)
|
||||
assert provider.circuit_state == CircuitState.CLOSED # Now closed
|
||||
assert provider.status == ProviderStatus.HEALTHY
|
||||
@@ -242,19 +244,19 @@ class TestCascadeRouterMetrics:
|
||||
|
||||
class TestCascadeRouterGetMetrics:
|
||||
"""Test get_metrics method."""
|
||||
|
||||
|
||||
def test_get_metrics_empty(self):
|
||||
"""Test getting metrics with no providers."""
|
||||
router = CascadeRouter(config_path=Path("/nonexistent"))
|
||||
metrics = router.get_metrics()
|
||||
|
||||
|
||||
assert "providers" in metrics
|
||||
assert len(metrics["providers"]) == 0
|
||||
|
||||
|
||||
def test_get_metrics_with_providers(self):
|
||||
"""Test getting metrics with providers."""
|
||||
router = CascadeRouter(config_path=Path("/nonexistent"))
|
||||
|
||||
|
||||
# Add a test provider
|
||||
provider = Provider(
|
||||
name="test",
|
||||
@@ -266,11 +268,11 @@ class TestCascadeRouterGetMetrics:
|
||||
provider.metrics.successful_requests = 8
|
||||
provider.metrics.failed_requests = 2
|
||||
provider.metrics.total_latency_ms = 2000.0
|
||||
|
||||
|
||||
router.providers = [provider]
|
||||
|
||||
|
||||
metrics = router.get_metrics()
|
||||
|
||||
|
||||
assert len(metrics["providers"]) == 1
|
||||
p_metrics = metrics["providers"][0]
|
||||
assert p_metrics["name"] == "test"
|
||||
@@ -281,11 +283,11 @@ class TestCascadeRouterGetMetrics:
|
||||
|
||||
class TestCascadeRouterGetStatus:
|
||||
"""Test get_status method."""
|
||||
|
||||
|
||||
def test_get_status(self):
|
||||
"""Test getting router status."""
|
||||
router = CascadeRouter(config_path=Path("/nonexistent"))
|
||||
|
||||
|
||||
provider = Provider(
|
||||
name="test",
|
||||
type="ollama",
|
||||
@@ -294,9 +296,9 @@ class TestCascadeRouterGetStatus:
|
||||
models=[{"name": "llama3", "default": True}],
|
||||
)
|
||||
router.providers = [provider]
|
||||
|
||||
|
||||
status = router.get_status()
|
||||
|
||||
|
||||
assert status["total_providers"] == 1
|
||||
assert status["healthy_providers"] == 1
|
||||
assert status["degraded_providers"] == 0
|
||||
@@ -307,11 +309,11 @@ class TestCascadeRouterGetStatus:
|
||||
@pytest.mark.asyncio
|
||||
class TestCascadeRouterComplete:
|
||||
"""Test complete method with failover."""
|
||||
|
||||
|
||||
async def test_complete_with_ollama(self):
|
||||
"""Test successful completion with Ollama."""
|
||||
router = CascadeRouter(config_path=Path("/nonexistent"))
|
||||
|
||||
|
||||
provider = Provider(
|
||||
name="ollama-local",
|
||||
type="ollama",
|
||||
@@ -321,7 +323,7 @@ class TestCascadeRouterComplete:
|
||||
models=[{"name": "llama3.2", "default": True}],
|
||||
)
|
||||
router.providers = [provider]
|
||||
|
||||
|
||||
# Mock the Ollama call
|
||||
with patch.object(router, "_call_ollama") as mock_call:
|
||||
mock_call.return_value = AsyncMock()()
|
||||
@@ -329,19 +331,19 @@ class TestCascadeRouterComplete:
|
||||
"content": "Hello, world!",
|
||||
"model": "llama3.2",
|
||||
}
|
||||
|
||||
|
||||
result = await router.complete(
|
||||
messages=[{"role": "user", "content": "Hi"}],
|
||||
)
|
||||
|
||||
|
||||
assert result["content"] == "Hello, world!"
|
||||
assert result["provider"] == "ollama-local"
|
||||
assert result["model"] == "llama3.2"
|
||||
|
||||
|
||||
async def test_failover_to_second_provider(self):
|
||||
"""Test failover when first provider fails."""
|
||||
router = CascadeRouter(config_path=Path("/nonexistent"))
|
||||
|
||||
|
||||
provider1 = Provider(
|
||||
name="ollama-failing",
|
||||
type="ollama",
|
||||
@@ -359,31 +361,31 @@ class TestCascadeRouterComplete:
|
||||
models=[{"name": "llama3.2", "default": True}],
|
||||
)
|
||||
router.providers = [provider1, provider2]
|
||||
|
||||
|
||||
# First provider fails, second succeeds
|
||||
call_count = [0]
|
||||
|
||||
|
||||
async def side_effect(*args, **kwargs):
|
||||
call_count[0] += 1
|
||||
# First 2 retries for provider1 fail, then provider2 succeeds
|
||||
if call_count[0] <= router.config.max_retries_per_provider:
|
||||
raise RuntimeError("Connection failed")
|
||||
return {"content": "Backup response", "model": "llama3.2"}
|
||||
|
||||
|
||||
with patch.object(router, "_call_ollama") as mock_call:
|
||||
mock_call.side_effect = side_effect
|
||||
|
||||
|
||||
result = await router.complete(
|
||||
messages=[{"role": "user", "content": "Hi"}],
|
||||
)
|
||||
|
||||
|
||||
assert result["content"] == "Backup response"
|
||||
assert result["provider"] == "ollama-backup"
|
||||
|
||||
|
||||
async def test_all_providers_fail(self):
|
||||
"""Test error when all providers fail."""
|
||||
router = CascadeRouter(config_path=Path("/nonexistent"))
|
||||
|
||||
|
||||
provider = Provider(
|
||||
name="failing",
|
||||
type="ollama",
|
||||
@@ -392,19 +394,19 @@ class TestCascadeRouterComplete:
|
||||
models=[{"name": "llama3.2", "default": True}],
|
||||
)
|
||||
router.providers = [provider]
|
||||
|
||||
|
||||
with patch.object(router, "_call_ollama") as mock_call:
|
||||
mock_call.side_effect = RuntimeError("Always fails")
|
||||
|
||||
|
||||
with pytest.raises(RuntimeError) as exc_info:
|
||||
await router.complete(messages=[{"role": "user", "content": "Hi"}])
|
||||
|
||||
|
||||
assert "All providers failed" in str(exc_info.value)
|
||||
|
||||
|
||||
async def test_skips_unhealthy_provider(self):
|
||||
"""Test that unhealthy providers are skipped."""
|
||||
router = CascadeRouter(config_path=Path("/nonexistent"))
|
||||
|
||||
|
||||
provider1 = Provider(
|
||||
name="unhealthy",
|
||||
type="ollama",
|
||||
@@ -423,25 +425,25 @@ class TestCascadeRouterComplete:
|
||||
models=[{"name": "llama3.2", "default": True}],
|
||||
)
|
||||
router.providers = [provider1, provider2]
|
||||
|
||||
|
||||
with patch.object(router, "_call_ollama") as mock_call:
|
||||
mock_call.return_value = {"content": "Success", "model": "llama3.2"}
|
||||
|
||||
|
||||
result = await router.complete(
|
||||
messages=[{"role": "user", "content": "Hi"}],
|
||||
)
|
||||
|
||||
|
||||
# Should use the healthy provider
|
||||
assert result["provider"] == "healthy"
|
||||
|
||||
|
||||
class TestProviderAvailabilityCheck:
|
||||
"""Test provider availability checking."""
|
||||
|
||||
|
||||
def test_check_ollama_without_requests(self):
|
||||
"""Test Ollama returns True when requests not available (fallback)."""
|
||||
router = CascadeRouter(config_path=Path("/nonexistent"))
|
||||
|
||||
|
||||
provider = Provider(
|
||||
name="ollama",
|
||||
type="ollama",
|
||||
@@ -449,20 +451,21 @@ class TestProviderAvailabilityCheck:
|
||||
priority=1,
|
||||
url="http://localhost:11434",
|
||||
)
|
||||
|
||||
|
||||
# When requests is None, assume available
|
||||
import infrastructure.router.cascade as cascade_module
|
||||
|
||||
old_requests = cascade_module.requests
|
||||
cascade_module.requests = None
|
||||
try:
|
||||
assert router._check_provider_available(provider) is True
|
||||
finally:
|
||||
cascade_module.requests = old_requests
|
||||
|
||||
|
||||
def test_check_openai_with_key(self):
|
||||
"""Test OpenAI with API key."""
|
||||
router = CascadeRouter(config_path=Path("/nonexistent"))
|
||||
|
||||
|
||||
provider = Provider(
|
||||
name="openai",
|
||||
type="openai",
|
||||
@@ -470,13 +473,13 @@ class TestProviderAvailabilityCheck:
|
||||
priority=1,
|
||||
api_key="sk-test123",
|
||||
)
|
||||
|
||||
|
||||
assert router._check_provider_available(provider) is True
|
||||
|
||||
|
||||
def test_check_openai_without_key(self):
|
||||
"""Test OpenAI without API key."""
|
||||
router = CascadeRouter(config_path=Path("/nonexistent"))
|
||||
|
||||
|
||||
provider = Provider(
|
||||
name="openai",
|
||||
type="openai",
|
||||
@@ -484,40 +487,40 @@ class TestProviderAvailabilityCheck:
|
||||
priority=1,
|
||||
api_key=None,
|
||||
)
|
||||
|
||||
|
||||
assert router._check_provider_available(provider) is False
|
||||
|
||||
|
||||
def test_check_airllm_installed(self):
|
||||
"""Test AirLLM when installed."""
|
||||
router = CascadeRouter(config_path=Path("/nonexistent"))
|
||||
|
||||
|
||||
provider = Provider(
|
||||
name="airllm",
|
||||
type="airllm",
|
||||
enabled=True,
|
||||
priority=1,
|
||||
)
|
||||
|
||||
|
||||
with patch("builtins.__import__") as mock_import:
|
||||
mock_import.return_value = MagicMock()
|
||||
assert router._check_provider_available(provider) is True
|
||||
|
||||
|
||||
def test_check_airllm_not_installed(self):
|
||||
"""Test AirLLM when not installed."""
|
||||
router = CascadeRouter(config_path=Path("/nonexistent"))
|
||||
|
||||
|
||||
provider = Provider(
|
||||
name="airllm",
|
||||
type="airllm",
|
||||
enabled=True,
|
||||
priority=1,
|
||||
)
|
||||
|
||||
|
||||
# Patch __import__ to simulate airllm not being available
|
||||
def raise_import_error(name, *args, **kwargs):
|
||||
if name == "airllm":
|
||||
raise ImportError("No module named 'airllm'")
|
||||
return __builtins__.__import__(name, *args, **kwargs)
|
||||
|
||||
|
||||
with patch("builtins.__import__", side_effect=raise_import_error):
|
||||
assert router._check_provider_available(provider) is False
|
||||
|
||||
Reference in New Issue
Block a user