fix: resolve 4 logic bugs found in code review
- utc_to_eastern: use zoneinfo instead of hardcoded EDT offset (-4) so start times are correct in both EST and EDT - standings: fetch before truncate so a failed API call doesn't wipe existing standings data - routes: call parse_games() once per request instead of three times - scheduler: wrap run_pending() in try/except so an unhandled exception doesn't kill the background thread Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
10
app/games.py
10
app/games.py
@@ -1,9 +1,12 @@
|
|||||||
import logging
|
import logging
|
||||||
import sqlite3
|
import sqlite3
|
||||||
from datetime import datetime, timedelta
|
from datetime import datetime, timezone
|
||||||
|
from zoneinfo import ZoneInfo
|
||||||
|
|
||||||
from app.config import DB_PATH
|
from app.config import DB_PATH
|
||||||
|
|
||||||
|
EASTERN = ZoneInfo("America/New_York")
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
@@ -226,6 +229,5 @@ def get_team_standings(team_name):
|
|||||||
|
|
||||||
def utc_to_eastern(utc_time):
|
def utc_to_eastern(utc_time):
|
||||||
utc_datetime = datetime.strptime(utc_time, "%Y-%m-%dT%H:%M:%SZ")
|
utc_datetime = datetime.strptime(utc_time, "%Y-%m-%dT%H:%M:%SZ")
|
||||||
est_offset = timedelta(hours=-4)
|
eastern_datetime = utc_datetime.replace(tzinfo=timezone.utc).astimezone(EASTERN)
|
||||||
est_datetime = utc_datetime + est_offset
|
return eastern_datetime.strftime("%I:%M %p")
|
||||||
return est_datetime.strftime("%I:%M %p")
|
|
||||||
|
|||||||
@@ -25,24 +25,12 @@ def get_scoreboard():
|
|||||||
)
|
)
|
||||||
|
|
||||||
if scoreboard_data:
|
if scoreboard_data:
|
||||||
live_games = [
|
games = parse_games(scoreboard_data)
|
||||||
game
|
|
||||||
for game in parse_games(scoreboard_data)
|
|
||||||
if game["Game State"] == "LIVE"
|
|
||||||
]
|
|
||||||
pre_games = [
|
|
||||||
game for game in parse_games(scoreboard_data) if game["Game State"] == "PRE"
|
|
||||||
]
|
|
||||||
final_games = [
|
|
||||||
game
|
|
||||||
for game in parse_games(scoreboard_data)
|
|
||||||
if game["Game State"] == "FINAL"
|
|
||||||
]
|
|
||||||
return jsonify(
|
return jsonify(
|
||||||
{
|
{
|
||||||
"live_games": live_games,
|
"live_games": [g for g in games if g["Game State"] == "LIVE"],
|
||||||
"pre_games": pre_games,
|
"pre_games": [g for g in games if g["Game State"] == "PRE"],
|
||||||
"final_games": final_games,
|
"final_games": [g for g in games if g["Game State"] == "FINAL"],
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
else:
|
else:
|
||||||
|
|||||||
@@ -14,5 +14,8 @@ def start_scheduler():
|
|||||||
schedule.every(10).seconds.do(refresh_scores)
|
schedule.every(10).seconds.do(refresh_scores)
|
||||||
logger.info("Background scheduler started")
|
logger.info("Background scheduler started")
|
||||||
while True:
|
while True:
|
||||||
|
try:
|
||||||
schedule.run_pending()
|
schedule.run_pending()
|
||||||
|
except Exception:
|
||||||
|
logger.exception("Scheduler encountered an unexpected error")
|
||||||
time.sleep(1)
|
time.sleep(1)
|
||||||
|
|||||||
@@ -67,8 +67,8 @@ def fetch_standings():
|
|||||||
def refresh_standings():
|
def refresh_standings():
|
||||||
conn = sqlite3.connect(DB_PATH)
|
conn = sqlite3.connect(DB_PATH)
|
||||||
create_standings_table(conn)
|
create_standings_table(conn)
|
||||||
truncate_standings_table(conn)
|
|
||||||
standings = fetch_standings()
|
standings = fetch_standings()
|
||||||
if standings:
|
if standings:
|
||||||
|
truncate_standings_table(conn)
|
||||||
insert_standings(conn, standings)
|
insert_standings(conn, standings)
|
||||||
conn.close()
|
conn.close()
|
||||||
|
|||||||
@@ -101,10 +101,16 @@ class TestGetGameOutcome:
|
|||||||
|
|
||||||
|
|
||||||
class TestUtcToEstTime:
|
class TestUtcToEstTime:
|
||||||
def test_converts_utc_to_est(self):
|
def test_converts_utc_to_edt(self):
|
||||||
|
# April is EDT (UTC-4): 23:00 UTC → 07:00 PM EDT
|
||||||
result = utc_to_eastern("2024-04-10T23:00:00Z")
|
result = utc_to_eastern("2024-04-10T23:00:00Z")
|
||||||
assert result == "07:00 PM"
|
assert result == "07:00 PM"
|
||||||
|
|
||||||
|
def test_converts_utc_to_est(self):
|
||||||
|
# January is EST (UTC-5): 23:00 UTC → 06:00 PM EST
|
||||||
|
result = utc_to_eastern("2024-01-15T23:00:00Z")
|
||||||
|
assert result == "06:00 PM"
|
||||||
|
|
||||||
|
|
||||||
class TestParseGames:
|
class TestParseGames:
|
||||||
def test_returns_empty_list_for_none(self):
|
def test_returns_empty_list_for_none(self):
|
||||||
|
|||||||
@@ -39,3 +39,20 @@ class TestStartScheduler:
|
|||||||
start_scheduler()
|
start_scheduler()
|
||||||
|
|
||||||
assert mock_schedule.run_pending.call_count >= 2
|
assert mock_schedule.run_pending.call_count >= 2
|
||||||
|
|
||||||
|
def test_continues_after_exception_in_run_pending(self, mocker):
|
||||||
|
mock_schedule = mocker.patch("app.scheduler.schedule")
|
||||||
|
call_count = {"n": 0}
|
||||||
|
|
||||||
|
def raise_then_stop(_):
|
||||||
|
call_count["n"] += 1
|
||||||
|
if call_count["n"] >= 2:
|
||||||
|
raise StopIteration
|
||||||
|
|
||||||
|
mock_schedule.run_pending.side_effect = RuntimeError("boom")
|
||||||
|
mocker.patch("app.scheduler.time.sleep", side_effect=raise_then_stop)
|
||||||
|
|
||||||
|
with pytest.raises(StopIteration):
|
||||||
|
start_scheduler()
|
||||||
|
|
||||||
|
assert mock_schedule.run_pending.call_count >= 2
|
||||||
|
|||||||
@@ -209,12 +209,25 @@ class TestRefreshStandings:
|
|||||||
assert count == 2
|
assert count == 2
|
||||||
|
|
||||||
def test_does_not_insert_when_fetch_fails(self, mocker, tmp_path):
|
def test_does_not_insert_when_fetch_fails(self, mocker, tmp_path):
|
||||||
mocker.patch("app.standings.fetch_standings", return_value=None)
|
db_path = str(tmp_path / "test.db")
|
||||||
mocker.patch("app.standings.DB_PATH", str(tmp_path / "test.db"))
|
mocker.patch("app.standings.DB_PATH", db_path)
|
||||||
|
|
||||||
|
# Seed with existing data before the failed refresh
|
||||||
|
seed = [
|
||||||
|
{
|
||||||
|
"team_common_name": "Bruins",
|
||||||
|
"league_sequence": 1,
|
||||||
|
"league_l10_sequence": 2,
|
||||||
|
}
|
||||||
|
]
|
||||||
|
mocker.patch("app.standings.fetch_standings", return_value=seed)
|
||||||
refresh_standings()
|
refresh_standings()
|
||||||
|
|
||||||
conn = sqlite3.connect(str(tmp_path / "test.db"))
|
# Now simulate a fetch failure — existing data must be preserved
|
||||||
|
mocker.patch("app.standings.fetch_standings", return_value=None)
|
||||||
|
refresh_standings()
|
||||||
|
|
||||||
|
conn = sqlite3.connect(db_path)
|
||||||
count = conn.execute("SELECT COUNT(*) FROM standings").fetchone()[0]
|
count = conn.execute("SELECT COUNT(*) FROM standings").fetchone()[0]
|
||||||
conn.close()
|
conn.close()
|
||||||
assert count == 0
|
assert count == 1
|
||||||
|
|||||||
Reference in New Issue
Block a user