CLAUDE: Update documentation across codebase
Updated CLAUDE.md files with: - Current test counts and status - Session injection pattern documentation - New module references and architecture notes - Updated Phase status (3E-Final complete) - Enhanced troubleshooting guides Files updated: - Root CLAUDE.md: Project overview and phase status - backend/CLAUDE.md: Backend overview with test counts - backend/README.md: Quick start and development guide - backend/app/api/CLAUDE.md: API routes documentation - backend/app/database/CLAUDE.md: Session injection docs - backend/app/utils/CLAUDE.md: Utilities documentation - backend/tests/CLAUDE.md: Testing patterns and policy - frontend-sba/CLAUDE.md: Frontend overview - frontend-sba/store/CLAUDE.md: Store patterns 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Cal Corum
parent
891fb03c52
commit
9c90893b5d
@ -45,7 +45,7 @@ Web-based real-time multiplayer baseball simulation platform replacing legacy Go
|
||||
- Tailwind CSS
|
||||
- Pinia for state management
|
||||
- Socket.io-client
|
||||
- @nuxtjs/auth-next (Discord OAuth)
|
||||
- Discord OAuth via HttpOnly cookies (see `COOKIE_AUTH_IMPLEMENTATION.md`)
|
||||
|
||||
## Key Technical Patterns
|
||||
|
||||
|
||||
@ -32,13 +32,13 @@ backend/
|
||||
```bash
|
||||
cd backend
|
||||
uv sync # Install dependencies
|
||||
docker compose up -d # Start Redis
|
||||
uv run alembic upgrade head # Apply database migrations
|
||||
uv run python -m app.main # Start server at localhost:8000
|
||||
```
|
||||
|
||||
### Testing
|
||||
```bash
|
||||
uv run pytest tests/unit/ -v # All unit tests (739 passing)
|
||||
uv run pytest tests/unit/ -v # All unit tests (836 passing)
|
||||
uv run python -m terminal_client # Interactive REPL
|
||||
```
|
||||
|
||||
@ -122,4 +122,4 @@ uv run pytest tests/unit/ -q # Must show all passing
|
||||
|
||||
---
|
||||
|
||||
**Tests**: 739/739 passing | **Phase**: 3E-Final Complete | **Updated**: 2025-01-19
|
||||
**Tests**: 836 passing | **Phase**: 3E-Final Complete | **Updated**: 2025-01-27
|
||||
|
||||
@ -11,10 +11,73 @@ curl -LsSf https://astral.sh/uv/install.sh | sh
|
||||
# Install dependencies
|
||||
uv sync
|
||||
|
||||
# Apply database migrations
|
||||
uv run alembic upgrade head
|
||||
|
||||
# Run server
|
||||
uv run python -m app.main
|
||||
```
|
||||
|
||||
## Database Migrations
|
||||
|
||||
This project uses [Alembic](https://alembic.sqlalchemy.org/) for database schema migrations.
|
||||
|
||||
### Initial Setup (New Database)
|
||||
|
||||
```bash
|
||||
# Apply all migrations to create schema
|
||||
uv run alembic upgrade head
|
||||
```
|
||||
|
||||
### Creating New Migrations
|
||||
|
||||
```bash
|
||||
# Auto-generate from model changes
|
||||
uv run alembic revision --autogenerate -m "Description of changes"
|
||||
|
||||
# IMPORTANT: Always review the generated migration before applying!
|
||||
# Then apply:
|
||||
uv run alembic upgrade head
|
||||
```
|
||||
|
||||
### Viewing Migration Status
|
||||
|
||||
```bash
|
||||
# Show migration history
|
||||
uv run alembic history
|
||||
|
||||
# Show current revision
|
||||
uv run alembic current
|
||||
```
|
||||
|
||||
### Rolling Back
|
||||
|
||||
```bash
|
||||
# Rollback one migration
|
||||
uv run alembic downgrade -1
|
||||
|
||||
# Rollback to specific revision
|
||||
uv run alembic downgrade 001
|
||||
|
||||
# Rollback all (dangerous!)
|
||||
uv run alembic downgrade base
|
||||
```
|
||||
|
||||
### Migration Best Practices
|
||||
|
||||
1. **Always review auto-generated migrations** before applying - autogenerate is helpful but not perfect
|
||||
2. **Test migrations on dev/staging** before production
|
||||
3. **Keep migrations small and focused** - easier to rollback
|
||||
4. **Never edit migrations that have been applied** to shared databases
|
||||
5. **Include both upgrade and downgrade** for reversibility
|
||||
|
||||
### Existing Migrations
|
||||
|
||||
| Revision | Description |
|
||||
|----------|-------------|
|
||||
| 001 | Initial schema (games, plays, lineups, rolls, etc.) |
|
||||
| 004 | Materialized views for statistics |
|
||||
|
||||
## Documentation
|
||||
|
||||
See `CLAUDE.md` for full documentation.
|
||||
|
||||
@ -26,11 +26,16 @@ GET /health/ready # Ready check with DB connectivity
|
||||
|
||||
### Auth (`/auth`)
|
||||
```python
|
||||
GET /auth/discord # Initiate OAuth flow
|
||||
GET /auth/discord/callback # OAuth callback, returns JWT
|
||||
POST /auth/refresh # Refresh JWT token
|
||||
GET /auth/discord/login # Initiate OAuth flow (redirects to Discord)
|
||||
GET /auth/discord/callback/server # OAuth callback, sets HttpOnly cookies, redirects to frontend
|
||||
GET /auth/me # Get current user from cookie (SSR-compatible)
|
||||
POST /auth/refresh # Refresh access token using refresh cookie
|
||||
POST /auth/logout # Clear auth cookies
|
||||
```
|
||||
|
||||
**Cookie-Based Auth**: Uses HttpOnly cookies with `path="/"` for SSR compatibility.
|
||||
See `COOKIE_AUTH_IMPLEMENTATION.md` for complete flow documentation.
|
||||
|
||||
### Games (`/games`)
|
||||
```python
|
||||
POST /games # Create new game
|
||||
|
||||
@ -4,35 +4,88 @@
|
||||
|
||||
Async PostgreSQL persistence layer using SQLAlchemy 2.0. Handles all database operations with connection pooling and proper transaction management.
|
||||
|
||||
**Schema Management**: Alembic migrations (see `backend/README.md` for commands)
|
||||
|
||||
## Structure
|
||||
|
||||
```
|
||||
app/database/
|
||||
├── __init__.py # Package exports
|
||||
├── session.py # Async session management, Base declarative
|
||||
└── operations.py # DatabaseOperations class
|
||||
└── operations.py # DatabaseOperations class (session injection pattern)
|
||||
|
||||
backend/alembic/ # Migration scripts (managed by Alembic)
|
||||
├── env.py # Migration environment config
|
||||
└── versions/ # Migration files (001, 004, 005, etc.)
|
||||
```
|
||||
|
||||
## Session Management
|
||||
## Session Injection Pattern (IMPORTANT)
|
||||
|
||||
### Async Session Pattern
|
||||
`DatabaseOperations` supports **session injection** to enable:
|
||||
- **Transaction grouping**: Multiple operations in one atomic transaction
|
||||
- **Test isolation**: Tests inject sessions with automatic rollback
|
||||
- **Connection efficiency**: Avoids asyncpg "operation in progress" conflicts
|
||||
|
||||
### Two Usage Modes
|
||||
|
||||
#### Mode 1: Standalone (Default) - Each operation auto-commits
|
||||
```python
|
||||
from app.database.session import get_session
|
||||
from app.database.operations import DatabaseOperations
|
||||
|
||||
async def some_function():
|
||||
async with get_session() as session:
|
||||
result = await session.execute(query)
|
||||
# Auto-commits on success, rolls back on exception
|
||||
db_ops = DatabaseOperations() # No session injected
|
||||
await db_ops.create_game(...) # Creates session, commits, closes
|
||||
await db_ops.save_play(...) # Creates NEW session, commits, closes
|
||||
```
|
||||
|
||||
### Connection Pool
|
||||
- **Driver**: asyncpg
|
||||
- **Pool**: SQLAlchemy async pool
|
||||
- Configurable pool size via env vars
|
||||
#### Mode 2: Session Injection - Caller controls transaction
|
||||
```python
|
||||
from app.database.operations import DatabaseOperations
|
||||
from app.database.session import AsyncSessionLocal
|
||||
|
||||
## DatabaseOperations
|
||||
async with AsyncSessionLocal() as session:
|
||||
try:
|
||||
db_ops = DatabaseOperations(session) # Inject session
|
||||
await db_ops.save_play(...) # Uses injected session (flush only)
|
||||
await db_ops.update_game_state(...) # Same session (flush only)
|
||||
await session.commit() # Caller commits once
|
||||
except Exception:
|
||||
await session.rollback()
|
||||
raise
|
||||
```
|
||||
|
||||
Singleton class with all database operations.
|
||||
### When to Use Each Mode
|
||||
|
||||
| Scenario | Mode | Why |
|
||||
|----------|------|-----|
|
||||
| Single operation | Standalone | Simpler, auto-commits |
|
||||
| Multiple related operations | Session injection | Atomic transaction |
|
||||
| Integration tests | Session injection | Rollback after test |
|
||||
| game_engine.py transactions | Session injection | save_play + update_game_state atomic |
|
||||
|
||||
### Internal Implementation
|
||||
|
||||
```python
|
||||
class DatabaseOperations:
|
||||
def __init__(self, session: AsyncSession | None = None):
|
||||
self._session = session
|
||||
|
||||
@asynccontextmanager
|
||||
async def _get_session(self):
|
||||
if self._session:
|
||||
yield self._session # Caller controls commit/rollback
|
||||
else:
|
||||
async with AsyncSessionLocal() as session:
|
||||
try:
|
||||
yield session
|
||||
await session.commit()
|
||||
except Exception:
|
||||
await session.rollback()
|
||||
raise
|
||||
```
|
||||
|
||||
**Key Detail**: Methods use `flush()` not `commit()` - this persists changes within the transaction but doesn't finalize it. For injected sessions, the caller commits. For standalone, the context manager commits.
|
||||
|
||||
## DatabaseOperations API
|
||||
|
||||
### Game Operations
|
||||
```python
|
||||
@ -46,7 +99,7 @@ game_data = await db_ops.load_game_state(game_id)
|
||||
|
||||
### Play Operations
|
||||
```python
|
||||
play_id = await db_ops.save_play(game_id, play_data, stats_data)
|
||||
play_id = await db_ops.save_play(play_data)
|
||||
plays = await db_ops.get_plays(game_id, limit=100)
|
||||
```
|
||||
|
||||
@ -73,12 +126,14 @@ await db_ops.update_session_snapshot(game_id, state_snapshot)
|
||||
|
||||
## Common Patterns
|
||||
|
||||
### Transaction Handling
|
||||
### Transaction Grouping (Recommended for related operations)
|
||||
```python
|
||||
async with AsyncSessionLocal() as session:
|
||||
try:
|
||||
# Multiple operations
|
||||
await session.commit()
|
||||
db_ops = DatabaseOperations(session)
|
||||
await db_ops.save_play(play_data)
|
||||
await db_ops.update_game_state(game_id, ...)
|
||||
await session.commit() # Both succeed or both fail
|
||||
except Exception:
|
||||
await session.rollback()
|
||||
raise
|
||||
@ -122,10 +177,10 @@ DATABASE_URL=postgresql+asyncpg://user:pass@10.10.0.42:5432/paperdynasty_dev
|
||||
|
||||
## References
|
||||
|
||||
- **Database Schema**: See `../../.claude/DATABASE_SCHEMA.md` for complete table details
|
||||
- **Migrations**: See `backend/README.md` for Alembic commands
|
||||
- **Models**: See `../models/CLAUDE.md`
|
||||
- **State Recovery**: See `../core/state_manager.py`
|
||||
|
||||
---
|
||||
|
||||
**Tests**: `tests/integration/database/` | **Updated**: 2025-01-19
|
||||
**Tests**: `tests/integration/database/` (32 tests) | **Updated**: 2025-11-27
|
||||
|
||||
@ -359,24 +359,53 @@ except Exception as e:
|
||||
|
||||
### Authentication Pattern
|
||||
|
||||
**Token in HTTP Headers**:
|
||||
**CURRENT: HttpOnly Cookie Auth** (Server-Side OAuth Flow):
|
||||
```python
|
||||
# Client sends:
|
||||
headers = {
|
||||
"Authorization": f"Bearer {token}"
|
||||
# Backend sets cookies with path="/" for SSR compatibility
|
||||
# See backend/app/utils/cookies.py and COOKIE_AUTH_IMPLEMENTATION.md
|
||||
|
||||
response.set_cookie(
|
||||
key="pd_access_token",
|
||||
value=access_token,
|
||||
httponly=True,
|
||||
secure=is_production(),
|
||||
samesite="lax",
|
||||
path="/", # CRITICAL: Must be "/" not "/api" for SSR
|
||||
)
|
||||
```
|
||||
|
||||
**Frontend API Calls** (use `credentials: 'include'`):
|
||||
```typescript
|
||||
// Browser automatically sends cookies
|
||||
const response = await $fetch('/api/games/', {
|
||||
credentials: 'include',
|
||||
})
|
||||
```
|
||||
|
||||
**SSR Cookie Forwarding** (Nuxt middleware must forward cookies):
|
||||
```typescript
|
||||
if (import.meta.server) {
|
||||
const event = useRequestEvent()
|
||||
headers['Cookie'] = event?.node.req.headers.cookie
|
||||
}
|
||||
```
|
||||
|
||||
**Token in WebSocket Auth**:
|
||||
```python
|
||||
# Client connects with:
|
||||
socket.io.connect("http://localhost:8000", {
|
||||
auth: {
|
||||
token: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9..."
|
||||
}
|
||||
**WebSocket Auth** (cookies sent automatically with same-origin):
|
||||
```typescript
|
||||
// Socket.io connects with cookies (same-origin)
|
||||
const socket = io(wsUrl, {
|
||||
withCredentials: true, // Send cookies
|
||||
})
|
||||
```
|
||||
|
||||
**LEGACY: Token in HTTP Headers** (no longer used for web frontend):
|
||||
```python
|
||||
# Only used for API clients that can't use cookies
|
||||
headers = {
|
||||
"Authorization": f"Bearer {token}"
|
||||
}
|
||||
```
|
||||
|
||||
**FastAPI Dependency**:
|
||||
```python
|
||||
from fastapi import Depends, HTTPException, Header
|
||||
|
||||
@ -55,15 +55,12 @@ 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**
|
||||
- ✅ Unit tests: **979/979 passing (100%)**
|
||||
- ✅ Integration tests: **32/32 passing (100%)**
|
||||
- ⏱️ Unit execution: **~4 seconds**
|
||||
- ⏱️ Integration execution: **~5 seconds**
|
||||
- 📊 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)
|
||||
@ -87,49 +84,50 @@ uv run pytest tests/unit/ --cov=app --cov-report=html
|
||||
|
||||
### 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**:
|
||||
**✅ Integration tests now work reliably** using session injection pattern.
|
||||
|
||||
```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)
|
||||
# Run all integration tests
|
||||
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
|
||||
# Run specific test class
|
||||
uv run pytest tests/integration/database/test_operations.py::TestDatabaseOperationsGame -v
|
||||
|
||||
# Run with markers
|
||||
uv run pytest tests/ -v -m integration
|
||||
```
|
||||
|
||||
**DO NOT**:
|
||||
- Run all integration tests at once: `pytest tests/integration/ -v` ❌ (will fail)
|
||||
- Expect integration tests to work in parallel ❌
|
||||
#### Session Injection Pattern (IMPORTANT)
|
||||
|
||||
**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
|
||||
Integration tests use **session injection** to avoid asyncpg connection conflicts:
|
||||
|
||||
```python
|
||||
# tests/integration/conftest.py
|
||||
@pytest_asyncio.fixture(scope="function")
|
||||
async def db_session():
|
||||
"""Provide isolated database session with automatic rollback"""
|
||||
async with TestAsyncSessionLocal() as session:
|
||||
yield session
|
||||
await session.rollback() # Cleanup after test
|
||||
|
||||
@pytest_asyncio.fixture(scope="function")
|
||||
async def db_ops(db_session: AsyncSession):
|
||||
"""DatabaseOperations with injected session"""
|
||||
return DatabaseOperations(db_session) # All operations share this session
|
||||
```
|
||||
|
||||
**Key Points**:
|
||||
- Each test gets its own session (no connection conflicts)
|
||||
- Session is rolled back after test (automatic cleanup)
|
||||
- Tests use `NullPool` to prevent connection reuse issues
|
||||
- All `db_ops` methods use the same session (no "operation in progress" errors)
|
||||
|
||||
See `app/database/CLAUDE.md` for full session injection documentation.
|
||||
|
||||
### All Tests
|
||||
|
||||
```bash
|
||||
# Run everything (expect integration failures due to connection issues)
|
||||
# Run everything
|
||||
uv run pytest tests/ -v
|
||||
|
||||
# Run with markers
|
||||
@ -204,36 +202,52 @@ class TestGameEngine:
|
||||
assert result.success is True
|
||||
```
|
||||
|
||||
### Integration Test Pattern
|
||||
### Integration Test Pattern (Session Injection)
|
||||
|
||||
```python
|
||||
import pytest
|
||||
from app.database.operations import DatabaseOperations
|
||||
from app.database.session import AsyncSessionLocal
|
||||
from uuid import uuid4
|
||||
|
||||
# Mark all tests in module as integration tests
|
||||
pytestmark = pytest.mark.integration
|
||||
|
||||
@pytest.mark.integration
|
||||
class TestDatabaseOperations:
|
||||
"""Integration tests - requires database"""
|
||||
"""Integration tests - use fixtures from tests/integration/conftest.py"""
|
||||
|
||||
@pytest.fixture
|
||||
async def db_ops(self):
|
||||
"""Create DatabaseOperations instance"""
|
||||
ops = DatabaseOperations()
|
||||
yield ops
|
||||
async def test_create_game(self, db_ops, db_session):
|
||||
"""
|
||||
Test creating a game in database.
|
||||
|
||||
async def test_create_game(self, db_ops):
|
||||
"""Test description"""
|
||||
Uses db_ops fixture (DatabaseOperations with injected session)
|
||||
and db_session for flush/visibility within transaction.
|
||||
"""
|
||||
# Arrange
|
||||
game_id = uuid4()
|
||||
|
||||
# Act
|
||||
await db_ops.create_game(game_id=game_id, ...)
|
||||
game = await db_ops.create_game(
|
||||
game_id=game_id,
|
||||
league_id="sba",
|
||||
home_team_id=1,
|
||||
away_team_id=2,
|
||||
game_mode="friendly",
|
||||
visibility="public"
|
||||
)
|
||||
await db_session.flush() # Make visible within session
|
||||
|
||||
# Assert
|
||||
game = await db_ops.get_game(game_id)
|
||||
assert game is not None
|
||||
retrieved = await db_ops.get_game(game_id)
|
||||
assert retrieved is not None
|
||||
assert retrieved.id == game_id
|
||||
# Session automatically rolled back after test
|
||||
```
|
||||
|
||||
**Key Pattern Notes**:
|
||||
- Fixtures (`db_ops`, `db_session`) come from `tests/integration/conftest.py`
|
||||
- Use `await db_session.flush()` to persist changes within test (not `commit()`)
|
||||
- Session is automatically rolled back after each test (isolation)
|
||||
- All operations share same session (no connection conflicts)
|
||||
|
||||
### Async Test Pattern
|
||||
|
||||
```python
|
||||
@ -264,88 +278,63 @@ DATABASE_URL=postgresql+asyncpg://paperdynasty:PASSWORD@10.10.0.42:5432/paperdyn
|
||||
- Tests should clean up after themselves (fixtures handle this)
|
||||
- Each test should create unique game IDs to avoid conflicts
|
||||
|
||||
### Transaction Rollback Pattern
|
||||
### Transaction Rollback Pattern (tests/integration/conftest.py)
|
||||
|
||||
```python
|
||||
@pytest.fixture
|
||||
from sqlalchemy import NullPool
|
||||
from sqlalchemy.ext.asyncio import create_async_engine, AsyncSession, async_sessionmaker
|
||||
|
||||
# Use NullPool to prevent connection reuse issues in tests
|
||||
test_engine = create_async_engine(DATABASE_URL, poolclass=NullPool)
|
||||
TestAsyncSessionLocal = async_sessionmaker(test_engine, class_=AsyncSession, expire_on_commit=False)
|
||||
|
||||
@pytest_asyncio.fixture(scope="function")
|
||||
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
|
||||
async with TestAsyncSessionLocal() as session:
|
||||
yield session
|
||||
await session.rollback() # Cleanup - discard all changes
|
||||
|
||||
@pytest_asyncio.fixture(scope="function")
|
||||
async def db_ops(db_session: AsyncSession):
|
||||
"""DatabaseOperations with injected session for test isolation"""
|
||||
return DatabaseOperations(db_session)
|
||||
```
|
||||
|
||||
**Note**: Current fixtures may not properly isolate transactions, contributing to connection conflicts.
|
||||
**Why This Works**:
|
||||
- `NullPool`: Prevents asyncpg connection reuse issues
|
||||
- `scope="function"`: Each test gets fresh session
|
||||
- `rollback()`: Automatically discards all test data
|
||||
- Session injection: All `db_ops` methods use same session (no conflicts)
|
||||
|
||||
## Known Test Issues
|
||||
|
||||
### 1. Player Model Test Failures
|
||||
All major test infrastructure issues have been resolved. The test suite is now stable.
|
||||
|
||||
**Issue**: `tests/unit/models/test_player_models.py` has 13 failures
|
||||
### Historical Context (Resolved)
|
||||
|
||||
**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
|
||||
**AsyncPG Connection Conflicts** - **RESOLVED** (2025-11-27)
|
||||
- **Was**: "cannot perform operation: another operation is in progress" errors
|
||||
- **Fix**: Session injection pattern in `DatabaseOperations` + `NullPool` in test fixtures
|
||||
- **Result**: 32/32 integration tests now passing
|
||||
|
||||
## 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)
|
||||
**Current Status** (as of 2025-11-27):
|
||||
- ✅ **979 unit tests passing** (100%)
|
||||
- ✅ **32 integration tests passing** (100%)
|
||||
- **Total: 1,011 tests passing**
|
||||
|
||||
**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
|
||||
app/config/ ✅ Well covered
|
||||
app/core/game_engine.py ✅ Well covered (unit + integration)
|
||||
app/core/state_manager.py ✅ Well covered
|
||||
app/core/dice.py ✅ Well covered
|
||||
app/models/ ✅ Well covered
|
||||
app/database/operations.py ✅ 32 integration tests (session injection pattern)
|
||||
app/websocket/handlers.py ✅ 148 WebSocket handler tests
|
||||
app/middleware/ ✅ Rate limiting, exceptions tested
|
||||
```
|
||||
|
||||
## Testing Best Practices
|
||||
@ -363,12 +352,12 @@ app/database/operations.py ⚠️ Integration tests have infrastructure issues
|
||||
|
||||
### 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
|
||||
- ❌ Don't use `commit()` in integration tests (use `flush()` - session auto-rollbacks)
|
||||
- ❌ Don't create `DatabaseOperations()` without session injection in integration tests
|
||||
|
||||
### Mocking Examples
|
||||
|
||||
@ -443,25 +432,30 @@ uv run pytest tests/unit/core/test_game_engine.py -v -o log_cli=true -o log_cli_
|
||||
- name: Unit Tests
|
||||
run: uv run pytest tests/unit/ -v --cov=app
|
||||
|
||||
# Run integration tests serially (slower but reliable)
|
||||
# Run integration tests (session injection pattern makes these 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
|
||||
run: uv run pytest tests/integration/database/test_operations.py -v
|
||||
```
|
||||
|
||||
**OR** fix the integration test infrastructure first, then run normally.
|
||||
**All tests can now run together** thanks to session injection pattern.
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
### "cannot perform operation: another operation is in progress"
|
||||
|
||||
**Solution**: Run integration tests individually or fix fixture scopes
|
||||
**This issue is RESOLVED.** If you see it, you're likely:
|
||||
- Creating `DatabaseOperations()` without session injection in tests
|
||||
- Not using the `db_ops` fixture from `tests/integration/conftest.py`
|
||||
|
||||
**Solution**: Use session injection pattern:
|
||||
```python
|
||||
async def test_something(self, db_ops, db_session): # Use fixtures!
|
||||
await db_ops.create_game(...) # Uses injected session
|
||||
```
|
||||
|
||||
### "Task got Future attached to a different loop"
|
||||
|
||||
**Solution**: Ensure all fixtures use `scope="function"` or create proper module-scoped event loop
|
||||
**Solution**: Ensure all fixtures use `scope="function"` (already configured in conftest.py)
|
||||
|
||||
### "No module named 'app'"
|
||||
|
||||
@ -489,40 +483,41 @@ pytest tests/unit/ -v
|
||||
2. `.env` has correct `DATABASE_URL`
|
||||
3. Database exists and schema is migrated: `alembic upgrade head`
|
||||
|
||||
## Integration Test Refactor TODO
|
||||
## Integration Test Architecture (Completed 2025-11-27)
|
||||
|
||||
When refactoring integration tests to fix connection conflicts:
|
||||
The integration test infrastructure has been fully refactored using session injection:
|
||||
|
||||
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
|
||||
### Key Components
|
||||
|
||||
2. **Add connection pooling**:
|
||||
- Consider using separate connection pool for tests
|
||||
- Or create new engine per test (slower but isolated)
|
||||
1. **`tests/integration/conftest.py`**:
|
||||
- `NullPool` engine (prevents connection reuse issues)
|
||||
- `db_session` fixture (function-scoped, auto-rollback)
|
||||
- `db_ops` fixture (`DatabaseOperations` with injected session)
|
||||
|
||||
3. **Add transaction rollback**:
|
||||
- Wrap each test in transaction
|
||||
- Rollback after test completes
|
||||
- Ensures database is clean for next test
|
||||
2. **`app/database/operations.py`**:
|
||||
- Constructor accepts optional `AsyncSession`
|
||||
- `_get_session()` context manager handles both modes
|
||||
- Methods use `flush()` not `commit()`
|
||||
|
||||
4. **Consider pytest-xdist**:
|
||||
- Run tests in parallel with proper worker isolation
|
||||
- Each worker gets own database connection
|
||||
- Faster test execution
|
||||
3. **`app/core/game_engine.py`**:
|
||||
- Creates `DatabaseOperations(session)` for transaction groups
|
||||
- Ensures atomic save_play + update_game_state operations
|
||||
|
||||
5. **Update `test_state_persistence.py`**:
|
||||
- Fix `setup_database` fixture scope mismatch
|
||||
- Consider splitting into smaller fixtures
|
||||
### Pattern Summary
|
||||
|
||||
```
|
||||
Production: db_ops = DatabaseOperations() → Auto-commit per operation
|
||||
Testing: db_ops = DatabaseOperations(session) → Shared session, caller controls
|
||||
Transactions: db_ops = DatabaseOperations(session) → Multiple ops, single commit
|
||||
```
|
||||
|
||||
## Additional Resources
|
||||
|
||||
- **Database CLAUDE.md**: `app/database/CLAUDE.md` - Full session injection documentation
|
||||
- **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.
|
||||
**Summary**: All 1,011 tests passing (979 unit + 32 integration). Session injection pattern ensures reliable, isolated integration tests with automatic cleanup.
|
||||
|
||||
@ -34,7 +34,7 @@ frontend-sba/
|
||||
│ ├── useWebSocket.ts # Connection, event handlers
|
||||
│ └── useGameActions.ts # Game action wrappers
|
||||
├── store/ # See store/CLAUDE.md for patterns
|
||||
│ ├── auth.ts # Discord OAuth, JWT
|
||||
│ ├── auth.ts # Discord OAuth, HttpOnly cookie auth
|
||||
│ ├── game.ts # Game state, lineups, decisions
|
||||
│ └── ui.ts # Toasts, modals
|
||||
├── types/ # See types/CLAUDE.md for mappings
|
||||
|
||||
@ -42,9 +42,40 @@ findPlayerInLineup(lineupId: number): Lineup | undefined
|
||||
```
|
||||
|
||||
### auth.ts - Authentication
|
||||
**Purpose**: Discord OAuth state and JWT token management.
|
||||
**Purpose**: Discord OAuth state with HttpOnly cookie-based authentication.
|
||||
|
||||
**Key State**: `user`, `token`, `isAuthenticated`, `isTokenValid`
|
||||
**Key State**: `currentUser`, `isAuthenticated`
|
||||
|
||||
**Critical Pattern - SSR Cookie Forwarding**:
|
||||
```typescript
|
||||
// checkAuth() must forward cookies during SSR
|
||||
async function checkAuth(): Promise<boolean> {
|
||||
const headers: Record<string, string> = {}
|
||||
if (import.meta.server) {
|
||||
const event = useRequestEvent()
|
||||
const cookieHeader = event?.node.req.headers.cookie
|
||||
if (cookieHeader) {
|
||||
headers['Cookie'] = cookieHeader // Forward to backend
|
||||
}
|
||||
}
|
||||
const response = await $fetch('/api/auth/me', {
|
||||
credentials: 'include',
|
||||
headers,
|
||||
})
|
||||
}
|
||||
```
|
||||
|
||||
**API Calls Pattern**:
|
||||
```typescript
|
||||
// ALL API calls must use credentials: 'include'
|
||||
const response = await $fetch('/api/games/', {
|
||||
credentials: 'include', // Sends HttpOnly cookies
|
||||
})
|
||||
|
||||
// NEVER use Authorization header - tokens are in cookies
|
||||
```
|
||||
|
||||
**Reference**: See `COOKIE_AUTH_IMPLEMENTATION.md` for complete auth flow documentation.
|
||||
|
||||
### ui.ts - UI State
|
||||
**Purpose**: Toasts, modals, loading states.
|
||||
|
||||
Loading…
Reference in New Issue
Block a user