Skip to content

Commit 65c81c9

Browse files
committed
test: stabilize flaky tests with proper async coordination
Fix timing-sensitive tests that were failing intermittently in CI by: - Adding retry loops with proper timeout handling for async background operations - Improving error messages to show actual vs expected counts after timeouts - Removing unused imports to clean up test files - Adding proper mocking for app creation to avoid blocking operations during tests The key changes address race conditions where background workers needed more time to process queued operations, particularly in access logging and storage tests. Tests now wait up to 3-5 seconds with polling rather than fixed delays, making them more reliable across different CI environments.
1 parent 3c1f7a6 commit 65c81c9

File tree

5 files changed

+66
-31
lines changed

5 files changed

+66
-31
lines changed

tests/test_access_logger_integration.py

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -397,13 +397,20 @@ async def test_high_volume_access_logging(
397397
# Should complete quickly
398398
assert log_time < 5.0, f"High-volume logging took too long: {log_time}s"
399399

400-
# Give background worker time to process
401-
await asyncio.sleep(2.0)
402-
403-
# Verify all were stored
404-
with Session(storage_with_db._engine) as session:
405-
count = len(session.exec(select(AccessLog)).all())
406-
assert count == num_logs, f"Expected {num_logs} logs, got {count}"
400+
# Give background worker time to process with retries
401+
for _attempt in range(10):
402+
await asyncio.sleep(0.5)
403+
with Session(storage_with_db._engine) as session:
404+
count = len(session.exec(select(AccessLog)).all())
405+
if count == num_logs:
406+
break
407+
else:
408+
# Final check with detailed error
409+
with Session(storage_with_db._engine) as session:
410+
count = len(session.exec(select(AccessLog)).all())
411+
assert count == num_logs, (
412+
f"Expected {num_logs} logs, got {count} after 5s wait"
413+
)
407414

408415
@pytest.mark.unit
409416
async def test_mixed_streaming_and_regular_logs(
@@ -443,15 +450,24 @@ async def test_mixed_streaming_and_regular_logs(
443450
# Execute all concurrently
444451
await asyncio.gather(*tasks)
445452

446-
# Give background worker time to process
447-
await asyncio.sleep(0.5)
448-
449-
# Verify all stored correctly
450-
with Session(storage_with_db._engine) as session:
451-
results = session.exec(
452-
select(AccessLog).order_by(AccessLog.request_id)
453-
).all()
454-
assert len(results) == 20
453+
# Give background worker time to process with retries
454+
for _attempt in range(10):
455+
await asyncio.sleep(0.3)
456+
with Session(storage_with_db._engine) as session:
457+
results = session.exec(
458+
select(AccessLog).order_by(AccessLog.request_id)
459+
).all()
460+
if len(results) == 20:
461+
break
462+
else:
463+
# Final check with detailed error
464+
with Session(storage_with_db._engine) as session:
465+
results = session.exec(
466+
select(AccessLog).order_by(AccessLog.request_id)
467+
).all()
468+
assert len(results) == 20, (
469+
f"Expected 20 logs, got {len(results)} after 3s wait"
470+
)
455471

456472
# Verify streaming flags are correct
457473
for i, result in enumerate(results):

tests/test_api.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
"""
66

77
from typing import Any
8-
from unittest.mock import AsyncMock
98

109
import pytest
1110
from fastapi.testclient import TestClient
@@ -35,8 +34,6 @@
3534
STANDARD_OPENAI_REQUEST,
3635
STREAMING_ANTHROPIC_REQUEST,
3736
STREAMING_OPENAI_REQUEST,
38-
create_anthropic_request,
39-
create_openai_request,
4037
)
4138

4239

tests/test_queue_duckdb_storage.py

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -213,15 +213,23 @@ def mock_store_sync(data: AccessLogPayload) -> bool:
213213
await initialized_storage.store_request(log1)
214214
await initialized_storage.store_request(log2)
215215

216-
# Give time for processing
217-
await asyncio.sleep(0.2)
216+
# Give time for processing with retries
217+
result = None
218+
for _attempt in range(10):
219+
await asyncio.sleep(0.3)
220+
with Session(initialized_storage._engine) as session:
221+
result = session.exec(
222+
select(AccessLog).where(
223+
AccessLog.request_id == "success-request-2"
224+
)
225+
).first()
226+
if result is not None:
227+
break
218228

219229
# Second request should succeed despite first failing
220-
with Session(initialized_storage._engine) as session:
221-
result = session.exec(
222-
select(AccessLog).where(AccessLog.request_id == "success-request-2")
223-
).first()
224-
assert result is not None
230+
assert result is not None, (
231+
"Expected success-request-2 to be processed after error recovery"
232+
)
225233

226234
await initialized_storage.close()
227235

tests/test_scheduler.py

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -554,16 +554,31 @@ async def test_app_lifespan_with_scheduler(
554554

555555
def test_scheduler_disabled_app_still_works(self) -> None:
556556
"""Test that app works when scheduler is disabled."""
557+
from unittest.mock import patch
558+
557559
from ccproxy.api.app import create_app
558560

559561
settings = Settings()
560562
settings.scheduler.enabled = False
561563

562-
app = create_app(settings)
564+
# Mock any potential blocking operations during app creation
565+
with (
566+
patch("ccproxy.observability.metrics.get_metrics") as mock_metrics,
567+
patch("ccproxy.config.settings.get_settings") as mock_get_settings,
568+
patch(
569+
"ccproxy.observability.stats_printer.get_stats_collector"
570+
) as mock_stats,
571+
patch("ccproxy.pricing.updater.PricingUpdater") as mock_pricing,
572+
patch("ccproxy.services.credentials.CredentialsManager") as mock_creds,
573+
):
574+
# Mock settings to return our test configuration
575+
mock_get_settings.return_value = settings
563576

564-
with TestClient(app) as client:
565-
response = client.get("/health")
566-
assert response.status_code == 200
577+
app = create_app(settings)
578+
579+
with TestClient(app) as client:
580+
response = client.get("/health")
581+
assert response.status_code == 200
567582

568583

569584
class TestSchedulerErrorScenarios:

tests/test_streaming.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515

1616
import json
1717
from typing import TYPE_CHECKING, Any
18-
from unittest.mock import AsyncMock
1918

2019
import pytest
2120
from fastapi.testclient import TestClient

0 commit comments

Comments
 (0)