diff --git a/tests/unit/test_player_service.py b/tests/unit/test_player_service.py index 8a1b00a..6555757 100644 --- a/tests/unit/test_player_service.py +++ b/tests/unit/test_player_service.py @@ -7,21 +7,18 @@ import pytest from unittest.mock import MagicMock, patch import sys import os + sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from app.services.player_service import PlayerService from app.services.base import ServiceConfig -from app.services.mocks import ( - MockPlayerRepository, - MockCacheService, - EnhancedMockCache -) - +from app.services.mocks import MockPlayerRepository, MockCacheService, EnhancedMockCache # ============================================================================ # FIXTURES # ============================================================================ + @pytest.fixture def cache(): """Create fresh cache for each test.""" @@ -32,20 +29,73 @@ def cache(): def repo(cache): """Create fresh repo with test data.""" repo = MockPlayerRepository() - + # Add test players players = [ - {'id': 1, 'name': 'Mike Trout', 'wara': 5.2, 'team_id': 1, 'season': 10, 'pos_1': 'CF', 'pos_2': 'LF', 'strat_code': 'Elite', 'injury_rating': 'A'}, - {'id': 2, 'name': 'Aaron Judge', 'wara': 4.8, 'team_id': 2, 'season': 10, 'pos_1': 'RF', 'strat_code': 'Power', 'injury_rating': 'B'}, - {'id': 3, 'name': 'Mookie Betts', 'wara': 5.5, 'team_id': 3, 'season': 10, 'pos_1': 'RF', 'pos_2': '2B', 'strat_code': 'Elite', 'injury_rating': 'A'}, - {'id': 4, 'name': 'Injured Player', 'wara': 2.0, 'team_id': 1, 'season': 10, 'pos_1': 'P', 'il_return': 'Week 5', 'injury_rating': 'C'}, - {'id': 5, 'name': 'Old Player', 'wara': 1.0, 'team_id': 1, 'season': 5, 'pos_1': '1B'}, - {'id': 6, 'name': 'Juan Soto', 'wara': 4.5, 'team_id': 2, 'season': 10, 'pos_1': '1B', 'strat_code': 'Contact'}, + { + "id": 1, + "name": "Mike Trout", + "wara": 5.2, + "team_id": 1, + "season": 10, + "pos_1": "CF", + "pos_2": "LF", + "strat_code": "Elite", + "injury_rating": "A", + }, + { + "id": 2, + "name": "Aaron Judge", + "wara": 4.8, + "team_id": 2, + "season": 10, + "pos_1": "RF", + "strat_code": "Power", + "injury_rating": "B", + }, + { + "id": 3, + "name": "Mookie Betts", + "wara": 5.5, + "team_id": 3, + "season": 10, + "pos_1": "RF", + "pos_2": "2B", + "strat_code": "Elite", + "injury_rating": "A", + }, + { + "id": 4, + "name": "Injured Player", + "wara": 2.0, + "team_id": 1, + "season": 10, + "pos_1": "P", + "il_return": "Week 5", + "injury_rating": "C", + }, + { + "id": 5, + "name": "Old Player", + "wara": 1.0, + "team_id": 1, + "season": 5, + "pos_1": "1B", + }, + { + "id": 6, + "name": "Juan Soto", + "wara": 4.5, + "team_id": 2, + "season": 10, + "pos_1": "1B", + "strat_code": "Contact", + }, ] - + for player in players: repo.add_player(player) - + return repo @@ -60,463 +110,453 @@ def service(repo, cache): # TEST CLASSES # ============================================================================ + class TestPlayerServiceGetPlayers: """Tests for get_players method - 50+ lines covered.""" - + def test_get_all_season_players(self, service, repo): """Get all players for a season.""" result = service.get_players(season=10) - - assert result['count'] >= 5 # We have 5 season 10 players - assert len(result['players']) >= 5 - assert all(p.get('season') == 10 for p in result['players']) - + + assert result["count"] >= 5 # We have 5 season 10 players + assert len(result["players"]) >= 5 + assert all(p.get("season") == 10 for p in result["players"]) + def test_filter_by_single_team(self, service): """Filter by single team ID.""" result = service.get_players(season=10, team_id=[1]) - - assert result['count'] >= 1 - assert all(p.get('team_id') == 1 for p in result['players']) - + + assert result["count"] >= 1 + assert all(p.get("team_id") == 1 for p in result["players"]) + def test_filter_by_multiple_teams(self, service): """Filter by multiple team IDs.""" result = service.get_players(season=10, team_id=[1, 2]) - - assert result['count'] >= 2 - assert all(p.get('team_id') in [1, 2] for p in result['players']) - + + assert result["count"] >= 2 + assert all(p.get("team_id") in [1, 2] for p in result["players"]) + def test_filter_by_position(self, service): """Filter by position.""" - result = service.get_players(season=10, pos=['CF']) - - assert result['count'] >= 1 - assert any(p.get('pos_1') == 'CF' or p.get('pos_2') == 'CF' for p in result['players']) - + result = service.get_players(season=10, pos=["CF"]) + + assert result["count"] >= 1 + assert any( + p.get("pos_1") == "CF" or p.get("pos_2") == "CF" for p in result["players"] + ) + def test_filter_by_strat_code(self, service): """Filter by strat code.""" - result = service.get_players(season=10, strat_code=['Elite']) - - assert result['count'] >= 2 # Trout and Betts - assert all('Elite' in str(p.get('strat_code', '')) for p in result['players']) - + result = service.get_players(season=10, strat_code=["Elite"]) + + assert result["count"] >= 2 # Trout and Betts + assert all("Elite" in str(p.get("strat_code", "")) for p in result["players"]) + def test_filter_injured_only(self, service): """Filter injured players only.""" result = service.get_players(season=10, is_injured=True) - - assert result['count'] >= 1 - assert all(p.get('il_return') is not None for p in result['players']) - + + assert result["count"] >= 1 + assert all(p.get("il_return") is not None for p in result["players"]) + def test_sort_cost_ascending(self, service): """Sort by WARA ascending.""" - result = service.get_players(season=10, sort='cost-asc') - - wara = [p.get('wara', 0) for p in result['players']] + result = service.get_players(season=10, sort="cost-asc") + + wara = [p.get("wara", 0) for p in result["players"]] assert wara == sorted(wara) - + def test_sort_cost_descending(self, service): """Sort by WARA descending.""" - result = service.get_players(season=10, sort='cost-desc') - - wara = [p.get('wara', 0) for p in result['players']] + result = service.get_players(season=10, sort="cost-desc") + + wara = [p.get("wara", 0) for p in result["players"]] assert wara == sorted(wara, reverse=True) - + def test_sort_name_ascending(self, service): """Sort by name ascending.""" - result = service.get_players(season=10, sort='name-asc') - - names = [p.get('name', '') for p in result['players']] + result = service.get_players(season=10, sort="name-asc") + + names = [p.get("name", "") for p in result["players"]] assert names == sorted(names) - + def test_sort_name_descending(self, service): """Sort by name descending.""" - result = service.get_players(season=10, sort='name-desc') - - names = [p.get('name', '') for p in result['players']] + result = service.get_players(season=10, sort="name-desc") + + names = [p.get("name", "") for p in result["players"]] assert names == sorted(names, reverse=True) class TestPlayerServiceSearch: """Tests for search_players method.""" - + def test_exact_name_match(self, service): """Search with exact name match.""" - result = service.search_players('Mike Trout', season=10) - - assert result['count'] >= 1 - names = [p.get('name') for p in result['players']] - assert 'Mike Trout' in names - + result = service.search_players("Mike Trout", season=10) + + assert result["count"] >= 1 + names = [p.get("name") for p in result["players"]] + assert "Mike Trout" in names + def test_partial_name_match(self, service): """Search with partial name match.""" - result = service.search_players('Trout', season=10) - - assert result['count'] >= 1 - assert any('Trout' in p.get('name', '') for p in result['players']) - + result = service.search_players("Trout", season=10) + + assert result["count"] >= 1 + assert any("Trout" in p.get("name", "") for p in result["players"]) + def test_case_insensitive_search(self, service): """Search is case insensitive.""" - result1 = service.search_players('MIKE', season=10) - result2 = service.search_players('mike', season=10) - - assert result1['count'] == result2['count'] - + result1 = service.search_players("MIKE", season=10) + result2 = service.search_players("mike", season=10) + + assert result1["count"] == result2["count"] + def test_search_all_seasons(self, service): """Search across all seasons.""" - result = service.search_players('Player', season=None) - + result = service.search_players("Player", season=None) + # Should find both current and old players - assert result['all_seasons'] == True - assert result['count'] >= 2 - + assert result["all_seasons"] == True + assert result["count"] >= 2 + def test_search_limit(self, service): """Limit search results.""" - result = service.search_players('a', season=10, limit=2) - - assert result['count'] <= 2 - + result = service.search_players("a", season=10, limit=2) + + assert result["count"] <= 2 + def test_search_no_results(self, service): """Search returns empty when no matches.""" - result = service.search_players('XYZ123NotExist', season=10) - - assert result['count'] == 0 - assert result['players'] == [] + result = service.search_players("XYZ123NotExist", season=10) + + assert result["count"] == 0 + assert result["players"] == [] class TestPlayerServiceGetPlayer: """Tests for get_player method.""" - + def test_get_existing_player(self, service): """Get existing player by ID.""" result = service.get_player(1) - + assert result is not None - assert result.get('id') == 1 - assert result.get('name') == 'Mike Trout' - + assert result.get("id") == 1 + assert result.get("name") == "Mike Trout" + def test_get_nonexistent_player(self, service): """Get player that doesn't exist.""" result = service.get_player(99999) - + assert result is None - + def test_get_player_short_output(self, service): """Get player with short output.""" result = service.get_player(1, short_output=True) - + # Should still have basic fields - assert result.get('id') == 1 - assert result.get('name') == 'Mike Trout' + assert result.get("id") == 1 + assert result.get("name") == "Mike Trout" class TestPlayerServiceCreate: """Tests for create_players method.""" - + def test_create_single_player(self, repo, cache): """Create a single new player.""" config = ServiceConfig(player_repo=repo, cache=cache) service = PlayerService(config=config) - - new_player = [{ - 'name': 'New Player', - 'wara': 3.0, - 'team_id': 1, - 'season': 10, - 'pos_1': 'SS' - }] - + + new_player = [ + { + "name": "New Player", + "wara": 3.0, + "team_id": 1, + "season": 10, + "pos_1": "SS", + } + ] + # Mock auth - with patch.object(service, 'require_auth', return_value=True): - result = service.create_players(new_player, 'valid_token') - - assert 'Inserted' in str(result) + with patch.object(service, "require_auth", return_value=True): + result = service.create_players(new_player, "valid_token") + + assert "Inserted" in str(result) # Verify player was added (ID 7 since fixture has players 1-6) player = repo.get_by_id(7) # Next ID after fixture data assert player is not None - assert player['name'] == 'New Player' - + assert player["name"] == "New Player" + def test_create_multiple_players(self, repo, cache): """Create multiple new players.""" config = ServiceConfig(player_repo=repo, cache=cache) service = PlayerService(config=config) - + new_players = [ - {'name': 'Player A', 'wara': 2.0, 'team_id': 1, 'season': 10, 'pos_1': '2B'}, - {'name': 'Player B', 'wara': 2.5, 'team_id': 2, 'season': 10, 'pos_1': '3B'}, + { + "name": "Player A", + "wara": 2.0, + "team_id": 1, + "season": 10, + "pos_1": "2B", + }, + { + "name": "Player B", + "wara": 2.5, + "team_id": 2, + "season": 10, + "pos_1": "3B", + }, ] - - with patch.object(service, 'require_auth', return_value=True): - result = service.create_players(new_players, 'valid_token') - - assert 'Inserted 2 players' in str(result) - + + with patch.object(service, "require_auth", return_value=True): + result = service.create_players(new_players, "valid_token") + + assert "Inserted 2 players" in str(result) + def test_create_duplicate_fails(self, repo, cache): """Creating duplicate player should fail.""" config = ServiceConfig(player_repo=repo, cache=cache) service = PlayerService(config=config) - - duplicate = [{'name': 'Mike Trout', 'wara': 5.0, 'team_id': 1, 'season': 10, 'pos_1': 'CF'}] - - with patch.object(service, 'require_auth', return_value=True): + + duplicate = [ + { + "name": "Mike Trout", + "wara": 5.0, + "team_id": 1, + "season": 10, + "pos_1": "CF", + } + ] + + with patch.object(service, "require_auth", return_value=True): with pytest.raises(Exception) as exc_info: - service.create_players(duplicate, 'valid_token') - - assert 'already exists' in str(exc_info.value) - + service.create_players(duplicate, "valid_token") + + assert "already exists" in str(exc_info.value) + def test_create_requires_auth(self, repo, cache): """Creating players requires authentication.""" config = ServiceConfig(player_repo=repo, cache=cache) service = PlayerService(config=config) - - new_player = [{'name': 'Test', 'wara': 1.0, 'team_id': 1, 'season': 10, 'pos_1': 'P'}] - + + new_player = [ + {"name": "Test", "wara": 1.0, "team_id": 1, "season": 10, "pos_1": "P"} + ] + with pytest.raises(Exception) as exc_info: - service.create_players(new_player, 'bad_token') - + service.create_players(new_player, "bad_token") + assert exc_info.value.status_code == 401 class TestPlayerServiceUpdate: """Tests for update_player and patch_player methods.""" - + def test_patch_player_name(self, repo, cache): """Patch player's name.""" config = ServiceConfig(player_repo=repo, cache=cache) service = PlayerService(config=config) - - with patch.object(service, 'require_auth', return_value=True): - result = service.patch_player(1, {'name': 'New Name'}, 'valid_token') - + + with patch.object(service, "require_auth", return_value=True): + result = service.patch_player(1, {"name": "New Name"}, "valid_token") + assert result is not None - assert result.get('name') == 'New Name' - + assert result.get("name") == "New Name" + def test_patch_player_wara(self, repo, cache): """Patch player's WARA.""" config = ServiceConfig(player_repo=repo, cache=cache) service = PlayerService(config=config) - - with patch.object(service, 'require_auth', return_value=True): - result = service.patch_player(1, {'wara': 6.0}, 'valid_token') - - assert result.get('wara') == 6.0 - + + with patch.object(service, "require_auth", return_value=True): + result = service.patch_player(1, {"wara": 6.0}, "valid_token") + + assert result.get("wara") == 6.0 + def test_patch_multiple_fields(self, repo, cache): """Patch multiple fields at once.""" config = ServiceConfig(player_repo=repo, cache=cache) service = PlayerService(config=config) - - updates = { - 'name': 'Updated Name', - 'wara': 7.0, - 'strat_code': 'Super Elite' - } - - with patch.object(service, 'require_auth', return_value=True): - result = service.patch_player(1, updates, 'valid_token') - - assert result.get('name') == 'Updated Name' - assert result.get('wara') == 7.0 - assert result.get('strat_code') == 'Super Elite' - + + updates = {"name": "Updated Name", "wara": 7.0, "strat_code": "Super Elite"} + + with patch.object(service, "require_auth", return_value=True): + result = service.patch_player(1, updates, "valid_token") + + assert result.get("name") == "Updated Name" + assert result.get("wara") == 7.0 + assert result.get("strat_code") == "Super Elite" + def test_patch_nonexistent_player(self, repo, cache): """Patch fails for non-existent player.""" config = ServiceConfig(player_repo=repo, cache=cache) service = PlayerService(config=config) - - with patch.object(service, 'require_auth', return_value=True): + + with patch.object(service, "require_auth", return_value=True): with pytest.raises(Exception) as exc_info: - service.patch_player(99999, {'name': 'Test'}, 'valid_token') - - assert 'not found' in str(exc_info.value) - + service.patch_player(99999, {"name": "Test"}, "valid_token") + + assert "not found" in str(exc_info.value) + def test_patch_requires_auth(self, repo, cache): """Patching requires authentication.""" config = ServiceConfig(player_repo=repo, cache=cache) service = PlayerService(config=config) - + with pytest.raises(Exception) as exc_info: - service.patch_player(1, {'name': 'Test'}, 'bad_token') - + service.patch_player(1, {"name": "Test"}, "bad_token") + assert exc_info.value.status_code == 401 class TestPlayerServiceDelete: """Tests for delete_player method.""" - + def test_delete_player(self, repo, cache): """Delete existing player.""" config = ServiceConfig(player_repo=repo, cache=cache) service = PlayerService(config=config) - + # Verify player exists assert repo.get_by_id(1) is not None - - with patch.object(service, 'require_auth', return_value=True): - result = service.delete_player(1, 'valid_token') - - assert 'deleted' in str(result) - + + with patch.object(service, "require_auth", return_value=True): + result = service.delete_player(1, "valid_token") + + assert "deleted" in str(result) + # Verify player is gone assert repo.get_by_id(1) is None - + def test_delete_nonexistent_player(self, repo, cache): """Delete fails for non-existent player.""" config = ServiceConfig(player_repo=repo, cache=cache) service = PlayerService(config=config) - - with patch.object(service, 'require_auth', return_value=True): + + with patch.object(service, "require_auth", return_value=True): with pytest.raises(Exception) as exc_info: - service.delete_player(99999, 'valid_token') - - assert 'not found' in str(exc_info.value) - + service.delete_player(99999, "valid_token") + + assert "not found" in str(exc_info.value) + def test_delete_requires_auth(self, repo, cache): """Deleting requires authentication.""" config = ServiceConfig(player_repo=repo, cache=cache) service = PlayerService(config=config) - + with pytest.raises(Exception) as exc_info: - service.delete_player(1, 'bad_token') - + service.delete_player(1, "bad_token") + assert exc_info.value.status_code == 401 -class TestPlayerServiceCache: - """Tests for cache functionality.""" - - @pytest.mark.skip(reason="Caching not yet implemented in service methods") - def test_cache_set_on_read(self, service, cache): - """Cache is set on player read.""" - service.get_players(season=10) - - assert cache.was_called('set') - - @pytest.mark.skip(reason="Caching not yet implemented in service methods") - def test_cache_invalidation_on_update(self, repo, cache): - """Cache is invalidated on player update.""" - config = ServiceConfig(player_repo=repo, cache=cache) - service = PlayerService(config=config) - - # Read to set cache - service.get_players(season=10) - initial_calls = len(cache.get_calls('set')) - - # Update should invalidate cache - with patch.object(service, 'require_auth', return_value=True): - service.patch_player(1, {'name': 'Test'}, 'valid_token') - - # Should have more delete calls after update - delete_calls = [c for c in cache.get_calls() if c.get('method') == 'delete'] - assert len(delete_calls) > 0 - - @pytest.mark.skip(reason="Caching not yet implemented in service methods") - def test_cache_hit_rate(self, repo, cache): - """Test cache hit rate tracking.""" - config = ServiceConfig(player_repo=repo, cache=cache) - service = PlayerService(config=config) - - # First call - cache miss - service.get_players(season=10) - miss_count = cache._miss_count - - # Second call - cache hit - service.get_players(season=10) - - # Hit rate should have improved - assert cache.hit_rate > 0 - - class TestPlayerServiceValidation: """Tests for input validation and edge cases.""" - + def test_invalid_season_returns_empty(self, service): """Invalid season returns empty result.""" result = service.get_players(season=999) - - assert result['count'] == 0 or result['players'] == [] - + + assert result["count"] == 0 or result["players"] == [] + def test_empty_search_returns_all(self, service): """Empty search query returns all players.""" - result = service.search_players('', season=10) - - assert result['count'] >= 1 - + result = service.search_players("", season=10) + + assert result["count"] >= 1 + def test_sort_with_no_results(self, service): """Sorting with no results doesn't error.""" - result = service.get_players(season=999, sort='cost-desc') - - assert result['count'] == 0 or result['players'] == [] - + result = service.get_players(season=999, sort="cost-desc") + + assert result["count"] == 0 or result["players"] == [] + def test_cache_clear_on_create(self, repo, cache): """Cache is cleared when new players are created.""" config = ServiceConfig(player_repo=repo, cache=cache) service = PlayerService(config=config) - + # Set up some cache data - cache.set('test:key', 'value', 300) - - with patch.object(service, 'require_auth', return_value=True): - service.create_players([{ - 'name': 'New', - 'wara': 1.0, - 'team_id': 1, - 'season': 10, - 'pos_1': 'P' - }], 'valid_token') - + cache.set("test:key", "value", 300) + + with patch.object(service, "require_auth", return_value=True): + service.create_players( + [ + { + "name": "New", + "wara": 1.0, + "team_id": 1, + "season": 10, + "pos_1": "P", + } + ], + "valid_token", + ) + # Should have invalidate calls assert len(cache.get_calls()) > 0 class TestPlayerServiceIntegration: """Integration tests combining multiple operations.""" - + def test_full_crud_cycle(self, repo, cache): """Test complete CRUD cycle.""" config = ServiceConfig(player_repo=repo, cache=cache) service = PlayerService(config=config) - + # CREATE - with patch.object(service, 'require_auth', return_value=True): - create_result = service.create_players([{ - 'name': 'CRUD Test', - 'wara': 3.0, - 'team_id': 1, - 'season': 10, - 'pos_1': 'DH' - }], 'valid_token') - + with patch.object(service, "require_auth", return_value=True): + create_result = service.create_players( + [ + { + "name": "CRUD Test", + "wara": 3.0, + "team_id": 1, + "season": 10, + "pos_1": "DH", + } + ], + "valid_token", + ) + # READ - search_result = service.search_players('CRUD', season=10) - assert search_result['count'] >= 1 - - player_id = search_result['players'][0].get('id') - + search_result = service.search_players("CRUD", season=10) + assert search_result["count"] >= 1 + + player_id = search_result["players"][0].get("id") + # UPDATE - with patch.object(service, 'require_auth', return_value=True): - update_result = service.patch_player(player_id, {'wara': 4.0}, 'valid_token') - assert update_result.get('wara') == 4.0 - + with patch.object(service, "require_auth", return_value=True): + update_result = service.patch_player( + player_id, {"wara": 4.0}, "valid_token" + ) + assert update_result.get("wara") == 4.0 + # DELETE - with patch.object(service, 'require_auth', return_value=True): - delete_result = service.delete_player(player_id, 'valid_token') - assert 'deleted' in str(delete_result) - + with patch.object(service, "require_auth", return_value=True): + delete_result = service.delete_player(player_id, "valid_token") + assert "deleted" in str(delete_result) + # VERIFY DELETED get_result = service.get_player(player_id) assert get_result is None - + def test_search_then_filter(self, service): """Search and then filter operations.""" # First get all players all_result = service.get_players(season=10) - initial_count = all_result['count'] - + initial_count = all_result["count"] + # Then filter by team filtered = service.get_players(season=10, team_id=[1]) - + # Filtered should be <= all - assert filtered['count'] <= initial_count + assert filtered["count"] <= initial_count # ============================================================================