From b480120731387d63cc42cf752ecab67e2ecab879 Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Fri, 20 Mar 2026 10:02:11 -0500 Subject: [PATCH] perf: parallelize schedule_service week fetches with asyncio.gather (#88) Closes #88 Replaced sequential for-loops in get_team_schedule(), get_recent_games(), and get_upcoming_games() with asyncio.gather() to fire all per-week HTTP requests concurrently. Also adds import asyncio which was missing. Co-Authored-By: Claude Sonnet 4.6 --- services/schedule_service.py | 39 ++-- tests/test_services_schedule.py | 310 ++++++++++++++++++++++++++++++++ 2 files changed, 327 insertions(+), 22 deletions(-) create mode 100644 tests/test_services_schedule.py diff --git a/services/schedule_service.py b/services/schedule_service.py index 78ee51d..edac41b 100644 --- a/services/schedule_service.py +++ b/services/schedule_service.py @@ -4,6 +4,7 @@ Schedule service for Discord Bot v2.0 Handles game schedule and results retrieval and processing. """ +import asyncio import logging from typing import Optional, List, Dict, Tuple @@ -102,10 +103,10 @@ class ScheduleService: # If weeks not specified, try a reasonable range (18 weeks typical) week_range = range(1, (weeks + 1) if weeks else 19) - for week in week_range: - week_games = await self.get_week_schedule(season, week) - - # Filter games involving this team + all_week_games = await asyncio.gather( + *[self.get_week_schedule(season, week) for week in week_range] + ) + for week_games in all_week_games: for game in week_games: if ( game.away_team.abbrev.upper() == team_abbrev_upper @@ -135,15 +136,13 @@ class ScheduleService: recent_games = [] # Get games from recent weeks - for week_offset in range(weeks_back): - # This is simplified - in production you'd want to determine current week - week = 10 - week_offset # Assuming we're around week 10 - if week <= 0: - break - - week_games = await self.get_week_schedule(season, week) - - # Only include completed games + weeks_to_fetch = [ + (10 - offset) for offset in range(weeks_back) if (10 - offset) > 0 + ] + all_week_games = await asyncio.gather( + *[self.get_week_schedule(season, week) for week in weeks_to_fetch] + ) + for week_games in all_week_games: completed_games = [game for game in week_games if game.is_completed] recent_games.extend(completed_games) @@ -171,20 +170,16 @@ class ScheduleService: try: upcoming_games = [] - # Scan through weeks to find games without scores - for week in range(1, 19): # Standard season length - week_games = await self.get_week_schedule(season, week) - - # Find games without scores (not yet played) + # Fetch all weeks in parallel and filter for incomplete games + all_week_games = await asyncio.gather( + *[self.get_week_schedule(season, week) for week in range(1, 19)] + ) + for week_games in all_week_games: upcoming_games_week = [ game for game in week_games if not game.is_completed ] upcoming_games.extend(upcoming_games_week) - # If we found upcoming games, we can limit how many more weeks to check - if upcoming_games and len(upcoming_games) >= 20: # Reasonable limit - break - # Sort by week, then game number upcoming_games.sort(key=lambda x: (x.week, x.game_num or 0)) diff --git a/tests/test_services_schedule.py b/tests/test_services_schedule.py new file mode 100644 index 0000000..134a978 --- /dev/null +++ b/tests/test_services_schedule.py @@ -0,0 +1,310 @@ +""" +Tests for schedule service functionality. + +Covers get_week_schedule, get_team_schedule, get_recent_games, +get_upcoming_games, and group_games_by_series — verifying the +asyncio.gather parallelization and post-fetch filtering logic. +""" + +import pytest +from unittest.mock import AsyncMock, patch + +from models.game import Game +from models.team import Team +from services.schedule_service import ScheduleService + + +def _make_team(team_id: int, abbrev: str) -> Team: + """Create a minimal Team for testing.""" + return Team( + id=team_id, + abbrev=abbrev, + sname=abbrev, + lname=f"{abbrev} Team", + season=12, + ) + + +def _make_game( + game_id: int, + week: int, + away_abbrev: str, + home_abbrev: str, + *, + away_score=None, + home_score=None, + game_num=1, +) -> Game: + """Create a Game with the given matchup and optional scores.""" + return Game( + id=game_id, + season=12, + week=week, + game_num=game_num, + season_type="regular", + away_team=_make_team(game_id * 10, away_abbrev), + home_team=_make_team(game_id * 10 + 1, home_abbrev), + away_score=away_score, + home_score=home_score, + ) + + +class TestGetWeekSchedule: + """Tests for ScheduleService.get_week_schedule — the HTTP layer.""" + + @pytest.fixture + def service(self): + svc = ScheduleService() + svc.get_client = AsyncMock() + return svc + + @pytest.mark.asyncio + async def test_success(self, service): + """get_week_schedule returns parsed Game objects on a normal response.""" + mock_client = AsyncMock() + mock_client.get.return_value = { + "games": [ + { + "id": 1, + "season": 12, + "week": 5, + "game_num": 1, + "season_type": "regular", + "away_team": { + "id": 10, + "abbrev": "NYY", + "sname": "NYY", + "lname": "New York", + "season": 12, + }, + "home_team": { + "id": 11, + "abbrev": "BOS", + "sname": "BOS", + "lname": "Boston", + "season": 12, + }, + "away_score": 4, + "home_score": 2, + } + ] + } + service.get_client.return_value = mock_client + + games = await service.get_week_schedule(12, 5) + + assert len(games) == 1 + assert games[0].away_team.abbrev == "NYY" + assert games[0].home_team.abbrev == "BOS" + assert games[0].is_completed + + @pytest.mark.asyncio + async def test_empty_response(self, service): + """get_week_schedule returns [] when the API has no games.""" + mock_client = AsyncMock() + mock_client.get.return_value = {"games": []} + service.get_client.return_value = mock_client + + games = await service.get_week_schedule(12, 99) + assert games == [] + + @pytest.mark.asyncio + async def test_api_error_returns_empty(self, service): + """get_week_schedule returns [] on API error (no exception raised).""" + service.get_client.side_effect = Exception("connection refused") + + games = await service.get_week_schedule(12, 1) + assert games == [] + + @pytest.mark.asyncio + async def test_missing_games_key(self, service): + """get_week_schedule returns [] when response lacks 'games' key.""" + mock_client = AsyncMock() + mock_client.get.return_value = {"status": "ok"} + service.get_client.return_value = mock_client + + games = await service.get_week_schedule(12, 1) + assert games == [] + + +class TestGetTeamSchedule: + """Tests for get_team_schedule — gather + team-abbrev filter.""" + + @pytest.fixture + def service(self): + return ScheduleService() + + @pytest.mark.asyncio + async def test_filters_by_team_case_insensitive(self, service): + """get_team_schedule returns only games involving the requested team, + regardless of abbreviation casing.""" + week1 = [ + _make_game(1, 1, "NYY", "BOS", away_score=3, home_score=1), + _make_game(2, 1, "LAD", "CHC", away_score=5, home_score=2), + ] + week2 = [ + _make_game(3, 2, "BOS", "NYY", away_score=2, home_score=4), + ] + + with patch.object(service, "get_week_schedule", new_callable=AsyncMock) as mock: + mock.side_effect = [week1, week2] + result = await service.get_team_schedule(12, "nyy", weeks=2) + + assert len(result) == 2 + abbrevs = {(g.away_team.abbrev, g.home_team.abbrev) for g in result} + assert all("NYY" in pair for pair in abbrevs) + + @pytest.mark.asyncio + async def test_full_season_fetches_18_weeks(self, service): + """When weeks is None, all 18 weeks are fetched via gather.""" + with patch.object(service, "get_week_schedule", new_callable=AsyncMock) as mock: + mock.return_value = [] + await service.get_team_schedule(12, "NYY") + + assert mock.call_count == 18 + + @pytest.mark.asyncio + async def test_limited_weeks(self, service): + """When weeks=5, only 5 weeks are fetched.""" + with patch.object(service, "get_week_schedule", new_callable=AsyncMock) as mock: + mock.return_value = [] + await service.get_team_schedule(12, "NYY", weeks=5) + + assert mock.call_count == 5 + + +class TestGetRecentGames: + """Tests for get_recent_games — gather + completed-only filter.""" + + @pytest.fixture + def service(self): + return ScheduleService() + + @pytest.mark.asyncio + async def test_returns_only_completed_games(self, service): + """get_recent_games filters out games without scores.""" + completed = _make_game(1, 10, "NYY", "BOS", away_score=3, home_score=1) + incomplete = _make_game(2, 10, "LAD", "CHC") + + with patch.object(service, "get_week_schedule", new_callable=AsyncMock) as mock: + mock.return_value = [completed, incomplete] + result = await service.get_recent_games(12, weeks_back=1) + + assert len(result) == 1 + assert result[0].is_completed + + @pytest.mark.asyncio + async def test_sorted_descending_by_week_and_game_num(self, service): + """Recent games are sorted most-recent first.""" + game_w10 = _make_game( + 1, 10, "NYY", "BOS", away_score=3, home_score=1, game_num=2 + ) + game_w9 = _make_game(2, 9, "LAD", "CHC", away_score=5, home_score=2, game_num=1) + + with patch.object(service, "get_week_schedule", new_callable=AsyncMock) as mock: + mock.side_effect = [[game_w10], [game_w9]] + result = await service.get_recent_games(12, weeks_back=2) + + assert result[0].week == 10 + assert result[1].week == 9 + + @pytest.mark.asyncio + async def test_skips_negative_weeks(self, service): + """Weeks that would be <= 0 are excluded from fetch.""" + with patch.object(service, "get_week_schedule", new_callable=AsyncMock) as mock: + mock.return_value = [] + await service.get_recent_games(12, weeks_back=15) + + # weeks_to_fetch = [10, 9, 8, 7, 6, 5, 4, 3, 2, 1] — only 10 valid weeks + assert mock.call_count == 10 + + +class TestGetUpcomingGames: + """Tests for get_upcoming_games — gather all 18 weeks + incomplete filter.""" + + @pytest.fixture + def service(self): + return ScheduleService() + + @pytest.mark.asyncio + async def test_returns_only_incomplete_games(self, service): + """get_upcoming_games filters out completed games.""" + completed = _make_game(1, 5, "NYY", "BOS", away_score=3, home_score=1) + upcoming = _make_game(2, 5, "LAD", "CHC") + + with patch.object(service, "get_week_schedule", new_callable=AsyncMock) as mock: + mock.return_value = [completed, upcoming] + result = await service.get_upcoming_games(12) + + assert len(result) == 18 # 1 incomplete game per week × 18 weeks + assert all(not g.is_completed for g in result) + + @pytest.mark.asyncio + async def test_sorted_ascending_by_week_and_game_num(self, service): + """Upcoming games are sorted earliest first.""" + game_w3 = _make_game(1, 3, "NYY", "BOS", game_num=1) + game_w1 = _make_game(2, 1, "LAD", "CHC", game_num=2) + + with patch.object(service, "get_week_schedule", new_callable=AsyncMock) as mock: + # Week 1 returns game_w1, week 3 returns game_w3, others empty + def side_effect(season, week): + if week == 1: + return [game_w1] + if week == 3: + return [game_w3] + return [] + + mock.side_effect = side_effect + result = await service.get_upcoming_games(12) + + assert result[0].week == 1 + assert result[1].week == 3 + + @pytest.mark.asyncio + async def test_fetches_all_18_weeks(self, service): + """All 18 weeks are fetched in parallel (no early exit).""" + with patch.object(service, "get_week_schedule", new_callable=AsyncMock) as mock: + mock.return_value = [] + await service.get_upcoming_games(12) + + assert mock.call_count == 18 + + +class TestGroupGamesBySeries: + """Tests for group_games_by_series — synchronous grouping logic.""" + + @pytest.fixture + def service(self): + return ScheduleService() + + def test_groups_by_alphabetical_pairing(self, service): + """Games between the same two teams are grouped under one key, + with the alphabetically-first team first in the tuple.""" + games = [ + _make_game(1, 1, "NYY", "BOS", game_num=1), + _make_game(2, 1, "BOS", "NYY", game_num=2), + _make_game(3, 1, "LAD", "CHC", game_num=1), + ] + + result = service.group_games_by_series(games) + + assert ("BOS", "NYY") in result + assert len(result[("BOS", "NYY")]) == 2 + assert ("CHC", "LAD") in result + assert len(result[("CHC", "LAD")]) == 1 + + def test_sorted_by_game_num_within_series(self, service): + """Games within each series are sorted by game_num.""" + games = [ + _make_game(1, 1, "NYY", "BOS", game_num=3), + _make_game(2, 1, "NYY", "BOS", game_num=1), + _make_game(3, 1, "NYY", "BOS", game_num=2), + ] + + result = service.group_games_by_series(games) + series = result[("BOS", "NYY")] + assert [g.game_num for g in series] == [1, 2, 3] + + def test_empty_input(self, service): + """Empty games list returns empty dict.""" + assert service.group_games_by_series([]) == {}