Skip to content

Commit cd069aa

Browse files
committed
refactor(tests): adopt action-oriented naming convention
Replace BDD-style test names with action-oriented pattern: test_request_{method}_{resource}_{param_or_context}_response_{outcome} Test naming changes: - Rename all 19 integration tests for consistency - Apply players/player resource distinction (collection vs single) - Simplify docstrings (single-line, no \"Expected:\" prefix) Test data updates: - Update nonexistent_player() to ID 24 (Thiago Almada) - Use ID 21 (Alejandro Gómez) for deletion test - Update database: 25 players, ID 24 reserved for creation - GET /players/ test expects 25 players initially Documentation: - Add test naming convention to copilot-instructions.md - Update AGENTS.md with pattern and examples Test flow: - Initial: 25 players (missing ID 24, has ID 21) - POST creates ID 24 → 26 players - DELETE removes ID 21 → 25 players Aligns with multi-language comparison project standards.
1 parent e8863e2 commit cd069aa

5 files changed

Lines changed: 103 additions & 168 deletions

File tree

.github/copilot-instructions.md

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ class PlayerModel(BaseModel):
129129
## Common Issues
130130

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

158+
## Test Naming Convention
159+
160+
Integration tests follow an action-oriented pattern:
161+
162+
**Pattern:**
163+
```
164+
test_request_{method}_{resource}_{param_or_context}_response_{outcome}
165+
```
166+
167+
**Components:**
168+
- `method` - HTTP verb: `get`, `post`, `put`, `delete`
169+
- `resource` - `players` (collection) or `player` (single resource)
170+
- `param_or_context` - Request details: `id_existing`, `squadnumber_nonexistent`, `body_empty`
171+
- `response` - Literal separator
172+
- `outcome` - What's asserted: `status_ok`, `status_not_found`, `body_players`, `header_cache_miss`
173+
174+
**Examples:**
175+
```python
176+
def test_request_get_players_response_status_ok(client):
177+
"""GET /players/ returns 200 OK"""
178+
179+
def test_request_get_player_id_existing_response_body_player_match(client):
180+
"""GET /players/{player_id} with existing ID returns matching player"""
181+
182+
def test_request_post_player_body_empty_response_status_unprocessable(client):
183+
"""POST /players/ with empty body returns 422 Unprocessable Entity"""
184+
```
185+
186+
**Docstrings:**
187+
- Single-line, concise descriptions
188+
- Complements test name (doesn't repeat)
189+
- No "Expected:" prefix (redundant)
190+
158191
## Commit Messages
159192

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

AGENTS.md

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,8 @@ async def get_stats(
7171
1. Tests (`tests/test_main.py`):
7272

7373
```python
74-
async def test_given_get_when_player_stats_then_returns_200():
74+
def test_request_get_player_id_existing_stats_response_status_ok(client):
75+
"""GET /players/{player_id}/stats with existing ID returns 200 OK"""
7576
...
7677
```
7778

@@ -110,10 +111,22 @@ No Alembic migrations:
110111

111112
## Testing Strategy
112113

114+
**Naming Pattern:**
115+
116+
```text
117+
test_request_{method}_{resource}_{param_or_context}_response_{outcome}
118+
```
119+
120+
- `players` (collection) vs `player` (single resource)
121+
- Examples: `test_request_get_players_response_status_ok`, `test_request_post_player_body_empty_response_status_unprocessable`
122+
123+
**Guidelines:**
124+
113125
- Use `tests/player_stub.py` for data
114126
- Test real DB (fixtures handle setup)
115127
- Cover: happy paths + errors + edges
116128
- Cache tests: verify `X-Cache` header
129+
- Docstrings: Single-line, concise, no "Expected:" prefix
117130

118131
## Planning Tips
119132

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

135148
## Cross-Tool Notes
136149

storage/players-sqlite3.db

4 KB
Binary file not shown.

tests/player_stub.py

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,16 +54,16 @@ def nonexistent_player():
5454
Creates a test stub for a nonexistent (new) Player.
5555
"""
5656
return Player(
57-
id=12,
58-
first_name="Leandro",
59-
middle_name="Daniel",
60-
last_name="Paredes",
61-
date_of_birth="1994-06-29T00:00:00.000Z",
62-
squad_number=5,
63-
position="Defensive Midfield",
64-
abbr_position="DM",
65-
team="AS Roma",
66-
league="Serie A",
57+
id=24,
58+
first_name="Thiago",
59+
middle_name="Ezequiel",
60+
last_name="Almada",
61+
date_of_birth="2001-04-26T00:00:00.000Z",
62+
squad_number=16,
63+
position="Attacking Midfield",
64+
abbr_position="AM",
65+
team="Atlanta United FC",
66+
league="Major League Soccer",
6767
starting11=0,
6868
)
6969

0 commit comments

Comments
 (0)