Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .coderabbit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ reviews:
- path: "tests/**/*.py"
instructions: |
- Tests should use pytest with fixtures from conftest.py
- Verify test naming follows given_when_then pattern
- Verify test naming follows test_request_{method}_{resource}_{param_or_context}_response_{outcome} pattern
- Check that TestClient is used for endpoint testing
- Ensure test data uses stubs (e.g., player_stub.py)
- Tests should use async test functions where appropriate
Expand Down Expand Up @@ -287,7 +287,8 @@ code_generation:
- path: "tests/**/*.py"
instructions: |
- Use pytest framework with async support (pytest-asyncio)
- Follow given_when_then or arrange_act_assert patterns
- Follow test_request_{method}_{resource}_{param_or_context}_response_{outcome} pattern
- Tests use arrange-act-assert structure within test body
- Use fixtures from conftest.py for TestClient
- Use test stubs for consistent test data
- Ensure async tests are properly decorated
Expand Down
52 changes: 51 additions & 1 deletion .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ Request → Routes → Services → SQLAlchemy → SQLite
```

**Key Directories:**

- `routes/` - API endpoints (player_route.py, health_route.py)
- `services/` - Business logic (player_service.py)
- `models/` - Pydantic validation (camelCase JSON API)
Expand All @@ -56,6 +57,7 @@ Request → Routes → Services → SQLAlchemy → SQLite
- `tests/` - pytest suite (test_main.py, conftest.py)

**Config Files:**

- `.flake8` - Linter (max-line-length=88, complexity=10)
- `pyproject.toml` - Black formatter (line-length=88)
- `.coveragerc` - Coverage config (80% target)
Expand All @@ -65,6 +67,7 @@ Request → Routes → Services → SQLAlchemy → SQLite
## API Endpoints

All async with `AsyncSession` injection:

- `POST /players/` → 201|409|422
- `GET /players/` → 200 (cached 10min)
- `GET /players/{player_id}` → 200|404
Expand All @@ -78,11 +81,13 @@ JSON: camelCase (e.g., `squadNumber`, `firstName`)
## CI/CD

**python-ci.yml** (push/PR to master):

1. Lint: commitlint → `flake8 .` → `black --check .`
2. Test: `pytest -v` → coverage
3. Upload to Codecov

**python-cd.yml** (tags `v*.*.*-*`):

1. Validate semver + coach name
2. Run tests
3. Build Docker (amd64/arm64)
Expand All @@ -92,19 +97,22 @@ JSON: camelCase (e.g., `squadNumber`, `firstName`)
## Critical Patterns

### Async Everywhere

```python
# Always use async/await
async def get_player(async_session: AsyncSession, player_id: int):
stmt = select(Player).where(Player.id == player_id)
result = await async_session.execute(stmt)
return result.scalar_one_or_none()
```

- All routes: `async def`
- Database: `AsyncSession` (never `Session`)
- Driver: `aiosqlite` (not `sqlite3`)
- SQLAlchemy 2.0: `select()` (not `session.query()`)

### camelCase API Contract

```python
class PlayerModel(BaseModel):
model_config = ConfigDict(alias_generator=to_camel)
Expand All @@ -113,14 +121,17 @@ class PlayerModel(BaseModel):
```

### Database Schema Changes

⚠️ No Alembic yet - manual process:

1. Update `schemas/player_schema.py`
2. Manually update `storage/players-sqlite3.db` (SQLite CLI/DB Browser)
3. Preserve 26 players
4. Update `models/player_model.py` if API changes
5. Update services + tests

### Caching

- Key: `"players"` (hardcoded)
- TTL: 600s (10min)
- Cleared on POST/PUT/DELETE
Expand All @@ -129,7 +140,7 @@ class PlayerModel(BaseModel):
## Common Issues

1. **SQLAlchemy errors** → Always catch + rollback in services
2. **Test file** → `test_main.py` excluded from Black (preserves long names)
2. **Test file** → `test_main.py` excluded from Black
3. **Database location** → Local: `./storage/`, Docker: `/storage/` (volume)
4. **Pydantic validation** → Returns 422 (not 400)
5. **Import order** → stdlib → third-party → local
Expand All @@ -155,19 +166,58 @@ curl http://localhost:9000/players # 200 OK
- Line length: 88
- Complexity: ≤10

## Test Naming Convention

Integration tests follow an action-oriented pattern:

**Pattern:**

```text
test_request_{method}_{resource}_{param_or_context}_response_{outcome}
```

**Components:**

- `method` - HTTP verb: `get`, `post`, `put`, `delete`
- `resource` - `players` (collection) or `player` (single resource)
- `param_or_context` - Request details: `id_existing`, `squadnumber_nonexistent`, `body_empty`
- `response` - Literal separator
- `outcome` - What's asserted: `status_ok`, `status_not_found`, `body_players`, `header_cache_miss`

**Examples:**

```python
def test_request_get_players_response_status_ok(client):
"""GET /players/ returns 200 OK"""

def test_request_get_player_id_existing_response_body_player_match(client):
"""GET /players/{player_id} with existing ID returns matching player"""

def test_request_post_player_body_empty_response_status_unprocessable(client):
"""POST /players/ with empty body returns 422 Unprocessable Entity"""
```

**Docstrings:**

- Single-line, concise descriptions
- Complements test name (doesn't repeat)
- No "Expected:" prefix (redundant)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

## Commit Messages

Follow Conventional Commits format (enforced by commitlint in CI):

**Format:** `type(scope): description (#issue)`

**Rules:**

- Max 80 characters
- Types: `feat`, `fix`, `docs`, `test`, `refactor`, `chore`, `ci`, `perf`, `style`, `build`
- Scope: Optional (e.g., `api`, `db`, `service`, `route`)
- Issue number: Required suffix

**Examples:**

```text
feat(api): add player stats endpoint (#42)
fix(db): resolve async session leak (#88)
Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/python-cd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,11 @@ jobs:
else
echo "📝 Generating changelog from $PREVIOUS_TAG to ${{ steps.version.outputs.tag_name }}"
CHANGELOG=$(git log --pretty=format:"- %s (%h)" ${PREVIOUS_TAG}..${{ steps.version.outputs.tag_name }})

# Guard against empty changelog (e.g., re-tagging same commit)
if [ -z "$CHANGELOG" ]; then
CHANGELOG="No new changes since $PREVIOUS_TAG"
fi
fi

# Write changelog to file
Expand Down
17 changes: 15 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ async def get_stats(
1. Tests (`tests/test_main.py`):

```python
async def test_given_get_when_player_stats_then_returns_200():
def test_request_get_player_id_existing_stats_response_status_ok(client):
"""GET /players/{player_id}/stats with existing ID returns 200 OK"""
...
```

Expand Down Expand Up @@ -110,10 +111,22 @@ No Alembic migrations:

## Testing Strategy

**Naming Pattern:**

```text
test_request_{method}_{resource}_{param_or_context}_response_{outcome}
```

- `players` (collection) vs `player` (single resource)
- Examples: `test_request_get_players_response_status_ok`, `test_request_post_player_body_empty_response_status_unprocessable`

**Guidelines:**

- Use `tests/player_stub.py` for data
- Test real DB (fixtures handle setup)
- Cover: happy paths + errors + edges
- Cache tests: verify `X-Cache` header
- Docstrings: Single-line, concise, no "Expected:" prefix

## Planning Tips

Expand All @@ -130,7 +143,7 @@ No Alembic migrations:
- `test_main.py`: Excluded from Black
- SQLAlchemy errors: Catch + rollback in services
- Validation errors: 422 (not 400)
- Test file naming: `test_given_when_then` pattern
- Test naming: `test_request_{method}_{resource}_{context}_response_{outcome}` pattern

## Cross-Tool Notes

Expand Down
Binary file modified storage/players-sqlite3.db
Binary file not shown.
20 changes: 10 additions & 10 deletions tests/player_stub.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,16 @@ def nonexistent_player():
Creates a test stub for a nonexistent (new) Player.
"""
return Player(
id=12,
first_name="Leandro",
middle_name="Daniel",
last_name="Paredes",
date_of_birth="1994-06-29T00:00:00.000Z",
squad_number=5,
position="Defensive Midfield",
abbr_position="DM",
team="AS Roma",
league="Serie A",
id=24,
first_name="Thiago",
middle_name="Ezequiel",
last_name="Almada",
date_of_birth="2001-04-26T00:00:00.000Z",
squad_number=16,
position="Attacking Midfield",
abbr_position="AM",
team="Atlanta United FC",
league="Major League Soccer",
starting11=0,
)

Expand Down
Loading