- Fix TypeError in check_steal_opportunity by properly mocking catcher defense - Correct tag_from_third test calculation to account for all adjustment conditions - Fix pitcher replacement test by setting appropriate allowed runners threshold - Add comprehensive test coverage for AI service business logic - Implement VS Code testing panel configuration with pytest integration - Create pytest.ini for consistent test execution and warning management - Add test isolation guidelines and factory pattern implementation - Establish 102 passing tests with zero failures 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
396 lines
9.8 KiB
Markdown
396 lines
9.8 KiB
Markdown
# 🚨 Test Isolation Best Practices Guide
|
|
|
|
**CRITICAL: This guide prevents data persistence issues and test conflicts.**
|
|
|
|
## The Problem We Solved
|
|
|
|
Previously, tests were creating their own database fixtures and using hardcoded IDs, causing:
|
|
- ❌ Data persistence between test runs
|
|
- ❌ Primary key conflicts
|
|
- ❌ Tests depending on execution order
|
|
- ❌ Intermittent test failures
|
|
- ❌ Polluted test database
|
|
|
|
## The Solution: Centralized Fixtures + Factory Pattern
|
|
|
|
### ✅ ALWAYS DO: Use Centralized Database Fixtures
|
|
|
|
**Use the `db_session` fixture from `conftest.py`:**
|
|
|
|
```python
|
|
# ✅ CORRECT
|
|
def test_create_team(db_session):
|
|
team = TeamFactory.create(db_session, abbrev="LAD")
|
|
assert team.id is not None
|
|
```
|
|
|
|
**This fixture provides:**
|
|
- ✅ Automatic transaction rollback after each test
|
|
- ✅ Complete test isolation
|
|
- ✅ Fast execution (no actual database writes)
|
|
- ✅ Deterministic results
|
|
|
|
### ❌ NEVER DO: Create Custom Database Fixtures
|
|
|
|
```python
|
|
# ❌ WRONG - Creates data persistence issues
|
|
@pytest.fixture
|
|
def session(test_db):
|
|
with Session(test_db) as session:
|
|
yield session # No rollback!
|
|
|
|
@pytest.fixture
|
|
def my_custom_session():
|
|
# Custom session logic
|
|
pass
|
|
```
|
|
|
|
**Why this is wrong:**
|
|
- Data persists between tests
|
|
- No automatic cleanup
|
|
- Tests interfere with each other
|
|
- Inconsistent test results
|
|
|
|
### ✅ ALWAYS DO: Use Test Factories
|
|
|
|
**Use factory classes for all test data:**
|
|
|
|
```python
|
|
# ✅ CORRECT
|
|
from tests.factories.team_factory import TeamFactory
|
|
|
|
def test_team_creation(db_session):
|
|
team = TeamFactory.create(db_session, abbrev="BOS")
|
|
assert team.abbrev == "BOS"
|
|
```
|
|
|
|
**Benefits:**
|
|
- ✅ Unique IDs every time
|
|
- ✅ No conflicts between tests
|
|
- ✅ Consistent, valid data
|
|
- ✅ Customizable per test
|
|
|
|
### ❌ NEVER DO: Manual Model Creation with Hardcoded IDs
|
|
|
|
```python
|
|
# ❌ WRONG - Hardcoded IDs cause conflicts
|
|
def test_bad_team_creation(db_session):
|
|
team = Team(
|
|
id=1, # ❌ Will conflict with other tests
|
|
abbrev="TST",
|
|
lname="Test Team",
|
|
# ... many required fields
|
|
)
|
|
db_session.add(team)
|
|
db_session.commit()
|
|
```
|
|
|
|
**Why this is wrong:**
|
|
- Primary key conflicts between tests
|
|
- Brittle when test data requirements change
|
|
- Verbose and hard to maintain
|
|
- No guarantee of unique data
|
|
|
|
## Detailed Implementation Guide
|
|
|
|
### 1. Database Session Usage
|
|
|
|
**✅ CORRECT Pattern:**
|
|
```python
|
|
def test_something(db_session): # Parameter name must be 'db_session'
|
|
# Create test data using factories
|
|
team = TeamFactory.create(db_session, abbrev="TEST")
|
|
|
|
# Perform test operations
|
|
result = some_service_operation(team)
|
|
|
|
# Make assertions
|
|
assert result is not None
|
|
|
|
# No cleanup needed - automatic rollback
|
|
```
|
|
|
|
**❌ WRONG Patterns:**
|
|
```python
|
|
# Don't define custom fixtures
|
|
@pytest.fixture
|
|
def session():
|
|
pass
|
|
|
|
# Don't use different parameter names
|
|
def test_something(custom_session):
|
|
pass
|
|
|
|
# Don't create sessions manually
|
|
def test_something():
|
|
with Session(engine) as session:
|
|
pass
|
|
```
|
|
|
|
### 2. Factory Usage Patterns
|
|
|
|
**✅ CORRECT Factory Usage:**
|
|
```python
|
|
# Basic creation
|
|
team = TeamFactory.create(db_session, abbrev="LAD")
|
|
|
|
# Custom values
|
|
ai_team = TeamFactory.create(db_session, is_ai=True, wallet=100000)
|
|
|
|
# Specialized methods
|
|
ai_team = TeamFactory.build_ai_team()
|
|
human_team = TeamFactory.build_human_team()
|
|
|
|
# Multiple objects
|
|
teams = TeamFactory.build_multiple(3, season=9)
|
|
```
|
|
|
|
**❌ WRONG Manual Creation:**
|
|
```python
|
|
# Don't create models manually
|
|
team = Team(id=1, abbrev="TST", ...)
|
|
|
|
# Don't use non-unique values
|
|
team1 = Team(id=100, abbrev="SAME")
|
|
team2 = Team(id=100, abbrev="SAME") # Conflict!
|
|
|
|
# Don't skip required fields
|
|
team = Team(abbrev="TST") # Missing required fields
|
|
```
|
|
|
|
### 3. Test Structure Template
|
|
|
|
**Use this template for all new database tests:**
|
|
|
|
```python
|
|
"""
|
|
Test module for [functionality].
|
|
|
|
Tests [describe what is being tested].
|
|
"""
|
|
|
|
import pytest
|
|
from tests.factories.team_factory import TeamFactory
|
|
from tests.factories.cardset_factory import CardsetFactory
|
|
# Import other factories as needed
|
|
|
|
class TestSomeFunctionality:
|
|
"""Test [specific functionality]."""
|
|
|
|
def test_basic_case(self, db_session):
|
|
"""Test basic functionality works."""
|
|
# Arrange - create test data
|
|
team = TeamFactory.create(db_session, abbrev="TEST")
|
|
|
|
# Act - perform operation
|
|
result = perform_operation(team)
|
|
|
|
# Assert - verify results
|
|
assert result.success is True
|
|
|
|
def test_edge_case(self, db_session):
|
|
"""Test edge case handling."""
|
|
# Arrange
|
|
special_team = TeamFactory.create(
|
|
db_session,
|
|
is_ai=True,
|
|
wallet=0 # Edge case: no money
|
|
)
|
|
|
|
# Act & Assert
|
|
with pytest.raises(InsufficientFundsError):
|
|
perform_expensive_operation(special_team)
|
|
|
|
def test_multiple_objects(self, db_session):
|
|
"""Test with multiple related objects."""
|
|
# Arrange
|
|
teams = TeamFactory.build_multiple(3)
|
|
cardset = CardsetFactory.create(db_session, ranked_legal=True)
|
|
|
|
for team in teams:
|
|
db_session.add(team)
|
|
db_session.commit()
|
|
|
|
# Act
|
|
result = operation_with_multiple_teams(teams, cardset)
|
|
|
|
# Assert
|
|
assert len(result) == 3
|
|
```
|
|
|
|
## Common Scenarios and Solutions
|
|
|
|
### Scenario 1: Testing Team Creation
|
|
|
|
**✅ CORRECT:**
|
|
```python
|
|
def test_create_team(db_session):
|
|
team = TeamFactory.create(
|
|
db_session,
|
|
abbrev="LAD",
|
|
lname="Los Angeles Dodgers",
|
|
wallet=50000
|
|
)
|
|
|
|
assert team.id is not None
|
|
assert team.abbrev == "LAD"
|
|
assert team.wallet == 50000
|
|
```
|
|
|
|
**❌ WRONG:**
|
|
```python
|
|
def test_create_team(db_session):
|
|
team = Team(
|
|
id=1, # Hardcoded ID
|
|
abbrev="LAD",
|
|
lname="Los Angeles Dodgers",
|
|
gmid=100, # More hardcoded values
|
|
# ... many required fields
|
|
)
|
|
db_session.add(team)
|
|
db_session.commit()
|
|
```
|
|
|
|
### Scenario 2: Testing with Related Objects
|
|
|
|
**✅ CORRECT:**
|
|
```python
|
|
def test_game_with_teams(db_session):
|
|
home_team = TeamFactory.create(db_session, abbrev="HOME")
|
|
away_team = TeamFactory.create(db_session, abbrev="AWAY")
|
|
cardset = CardsetFactory.create(db_session, ranked_legal=True)
|
|
|
|
# Each object has unique ID automatically
|
|
game = create_game(home_team, away_team, cardset)
|
|
assert game.home_team_id == home_team.id
|
|
```
|
|
|
|
**❌ WRONG:**
|
|
```python
|
|
def test_game_with_teams(db_session):
|
|
home_team = Team(id=1, abbrev="HOME", ...)
|
|
away_team = Team(id=2, abbrev="AWAY", ...)
|
|
# Verbose and error-prone
|
|
```
|
|
|
|
### Scenario 3: Testing AI Behavior
|
|
|
|
**✅ CORRECT:**
|
|
```python
|
|
def test_ai_decision_making(db_session):
|
|
aggressive_ai = ManagerAiFactory.create_aggressive(db_session)
|
|
conservative_ai = ManagerAiFactory.create_conservative(db_session)
|
|
|
|
# Test different AI personalities
|
|
agg_decision = aggressive_ai.make_decision(situation)
|
|
cons_decision = conservative_ai.make_decision(situation)
|
|
|
|
assert agg_decision.risk_level > cons_decision.risk_level
|
|
```
|
|
|
|
**❌ WRONG:**
|
|
```python
|
|
def test_ai_decision_making(db_session):
|
|
ai1 = ManagerAi(id=1, steal=10, running=10, ...)
|
|
ai2 = ManagerAi(id=2, steal=2, running=2, ...)
|
|
# Manual setup of complex objects
|
|
```
|
|
|
|
## Verification Checklist
|
|
|
|
Before submitting any test that uses the database, verify:
|
|
|
|
### ✅ Fixture Usage
|
|
- [ ] Uses `db_session` parameter from `conftest.py`
|
|
- [ ] Does NOT define custom session fixtures
|
|
- [ ] Does NOT use `session`, `test_db`, or other custom names
|
|
|
|
### ✅ Factory Usage
|
|
- [ ] Imports factories from `tests.factories`
|
|
- [ ] Uses `Factory.create()` or `Factory.build()` methods
|
|
- [ ] Does NOT create models with `Model(id=hardcoded_value)`
|
|
- [ ] Does NOT use static/hardcoded values that could conflict
|
|
|
|
### ✅ Test Isolation
|
|
- [ ] Test can be run independently
|
|
- [ ] Test can be run multiple times without failure
|
|
- [ ] Test does not depend on execution order
|
|
- [ ] Test does not modify shared state
|
|
|
|
### ✅ Data Cleanup
|
|
- [ ] No manual cleanup code needed
|
|
- [ ] Relies on automatic transaction rollback
|
|
- [ ] Does not call `session.rollback()` manually
|
|
|
|
## Debugging Test Isolation Issues
|
|
|
|
### Problem: Tests pass individually but fail when run together
|
|
|
|
**Diagnosis:**
|
|
```bash
|
|
# Run individual test
|
|
pytest tests/unit/models/test_team.py::test_create_team -v # ✅ Passes
|
|
|
|
# Run all tests
|
|
pytest tests/unit/models/test_team.py -v # ❌ Fails
|
|
```
|
|
|
|
**Likely Causes:**
|
|
1. Using hardcoded IDs that conflict
|
|
2. Not using the `db_session` fixture
|
|
3. Sharing mutable state between tests
|
|
4. Custom fixtures without proper cleanup
|
|
|
|
**Solution:**
|
|
1. Check all model creation uses factories
|
|
2. Verify `db_session` fixture usage
|
|
3. Ensure unique IDs via `generate_unique_id()`
|
|
|
|
### Problem: "duplicate key value violates unique constraint"
|
|
|
|
**Error Message:**
|
|
```
|
|
IntegrityError: (psycopg2.errors.UniqueViolation) duplicate key value violates unique constraint "team_pkey"
|
|
DETAIL: Key (id)=(1) already exists.
|
|
```
|
|
|
|
**Cause:** Using hardcoded IDs instead of factory-generated unique IDs
|
|
|
|
**Solution:**
|
|
```python
|
|
# ❌ WRONG
|
|
team = Team(id=1, ...)
|
|
|
|
# ✅ CORRECT
|
|
team = TeamFactory.create(db_session, ...)
|
|
```
|
|
|
|
### Problem: Tests find unexpected data
|
|
|
|
**Symptom:**
|
|
```python
|
|
# Expected 1 cardset, found 8
|
|
assert len(cardsets) == 1 # Fails: found old data
|
|
```
|
|
|
|
**Cause:** Previous tests didn't use transaction rollback
|
|
|
|
**Solution:**
|
|
1. Clean test database: `TRUNCATE TABLE cardset CASCADE`
|
|
2. Fix all tests to use `db_session` fixture
|
|
3. Verify proper transaction rollback
|
|
|
|
## Summary: The Two Golden Rules
|
|
|
|
### 🥇 Rule #1: Always Use `db_session` Fixture
|
|
```python
|
|
def test_anything_with_database(db_session): # ✅ CORRECT
|
|
pass
|
|
```
|
|
|
|
### 🥇 Rule #2: Always Use Factory Classes
|
|
```python
|
|
team = TeamFactory.create(db_session, custom_field="value") # ✅ CORRECT
|
|
```
|
|
|
|
Following these two rules prevents 99% of test isolation issues and ensures reliable, maintainable tests. |