mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-10 03:28:53 +00:00
Add two detailed guides for addressing CI test flakiness: 1. test-flakiness-guide.md - Developer guide with: - How to use @pytest.mark.no_parallel for async mocks - Patterns for robust async mock setup - Retry logic strategies - Module reload issues and fixes - Quick reference and checklist 2. ci-test-improvements.md - Implementation plan with: - Priority phased rollout (Quick wins → CI → Enforcement) - pytest-rerunfailures plugin setup - GitHub Actions improvements for retries - Makefile targets for testing - Pre-commit hooks for test quality - Test utilities module with decorators - Success metrics and monitoring These guides provide actionable solutions for the CI test failures observed in PRs #21107, #21388, and #21390. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
8.8 KiB
8.8 KiB
CI Test Improvements - Implementation Plan
Current Issues
- Tests pass locally but fail in CI
- Async mock timing issues
- Module reload causing
isinstance()failures
Implemented Solutions
✅ Already In Place
-
@pytest.mark.no_parallelmarker - Line 200 in pyproject.toml- Use for tests with async mocks or timing dependencies
- MCP OAuth test already uses this correctly
-
Retry logic - Lines 195-196 in pyproject.toml
retries = 20 retry_delay = 5 -
Async mode - Line 194 in pyproject.toml
asyncio_mode = "auto"
🔧 Recommended Additions
1. Install pytest-rerunfailures Plugin
Add to pyproject.toml:
[tool.poetry.group.dev.dependencies]
pytest-rerunfailures = "^15.0"
Then use in tests:
@pytest.mark.flaky(reruns=3, reruns_delay=1)
async def test_sometimes_flaky():
# Test code
pass
2. Add Test Categories
Update pyproject.toml:
[tool.pytest.ini_options]
markers = [
"asyncio: mark test as an asyncio test",
"limit_leaks: mark test with memory limit for leak detection",
"no_parallel: mark test to run sequentially (not in parallel)",
"flaky: mark test as potentially flaky (auto-retry)", # NEW
"requires_credentials: mark test that needs external credentials", # NEW
"slow: mark test as slow running (> 10 seconds)", # NEW
]
3. GitHub Actions Improvements
Option A: Run Flaky Tests Separately
.github/workflows/test.yml:
jobs:
test-stable:
name: Stable Tests
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Run Stable Tests
run: |
poetry run pytest tests/ \
-m "not flaky" \
-n 4 \
--maxfail=5
test-flaky:
name: Flaky Tests (with retries)
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Run Flaky Tests
run: |
poetry run pytest tests/ \
-m "flaky" \
--reruns 3 \
--reruns-delay 2 \
-n 1 # Sequential for flaky tests
Option B: Add Retry Step
- name: Run Tests
id: tests
run: poetry run pytest tests/ -v -n 4
- name: Retry Failed Tests
if: failure() && steps.tests.outcome == 'failure'
run: |
echo "Retrying failed tests..."
poetry run pytest tests/ --lf -v --maxfail=5
4. Pre-commit Hook for Test Best Practices
.pre-commit-config.yaml:
repos:
- repo: local
hooks:
- id: check-async-test-markers
name: Check async tests have proper markers
entry: python scripts/check_test_markers.py
language: python
files: ^tests/.*test.*\.py$
scripts/check_test_markers.py:
#!/usr/bin/env python3
import ast
import sys
import re
def check_test_file(filename):
"""Check that async tests with mocks have no_parallel marker"""
with open(filename) as f:
content = f.read()
# Parse AST
tree = ast.parse(content)
issues = []
for node in ast.walk(tree):
if isinstance(node, ast.AsyncFunctionDef) and node.name.startswith('test_'):
# Check if it has @patch or Mock
has_mock = 'patch' in content or 'Mock' in content or 'mock_' in node.name.lower()
# Check if it has @pytest.mark.no_parallel
decorators = [d for d in node.decorator_list]
has_no_parallel = any(
'no_parallel' in ast.unparse(d) for d in decorators
)
if has_mock and not has_no_parallel:
issues.append(
f"{filename}:{node.lineno} - Async test '{node.name}' "
f"uses mocks but missing @pytest.mark.no_parallel"
)
return issues
if __name__ == '__main__':
all_issues = []
for filename in sys.argv[1:]:
all_issues.extend(check_test_file(filename))
if all_issues:
print("❌ Test marker issues found:")
for issue in all_issues:
print(f" {issue}")
sys.exit(1)
else:
print("✅ All async tests properly marked")
sys.exit(0)
5. Makefile Targets for Testing
Makefile:
.PHONY: test test-fast test-flaky test-repeat
# Regular test run
test:
poetry run pytest tests/ -v
# Fast: parallel execution, stop on first failure
test-fast:
poetry run pytest tests/ -n 4 -x
# Only flaky tests with retries
test-flaky:
poetry run pytest tests/ -m flaky --reruns 3 --reruns-delay 1
# Repeat test N times to catch flakiness
test-repeat:
poetry run pytest tests/ --count=100 -x
# Test specific file 100 times
test-file-repeat:
@if [ -z "$(FILE)" ]; then \
echo "Usage: make test-file-repeat FILE=tests/path/to/test.py"; \
exit 1; \
fi
for i in {1..100}; do \
echo "Run $$i/100"; \
poetry run pytest $(FILE) || exit 1; \
done
Usage:
make test-fast # Quick CI-like run
make test-flaky # Test only flaky tests
make test-file-repeat FILE=tests/test_mcp_server.py # Stress test
6. Test Utilities Module
tests/test_utils.py:
"""Common test utilities for handling flakiness"""
import asyncio
import functools
import os
import pytest
from typing import Callable, TypeVar
T = TypeVar('T')
def retry_on_assertion_error(max_attempts: int = 3, delay: float = 0.1):
"""Retry async test on AssertionError (for flaky CI tests)"""
def decorator(func: Callable[..., T]) -> Callable[..., T]:
@functools.wraps(func)
async def wrapper(*args, **kwargs):
last_error = None
for attempt in range(max_attempts):
try:
return await func(*args, **kwargs)
except AssertionError as e:
last_error = e
if attempt < max_attempts - 1:
print(f" ⚠️ Attempt {attempt + 1} failed, retrying...")
await asyncio.sleep(delay)
raise last_error
return wrapper
return decorator
def skip_in_ci(reason: str = "Flaky in CI"):
"""Skip test in CI environment"""
is_ci = os.getenv('CI') == 'true' or os.getenv('GITHUB_ACTIONS') == 'true'
return pytest.mark.skipif(is_ci, reason=reason)
def requires_credentials(*env_vars: str):
"""Skip test if required credentials are missing"""
has_creds = all(os.getenv(var) for var in env_vars)
var_names = ', '.join(env_vars)
return pytest.mark.skipif(
not has_creds,
reason=f"Missing credentials: {var_names}"
)
# Usage examples:
# @retry_on_assertion_error(max_attempts=3)
# async def test_flaky():
# ...
#
# @skip_in_ci("Known timing issue in CI - tracked in #12345")
# async def test_problematic():
# ...
#
# @requires_credentials("GOOGLE_APPLICATION_CREDENTIALS", "VERTEX_PROJECT")
# def test_vertex_ai():
# ...
7. Update Test Documentation
Add to CLAUDE.md:
### Test Best Practices
When writing tests:
1. **Async tests with mocks** → Add `@pytest.mark.no_parallel`
2. **Tests needing credentials** → Add `@requires_credentials("ENV_VAR")`
3. **Known flaky tests** → Add `@pytest.mark.flaky(reruns=3)`
4. **Slow tests** → Add `@pytest.mark.slow`
Example:
\`\`\`python
@pytest.mark.asyncio
@pytest.mark.no_parallel # Has async mocks
async def test_mcp_oauth():
with patch("module.func") as mock:
# Test code
mock.assert_called_once()
\`\`\`
Check test flakiness locally:
\`\`\`bash
make test-file-repeat FILE=tests/path/to/test.py
\`\`\`
Priority Implementation Order
Phase 1: Quick Wins (1 day)
- ✅ Document test patterns (done - test-flakiness-guide.md)
- Add
pytest-rerunfailuresto dependencies - Mark known flaky tests with
@pytest.mark.flaky - Add test utilities module
Phase 2: CI Improvements (2-3 days)
- Update GitHub Actions to retry failed tests
- Split stable vs flaky test runs
- Add Makefile targets for common test scenarios
Phase 3: Enforcement (1 week)
- Add pre-commit hook for test markers
- Update CLAUDE.md with test guidelines
- Review and mark all async tests appropriately
Monitoring Success
Track these metrics:
- Flaky test rate: Tests that fail <10% of runs
- False failure rate: CI failures that pass on retry
- Test runtime: Should decrease with better parallelization
Target goals:
- ✅ <5% flaky test rate
- ✅ <2% false failure rate
- ✅ Test suite completes in <15 minutes
Quick Checklist
Before merging a PR with new tests:
- All async tests with mocks have
@pytest.mark.no_parallel - Tests requiring credentials have skip conditions
- No
isinstance()checks on module-level imports - Async mocks are configured before execution
- Tests pass 10 times locally:
make test-file-repeat FILE=... - Tests pass in parallel:
pytest tests/file.py -n 4