Test Fixes (609/609 passing): - Fixed DiceSystem API to accept team_id/player_id parameters for audit trails - Fixed dice roll history timing issue in test - Fixed terminal client mock to match resolve_play signature (X-Check params) - Fixed result chart test mocks with missing pitching fields - Fixed flaky test by using groundball_a (exists in both batting/pitching) Documentation Updates: - Added Testing Policy section to backend/CLAUDE.md - Added Testing Policy section to tests/CLAUDE.md - Documented 100% unit test requirement before commits - Added git hook setup instructions Git Hook System: - Created .git-hooks/pre-commit script (enforces 100% test pass) - Created .git-hooks/install-hooks.sh (easy installation) - Created .git-hooks/README.md (hook documentation) - Hook automatically runs all unit tests before each commit - Blocks commits if any test fails All 609 unit tests now passing (100%) Integration tests have known asyncpg connection issues (documented) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
529 lines
16 KiB
Markdown
529 lines
16 KiB
Markdown
# Backend Tests - Developer Guide
|
||
|
||
## Overview
|
||
|
||
Comprehensive test suite for the Paper Dynasty backend game engine covering unit tests, integration tests, and end-to-end scenarios.
|
||
|
||
**Test Structure**:
|
||
```
|
||
tests/
|
||
├── unit/ # Fast, isolated unit tests (no DB)
|
||
│ ├── config/ # League configs, PlayOutcome enum
|
||
│ ├── core/ # Game engine, dice, state manager, validators
|
||
│ ├── models/ # Pydantic models (game, player, roster)
|
||
│ └── terminal_client/ # Terminal client modules
|
||
├── integration/ # Database-dependent tests
|
||
│ ├── database/ # DatabaseOperations tests
|
||
│ ├── test_game_engine.py # Full game engine with DB
|
||
│ └── test_state_persistence.py # State recovery tests
|
||
└── e2e/ # End-to-end tests (future)
|
||
```
|
||
|
||
## Testing Policy
|
||
|
||
**🚨 REQUIRED: 100% unit tests passing before committing to any feature branch.**
|
||
|
||
### Commit Policy
|
||
|
||
This project enforces a strict testing policy to maintain code quality and prevent regressions.
|
||
|
||
**Before Every Commit:**
|
||
- ✅ **MUST**: Run `uv run pytest tests/unit/ -q`
|
||
- ✅ **MUST**: All 609 unit tests passing (100%)
|
||
- ✅ **MUST**: Fix any failing tests before committing
|
||
- ⚠️ **OPTIONAL**: Use `--no-verify` for `[WIP]` commits (feature branches only)
|
||
|
||
**Before Merging to Main:**
|
||
- ✅ **MUST**: 100% unit tests passing
|
||
- ✅ **MUST**: Code review approval
|
||
- ✅ **MUST**: CI/CD green build
|
||
- ❌ **NEVER**: Merge with failing tests
|
||
|
||
**Automated Enforcement:**
|
||
|
||
A git pre-commit hook is available to automatically run tests before each commit.
|
||
|
||
```bash
|
||
# Install the hook (one-time setup)
|
||
cd /mnt/NV2/Development/strat-gameplay-webapp/backend
|
||
cp .git-hooks/pre-commit .git/hooks/pre-commit
|
||
chmod +x .git/hooks/pre-commit
|
||
```
|
||
|
||
See `backend/CLAUDE.md` → "Testing Policy" section for full details.
|
||
|
||
### Current Test Baseline
|
||
|
||
**Must maintain or improve:**
|
||
- ✅ Unit tests: **609/609 passing (100%)**
|
||
- ⏱️ Execution: **~1 second**
|
||
- 📊 Coverage: High coverage of core systems
|
||
|
||
**Integration tests status:**
|
||
- ⚠️ Known infrastructure issues (49 errors)
|
||
- ℹ️ Not required for commits (fix infrastructure separately)
|
||
- ℹ️ Run individually during development
|
||
|
||
## Running Tests
|
||
|
||
### Unit Tests (Recommended)
|
||
|
||
**Fast, reliable, no database required**:
|
||
```bash
|
||
# All unit tests
|
||
uv run pytest tests/unit/ -v
|
||
|
||
# Specific module
|
||
uv run pytest tests/unit/core/test_game_engine.py -v
|
||
|
||
# Specific test
|
||
uv run pytest tests/unit/core/test_game_engine.py::TestGameEngine::test_start_game -v
|
||
|
||
# With coverage
|
||
uv run pytest tests/unit/ --cov=app --cov-report=html
|
||
```
|
||
|
||
**Unit tests should always pass**. If they don't, it's a real code issue.
|
||
|
||
### Integration Tests (Database Required)
|
||
|
||
**⚠️ CRITICAL: Integration tests have known infrastructure issues**
|
||
|
||
#### Known Issue: AsyncPG Connection Conflicts
|
||
|
||
**Problem**: Integration tests share database connections and event loops, causing:
|
||
- `asyncpg.exceptions._base.InterfaceError: cannot perform operation: another operation is in progress`
|
||
- `Task got Future attached to a different loop`
|
||
|
||
**Why This Happens**:
|
||
- AsyncPG connections don't support concurrent operations
|
||
- pytest-asyncio fixtures with mismatched scopes (module vs function)
|
||
- Tests running in parallel try to reuse the same connection
|
||
|
||
**Current Workaround**: Run integration tests **individually** or **serially**:
|
||
|
||
```bash
|
||
# Run one test at a time (always works)
|
||
uv run pytest tests/integration/database/test_operations.py::TestDatabaseOperationsGame::test_create_game -v
|
||
|
||
# Run test class serially
|
||
uv run pytest tests/integration/database/test_operations.py::TestDatabaseOperationsGame -v
|
||
|
||
# Run entire file (may have conflicts after first test)
|
||
uv run pytest tests/integration/database/test_operations.py -v
|
||
|
||
# Force serial execution (slower but more reliable)
|
||
uv run pytest tests/integration/ -v -x # -x stops on first failure
|
||
```
|
||
|
||
**DO NOT**:
|
||
- Run all integration tests at once: `pytest tests/integration/ -v` ❌ (will fail)
|
||
- Expect integration tests to work in parallel ❌
|
||
|
||
**Long-term Fix Needed**:
|
||
1. Update fixtures to use proper asyncio scope management
|
||
2. Ensure each test gets isolated database session
|
||
3. Consider using `pytest-xdist` with proper worker isolation
|
||
4. Or redesign fixtures to create fresh connections per test
|
||
|
||
### All Tests
|
||
|
||
```bash
|
||
# Run everything (expect integration failures due to connection issues)
|
||
uv run pytest tests/ -v
|
||
|
||
# Run with markers
|
||
uv run pytest tests/ -v -m "not integration" # Skip integration tests
|
||
uv run pytest tests/ -v -m integration # Only integration tests
|
||
```
|
||
|
||
## Test Configuration
|
||
|
||
### pytest.ini
|
||
|
||
```ini
|
||
[pytest]
|
||
asyncio_mode = auto
|
||
asyncio_default_fixture_loop_scope = function
|
||
testpaths = tests
|
||
python_files = test_*.py
|
||
python_classes = Test*
|
||
python_functions = test_*
|
||
markers =
|
||
integration: marks tests that require database (deselect with '-m "not integration"')
|
||
```
|
||
|
||
**Key Settings**:
|
||
- `asyncio_mode = auto`: Automatically detect async tests
|
||
- `asyncio_default_fixture_loop_scope = function`: Each test gets own event loop
|
||
|
||
### Fixture Scopes
|
||
|
||
**Critical for async tests**:
|
||
|
||
```python
|
||
# ✅ CORRECT - Matching scopes
|
||
@pytest.fixture(scope="function")
|
||
async def event_loop():
|
||
...
|
||
|
||
@pytest.fixture(scope="function")
|
||
async def db_session(event_loop):
|
||
...
|
||
|
||
# ❌ WRONG - Mismatched scopes cause errors
|
||
@pytest.fixture(scope="module") # Module scope
|
||
async def setup_database(event_loop): # But depends on function-scoped event_loop
|
||
...
|
||
```
|
||
|
||
**Rule**: Async fixtures should typically use `scope="function"` to avoid event loop conflicts.
|
||
|
||
## Common Test Patterns
|
||
|
||
### Unit Test Pattern
|
||
|
||
```python
|
||
import pytest
|
||
from app.core.game_engine import GameEngine
|
||
from app.models.game_models import GameState
|
||
|
||
class TestGameEngine:
|
||
"""Unit tests for game engine - no database required"""
|
||
|
||
def test_something(self):
|
||
"""Test description"""
|
||
# Arrange
|
||
engine = GameEngine()
|
||
state = GameState(game_id=uuid4(), league_id="sba", ...)
|
||
|
||
# Act
|
||
result = engine.some_method(state)
|
||
|
||
# Assert
|
||
assert result.success is True
|
||
```
|
||
|
||
### Integration Test Pattern
|
||
|
||
```python
|
||
import pytest
|
||
from app.database.operations import DatabaseOperations
|
||
from app.database.session import AsyncSessionLocal
|
||
|
||
@pytest.mark.integration
|
||
class TestDatabaseOperations:
|
||
"""Integration tests - requires database"""
|
||
|
||
@pytest.fixture
|
||
async def db_ops(self):
|
||
"""Create DatabaseOperations instance"""
|
||
ops = DatabaseOperations()
|
||
yield ops
|
||
|
||
async def test_create_game(self, db_ops):
|
||
"""Test description"""
|
||
# Arrange
|
||
game_id = uuid4()
|
||
|
||
# Act
|
||
await db_ops.create_game(game_id=game_id, ...)
|
||
|
||
# Assert
|
||
game = await db_ops.get_game(game_id)
|
||
assert game is not None
|
||
```
|
||
|
||
### Async Test Pattern
|
||
|
||
```python
|
||
import pytest
|
||
|
||
# pytest-asyncio automatically detects async tests
|
||
async def test_async_operation():
|
||
result = await some_async_function()
|
||
assert result is not None
|
||
|
||
# Or explicit marker (not required with asyncio_mode=auto)
|
||
@pytest.mark.asyncio
|
||
async def test_another_async_operation():
|
||
...
|
||
```
|
||
|
||
## Database Testing
|
||
|
||
### Test Database Setup
|
||
|
||
**Required Environment Variables** (`.env`):
|
||
```bash
|
||
DATABASE_URL=postgresql+asyncpg://paperdynasty:PASSWORD@10.10.0.42:5432/paperdynasty_dev
|
||
```
|
||
|
||
**Database Connection**:
|
||
- Integration tests use the **same database** as development
|
||
- Tests should clean up after themselves (fixtures handle this)
|
||
- Each test should create unique game IDs to avoid conflicts
|
||
|
||
### Transaction Rollback Pattern
|
||
|
||
```python
|
||
@pytest.fixture
|
||
async def db_session():
|
||
"""Provide isolated database session with automatic rollback"""
|
||
async with AsyncSessionLocal() as session:
|
||
async with session.begin():
|
||
yield session
|
||
# Automatic rollback on fixture teardown
|
||
```
|
||
|
||
**Note**: Current fixtures may not properly isolate transactions, contributing to connection conflicts.
|
||
|
||
## Known Test Issues
|
||
|
||
### 1. Player Model Test Failures
|
||
|
||
**Issue**: `tests/unit/models/test_player_models.py` has 13 failures
|
||
|
||
**Root Causes**:
|
||
- `BasePlayer.get_image_url()` not marked as `@abstractmethod`
|
||
- Factory methods (`from_api_response()`) expect `pos_1` field but test fixtures don't provide it
|
||
- Attribute name mismatch: tests expect `player_id` but model has `id`
|
||
- Display name format mismatch in `PdPlayer.get_display_name()`
|
||
|
||
**Files to Fix**:
|
||
- `app/models/player_models.py` - Update abstract methods and factory methods
|
||
- `tests/unit/models/test_player_models.py` - Update test fixtures to match API response format
|
||
|
||
### 2. Dice System Test Failure
|
||
|
||
**Issue**: `tests/unit/core/test_dice.py::TestRollHistory::test_get_rolls_since`
|
||
|
||
**Symptom**: Expects 1 roll but gets 0
|
||
|
||
**Likely Cause**: Roll history not properly persisting or timestamp filtering issue
|
||
|
||
### 3. Integration Test Connection Conflicts
|
||
|
||
**Issue**: 49 integration test errors due to AsyncPG connection conflicts
|
||
|
||
**Status**: **Known infrastructure issue** - not code bugs
|
||
|
||
**When It Matters**: Only when running multiple integration tests in sequence
|
||
|
||
**When It Doesn't**: Unit tests (474 passing) validate business logic
|
||
|
||
### 4. Event Loop Scope Mismatch
|
||
|
||
**Issue**: `tests/integration/test_state_persistence.py` - all 7 tests fail with scope mismatch
|
||
|
||
**Error**: `ScopeMismatch: You tried to access the function scoped fixture event_loop with a module scoped request object`
|
||
|
||
**Root Cause**: `setup_database` fixture is `scope="module"` but depends on `event_loop` which is `scope="function"`
|
||
|
||
**Fix**: Change `setup_database` to `scope="function"` or create module-scoped event loop
|
||
|
||
## Test Coverage
|
||
|
||
**Current Status** (as of 2025-11-01):
|
||
- ✅ **519 unit tests passing** (92% of unit tests)
|
||
- Added 45 new tests for Phase 3B (X-Check tables and placeholders)
|
||
- ❌ **14 unit tests failing** (player models + 1 dice test - pre-existing)
|
||
- ❌ **49 integration test errors** (connection conflicts - infrastructure issue)
|
||
- ❌ **28 integration test failures** (various - pre-existing)
|
||
|
||
**Coverage by Module**:
|
||
```
|
||
app/config/ ✅ 94/94 tests passing (+36 X-Check table tests)
|
||
- test_league_configs.py 28 tests
|
||
- test_play_outcome.py 30 tests
|
||
- test_x_check_tables.py 36 tests (NEW - Phase 3B)
|
||
app/core/game_engine.py ✅ Well covered (unit tests)
|
||
app/core/runner_advancement.py ✅ 60/60 tests passing (+9 X-Check placeholders)
|
||
- test_runner_advancement.py 51 tests (groundball + placeholders)
|
||
- test_flyball_advancement.py 21 tests
|
||
app/core/state_manager.py ✅ 26/26 tests passing
|
||
app/core/dice.py ⚠️ 1 failure (roll history - pre-existing)
|
||
app/models/game_models.py ✅ 60/60 tests passing
|
||
app/models/player_models.py ❌ 13/32 tests failing (pre-existing)
|
||
app/database/operations.py ⚠️ Integration tests have infrastructure issues
|
||
```
|
||
|
||
## Testing Best Practices
|
||
|
||
### DO
|
||
|
||
- ✅ Write unit tests first (fast, reliable, no DB)
|
||
- ✅ Use descriptive test names: `test_game_ends_after_27_outs`
|
||
- ✅ Follow Arrange-Act-Assert pattern
|
||
- ✅ Use fixtures for common setup
|
||
- ✅ Test edge cases and error conditions
|
||
- ✅ Mock external dependencies (API calls, time, random)
|
||
- ✅ Keep tests independent (no shared state)
|
||
- ✅ Run unit tests frequently during development
|
||
|
||
### DON'T
|
||
|
||
- ❌ Don't run all integration tests at once (connection conflicts)
|
||
- ❌ Don't share state between tests
|
||
- ❌ Don't test implementation details (test behavior)
|
||
- ❌ Don't use real API calls in tests (use mocks)
|
||
- ❌ Don't depend on test execution order
|
||
- ❌ Don't skip writing tests because integration tests are flaky
|
||
|
||
### Mocking Examples
|
||
|
||
```python
|
||
from unittest.mock import Mock, patch, AsyncMock
|
||
|
||
# Mock database operations
|
||
@patch('app.core.game_engine.DatabaseOperations')
|
||
def test_with_mock_db(mock_db_class):
|
||
mock_db = Mock()
|
||
mock_db_class.return_value = mock_db
|
||
mock_db.create_game = AsyncMock(return_value=None)
|
||
|
||
# Test code that uses DatabaseOperations
|
||
...
|
||
|
||
# Mock dice rolls for deterministic tests
|
||
@patch('app.core.dice.DiceSystem.roll_d20')
|
||
def test_with_fixed_roll(mock_roll):
|
||
mock_roll.return_value = 15
|
||
# Test code expecting roll of 15
|
||
...
|
||
|
||
# Mock Pendulum time
|
||
with time_machine.travel("2025-10-31 12:00:00", tick=False):
|
||
# Test time-dependent code
|
||
...
|
||
```
|
||
|
||
## Debugging Failed Tests
|
||
|
||
### Verbose Output
|
||
|
||
```bash
|
||
# Show full output including print statements
|
||
uv run pytest tests/unit/core/test_game_engine.py -v -s
|
||
|
||
# Show local variables on failure
|
||
uv run pytest tests/unit/core/test_game_engine.py -v -l
|
||
|
||
# Stop on first failure
|
||
uv run pytest tests/unit/core/test_game_engine.py -v -x
|
||
|
||
# Show full traceback
|
||
uv run pytest tests/unit/core/test_game_engine.py -v --tb=long
|
||
```
|
||
|
||
### Interactive Debugging
|
||
|
||
```bash
|
||
# Drop into debugger on failure
|
||
uv run pytest tests/unit/core/test_game_engine.py --pdb
|
||
|
||
# Drop into debugger on first failure
|
||
uv run pytest tests/unit/core/test_game_engine.py --pdb -x
|
||
```
|
||
|
||
### Logging in Tests
|
||
|
||
Tests capture logs by default. View with `-o log_cli=true`:
|
||
|
||
```bash
|
||
uv run pytest tests/unit/core/test_game_engine.py -v -o log_cli=true -o log_cli_level=DEBUG
|
||
```
|
||
|
||
## CI/CD Considerations
|
||
|
||
**Recommended CI Test Strategy**:
|
||
|
||
```yaml
|
||
# Run fast unit tests on every commit
|
||
- name: Unit Tests
|
||
run: uv run pytest tests/unit/ -v --cov=app
|
||
|
||
# Run integration tests serially (slower but reliable)
|
||
- name: Integration Tests
|
||
run: |
|
||
uv run pytest tests/integration/database/test_operations.py::TestDatabaseOperationsGame -v
|
||
uv run pytest tests/integration/database/test_operations.py::TestDatabaseOperationsLineup -v
|
||
# ... run each test class separately
|
||
```
|
||
|
||
**OR** fix the integration test infrastructure first, then run normally.
|
||
|
||
## Troubleshooting
|
||
|
||
### "cannot perform operation: another operation is in progress"
|
||
|
||
**Solution**: Run integration tests individually or fix fixture scopes
|
||
|
||
### "Task got Future attached to a different loop"
|
||
|
||
**Solution**: Ensure all fixtures use `scope="function"` or create proper module-scoped event loop
|
||
|
||
### "No module named 'app'"
|
||
|
||
**Solution**:
|
||
```bash
|
||
# Recommended: Use UV (handles PYTHONPATH automatically)
|
||
cd /mnt/NV2/Development/strat-gameplay-webapp/backend
|
||
uv run pytest tests/unit/ -v
|
||
|
||
# Alternative: Set PYTHONPATH manually
|
||
export PYTHONPATH=/mnt/NV2/Development/strat-gameplay-webapp/backend
|
||
pytest tests/unit/ -v
|
||
```
|
||
|
||
### Tests hang indefinitely
|
||
|
||
**Likely Cause**: Async test without proper event loop cleanup
|
||
|
||
**Solution**: Check fixture scopes and ensure `asyncio_mode = auto` in pytest.ini
|
||
|
||
### Database connection errors
|
||
|
||
**Check**:
|
||
1. PostgreSQL is running: `psql $DATABASE_URL`
|
||
2. `.env` has correct `DATABASE_URL`
|
||
3. Database exists and schema is migrated: `alembic upgrade head`
|
||
|
||
## Integration Test Refactor TODO
|
||
|
||
When refactoring integration tests to fix connection conflicts:
|
||
|
||
1. **Update fixtures in `tests/integration/conftest.py`**:
|
||
- Change all fixtures to `scope="function"`
|
||
- Ensure each test gets fresh database session
|
||
- Implement proper session cleanup
|
||
|
||
2. **Add connection pooling**:
|
||
- Consider using separate connection pool for tests
|
||
- Or create new engine per test (slower but isolated)
|
||
|
||
3. **Add transaction rollback**:
|
||
- Wrap each test in transaction
|
||
- Rollback after test completes
|
||
- Ensures database is clean for next test
|
||
|
||
4. **Consider pytest-xdist**:
|
||
- Run tests in parallel with proper worker isolation
|
||
- Each worker gets own database connection
|
||
- Faster test execution
|
||
|
||
5. **Update `test_state_persistence.py`**:
|
||
- Fix `setup_database` fixture scope mismatch
|
||
- Consider splitting into smaller fixtures
|
||
|
||
## Additional Resources
|
||
|
||
- **pytest-asyncio docs**: https://pytest-asyncio.readthedocs.io/
|
||
- **AsyncPG docs**: https://magicstack.github.io/asyncpg/
|
||
- **SQLAlchemy async**: https://docs.sqlalchemy.org/en/20/orm/extensions/asyncio.html
|
||
- **Backend CLAUDE.md**: `../CLAUDE.md` - Main backend documentation
|
||
|
||
---
|
||
|
||
**Summary**: Unit tests are solid (91% passing), integration tests have known infrastructure issues that need fixture refactoring. Focus on unit tests for development, fix integration test infrastructure as separate task.
|