Implement unit tests with mocking for service and route layers

[coderabbitai]

Problem

Current test coverage is limited to integration tests (tests/test_main.py), which test the full stack end-to-end. This approach:

• Misses error paths: Exception handlers in service layer (SQLAlchemyError catches) are not covered (currently 79% coverage in services/player_service.py - lines 42-45, 132-135, 157-160 uncovered)
• Lacks layer isolation: Cannot test route/service logic independently from database
• Harder to debug: Integration test failures don't pinpoint which layer failed
• Slow test execution: Every test hits SQLite database
• Limited edge case testing: Can't easily simulate database failures, timeouts, or constraint violations

Proposed Solution

Add unit tests alongside existing integration tests:

1. Service Layer Unit Tests (tests/test_player_service.py)

   • Mock AsyncSession to test service functions in isolation
   • Cover SQLAlchemyError exception handling paths
   • Test edge cases (null values, constraint violations, rollback behavior)

2. Route Layer Unit Tests (tests/test_player_route.py)

   • Mock service layer functions to test route logic independently
   • Verify HTTP status codes, headers, response formatting
   • Test cache invalidation logic

3. Maintain Integration Tests (tests/test_main.py)

   • Keep existing tests as end-to-end validation
   • Continue testing real database interactions

Result: Comprehensive test pyramid with unit tests (fast, isolated) at the base and integration tests (realistic) at the top.

Suggested Approach

1. Service Layer Unit Tests

File: tests/test_player_service.py

Pattern: Mock AsyncSession with AsyncMock from unittest.mock

Example structure:

import pytest
from unittest.mock import AsyncMock, MagicMock
from sqlalchemy.exc import SQLAlchemyError
from services.player_service import create_async, update_async, delete_async
from tests.player_stub import nonexistent_player, existing_player

@pytest.mark.asyncio
async def test_create_async_success():
    """create_async returns True when player is created successfully"""
    mock_session = AsyncMock()
    mock_session.commit = AsyncMock()
    player = nonexistent_player()
    
    result = await create_async(mock_session, player)
    
    assert result is True
    mock_session.add.assert_called_once()
    mock_session.commit.assert_called_once()

@pytest.mark.asyncio
async def test_create_async_handles_sqlalchemy_error():
    """create_async returns False and rolls back when SQLAlchemyError occurs"""
    mock_session = AsyncMock()
    mock_session.commit.side_effect = SQLAlchemyError("Database constraint violation")
    player = nonexistent_player()
    
    result = await create_async(mock_session, player)
    
    assert result is False
    mock_session.rollback.assert_called_once()

Test Coverage Goals:

• create_async: Success path, SQLAlchemyError handling
• retrieve_all_async: Returns all players, handles empty database
• retrieve_by_id_async: Found, not found cases
• retrieve_by_squad_number_async: Found, not found cases
• update_async: Success path, SQLAlchemyError handling, player not found
• delete_async: Success path, SQLAlchemyError handling, player not found

2. Route Layer Unit Tests

File: tests/test_player_route.py

Pattern: Mock service layer functions, use TestClient with override dependencies

Example structure:

import pytest
from unittest.mock import AsyncMock, patch
from fastapi.testclient import TestClient
from main import app
from tests.player_stub import existing_player

@patch('routes.player_route.player_service.retrieve_all_async')
def test_get_all_async_returns_cached_players(mock_retrieve_all, client):
    """GET /players/ returns players from cache on second request"""
    mock_retrieve_all.return_value = [existing_player()]
    
    # First request - cache miss
    response1 = client.get("/players/")
    # Second request - cache hit
    response2 = client.get("/players/")
    
    assert response1.status_code == 200
    assert response2.status_code == 200
    assert mock_retrieve_all.call_count == 1  # Called only once

Test Coverage Goals:

• Cache behavior: cache miss/hit headers, cache invalidation on POST
• Service call delegation: Verify correct service methods called with correct params
• Error mapping: Service failures mapped to correct HTTP status codes

3. Dependencies & Setup

Add to requirements-test.txt:

pytest-mock>=3.14.0  # For better mocking support

Update conftest.py (if needed for shared mocks)

4. Documentation Updates

Update AGENTS.md:

• Add "Unit Testing" section explaining the mock-based approach
• Document when to use unit vs integration tests
• Add examples of mock patterns used

Update .github/copilot-instructions.md:

• Clarify test layers: "Integration tests in test_main.py, unit tests in test_*_service.py and test_*_route.py"

Acceptance Criteria

• tests/test_player_service.py created with unit tests for all service functions
  • All 6 service functions have success path tests
  • All 3 exception handlers (create, update, delete) have SQLAlchemyError tests
  • Coverage for services/player_service.py reaches 95%+ (covering lines 42-45, 132-135, 157-160)
• tests/test_player_route.py created with unit tests for route layer
  • Cache behavior tests (miss, hit, invalidation)
  • Service call delegation tests
• All tests pass: pytest -v (unit + integration)
• Coverage targets met: pytest --cov shows 90%+ overall
• Documentation updated (AGENTS.md, copilot-instructions.md)
• CI/CD passes with new tests

References

• Current coverage gap: services/player_service.py at 79% (lines 42-45, 132-135, 157-160 uncovered)
• Related PR: refactor(tests): adopt action-oriented naming convention #489 (test naming convention refactor)
• pytest-mock documentation: https://pytest-mock.readthedocs.io/
• unittest.mock.AsyncMock: https://docs.python.org/3/library/unittest.mock.html#unittest.mock.AsyncMock
• FastAPI Testing Guide: https://fastapi.tiangolo.com/tutorial/testing/