mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-02 09:18:06 +00:00
fix(sports): harden the live game-over check and font/log init
Follow-up review findings on the promoted base-class methods. _is_game_really_over: - `period` present-but-None raised TypeError on `None >= FINAL_PERIOD`, taking down the whole live-update pass (_detect_stale_games has no try/except). Same failure shape as the null `period_text` already fixed. - An expired clock spelled "00:00" normalizes to "0000", which matched none of the hand-listed literals, so a finished game with a two-digit minute clock stayed on the scoreboard forever. Compare numerically. SportsCore: - _load_fonts kept the cwd-relative "assets/fonts/..." literals the _font_root() seam exists to remove, so every scoreboard font degraded to PIL's default face outside the install root. - _should_log read self._last_warning_time unguarded while only an unrelated method initialized it lazily; the first warning of a run raised AttributeError. Initialize it in __init__. Also documents that game_update_timestamps is written by subclasses, not by the base class, so the staleness branch is inert until B5 adoption. 14 new tests. Gates: 463 core unit, 60 plugin safety. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
This commit is contained in:
@@ -62,6 +62,20 @@ release that ships it.
|
|||||||
MiLB events synthesized from the MLB Stats API do not, so the lookup raised
|
MiLB events synthesized from the MLB Stats API do not, so the lookup raised
|
||||||
a bare `KeyError`. It now reads the already-validated competition-level
|
a bare `KeyError`. It now reads the already-validated competition-level
|
||||||
status.
|
status.
|
||||||
|
- `SportsLive._is_game_really_over` no longer crashes the live-update pass when
|
||||||
|
a feed sends an explicit null `period`. `None >= FINAL_PERIOD` raised
|
||||||
|
`TypeError`, and the only caller (`_detect_stale_games`) has no `try/except`
|
||||||
|
— the same failure shape as the already-fixed null `period_text`.
|
||||||
|
- An expired clock spelled `"00:00"` now ends the game. The check compared the
|
||||||
|
colon-stripped clock against a hand-listed set of literals, which `"0000"` is
|
||||||
|
not a member of, so a finished game with a two-digit-minute clock stayed on
|
||||||
|
the scoreboard indefinitely. The comparison is now numeric.
|
||||||
|
- `SportsCore._load_fonts` resolves `assets/fonts` through the `_font_root()`
|
||||||
|
seam instead of the process working directory. Started outside the install
|
||||||
|
root, every scoreboard font silently degraded to PIL's default bitmap face.
|
||||||
|
- `SportsCore._should_log` no longer raises `AttributeError` on the first
|
||||||
|
warning of a run; `_last_warning_time` is initialized in `__init__` rather
|
||||||
|
than lazily by an unrelated method.
|
||||||
- `SportsCore._resolve_project_path` resolved relative logo directories
|
- `SportsCore._resolve_project_path` resolved relative logo directories
|
||||||
against `<root>/src` instead of the repo root after `sports.py` became a
|
against `<root>/src` instead of the repo root after `sports.py` became a
|
||||||
package — the class bodies moved byte-identically but `__file__` gained a
|
package — the class bodies moved byte-identically but `__file__` gained a
|
||||||
|
|||||||
@@ -185,6 +185,10 @@ class SportsCore(ABC):
|
|||||||
}
|
}
|
||||||
self.last_update = 0
|
self.last_update = 0
|
||||||
self.current_game = None
|
self.current_game = None
|
||||||
|
# Cooldown clock for _should_log(). Initialized here rather than lazily:
|
||||||
|
# _should_log() reads it unguarded, so whichever warning fires first
|
||||||
|
# would otherwise raise AttributeError instead of logging.
|
||||||
|
self._last_warning_time = 0
|
||||||
self.fonts = self._load_fonts()
|
self.fonts = self._load_fonts()
|
||||||
|
|
||||||
# Optional visual skin (see docs/SKIN_SYSTEM.md). "skin" is either a
|
# Optional visual skin (see docs/SKIN_SYSTEM.md). "skin" is either a
|
||||||
@@ -425,17 +429,26 @@ class SportsCore(ABC):
|
|||||||
|
|
||||||
|
|
||||||
def _load_fonts(self):
|
def _load_fonts(self):
|
||||||
"""Load fonts used by the scoreboard."""
|
"""Load fonts used by the scoreboard.
|
||||||
|
|
||||||
|
Paths go through :meth:`_resolve_font_path` so the bundled fonts are
|
||||||
|
found regardless of the process working directory — a bare
|
||||||
|
``"assets/fonts/..."`` silently degraded every scoreboard to the PIL
|
||||||
|
default font whenever the process started elsewhere (the plugin safety
|
||||||
|
harness on CI being the case that surfaced it).
|
||||||
|
"""
|
||||||
fonts = {}
|
fonts = {}
|
||||||
|
press_start = self._resolve_font_path("PressStart2P-Regular.ttf")
|
||||||
|
four_by_six = self._resolve_font_path("4x6-font.ttf")
|
||||||
try:
|
try:
|
||||||
fonts['score'] = ImageFont.truetype("assets/fonts/PressStart2P-Regular.ttf", 10)
|
fonts['score'] = ImageFont.truetype(press_start, 10)
|
||||||
fonts['time'] = ImageFont.truetype("assets/fonts/PressStart2P-Regular.ttf", 8)
|
fonts['time'] = ImageFont.truetype(press_start, 8)
|
||||||
fonts['team'] = ImageFont.truetype("assets/fonts/PressStart2P-Regular.ttf", 8)
|
fonts['team'] = ImageFont.truetype(press_start, 8)
|
||||||
fonts['status'] = ImageFont.truetype("assets/fonts/4x6-font.ttf", 6) # Using 4x6 for status
|
fonts['status'] = ImageFont.truetype(four_by_six, 6) # Using 4x6 for status
|
||||||
fonts['detail'] = ImageFont.truetype("assets/fonts/4x6-font.ttf", 6) # Added detail font
|
fonts['detail'] = ImageFont.truetype(four_by_six, 6) # Added detail font
|
||||||
fonts['rank'] = ImageFont.truetype("assets/fonts/PressStart2P-Regular.ttf", 10)
|
fonts['rank'] = ImageFont.truetype(press_start, 10)
|
||||||
logging.info("Successfully loaded fonts") # Changed log prefix
|
logging.info("Successfully loaded fonts") # Changed log prefix
|
||||||
except IOError:
|
except OSError:
|
||||||
logging.warning("Fonts not found, using default PIL font.") # Changed log prefix
|
logging.warning("Fonts not found, using default PIL font.") # Changed log prefix
|
||||||
fonts['score'] = ImageFont.load_default()
|
fonts['score'] = ImageFont.load_default()
|
||||||
fonts['time'] = ImageFont.load_default()
|
fonts['time'] = ImageFont.load_default()
|
||||||
|
|||||||
@@ -842,8 +842,11 @@ class SportsLive(SportsCore):
|
|||||||
self.count_log_interval = 5 # Only log count data every 5 seconds
|
self.count_log_interval = 5 # Only log count data every 5 seconds
|
||||||
# Initialize test_mode - defaults to False (live mode)
|
# Initialize test_mode - defaults to False (live mode)
|
||||||
self.test_mode = self.mode_config.get("test_mode", False)
|
self.test_mode = self.mode_config.get("test_mode", False)
|
||||||
# Freshness bookkeeping for _detect_stale_games():
|
# Freshness bookkeeping for _detect_stale_games(). The base class only
|
||||||
# {game_id: {"clock": ts, "score": ts, "last_seen": ts}}
|
# *reads* this map; a subclass's update() stamps entries as it ingests a
|
||||||
|
# feed: {game_id: {"clock": ts, "score": ts, "last_seen": ts}}.
|
||||||
|
# Until a subclass writes "last_seen", the staleness branch of
|
||||||
|
# _detect_stale_games is inert and only the game-over check applies.
|
||||||
self.game_update_timestamps = {}
|
self.game_update_timestamps = {}
|
||||||
self.stale_game_timeout = self.mode_config.get("stale_game_timeout", 300) # 5 minutes default
|
self.stale_game_timeout = self.mode_config.get("stale_game_timeout", 300) # 5 minutes default
|
||||||
|
|
||||||
@@ -884,7 +887,11 @@ class SportsLive(SportsCore):
|
|||||||
return False
|
return False
|
||||||
|
|
||||||
raw_clock = game.get("clock")
|
raw_clock = game.get("clock")
|
||||||
period = game.get("period", 0)
|
# `or 0` rather than a get() default: feeds routinely send an explicit
|
||||||
|
# null period, and `None >= FINAL_PERIOD` raises TypeError — which would
|
||||||
|
# take down the whole live-update pass, since the only caller
|
||||||
|
# (_detect_stale_games) has no try/except around it.
|
||||||
|
period = game.get("period") or 0
|
||||||
|
|
||||||
# Only check clock-based finish if we have a valid clock string. A
|
# Only check clock-based finish if we have a valid clock string. A
|
||||||
# missing or non-string clock is NOT coerced to "0:00": sports without a
|
# missing or non-string clock is NOT coerced to "0:00": sports without a
|
||||||
@@ -892,8 +899,11 @@ class SportsLive(SportsCore):
|
|||||||
# otherwise be declared over from the FINAL_PERIOD-th period onward.
|
# otherwise be declared over from the FINAL_PERIOD-th period onward.
|
||||||
if isinstance(raw_clock, str) and raw_clock.strip() and period >= self.FINAL_PERIOD:
|
if isinstance(raw_clock, str) and raw_clock.strip() and period >= self.FINAL_PERIOD:
|
||||||
clock = raw_clock
|
clock = raw_clock
|
||||||
|
# Compare numerically rather than against a literal set: feeds spell
|
||||||
|
# an expired clock "0:00", ":00" and "00:00" depending on sport, and
|
||||||
|
# a membership test silently misses every spelling not listed.
|
||||||
clock_normalized = clock.replace(":", "").strip()
|
clock_normalized = clock.replace(":", "").strip()
|
||||||
if clock_normalized in ("000", "00") or clock in ("0:00", ":00"):
|
if clock_normalized.isdigit() and int(clock_normalized) == 0:
|
||||||
self.logger.debug(
|
self.logger.debug(
|
||||||
f"_is_game_really_over({game_str}): "
|
f"_is_game_really_over({game_str}): "
|
||||||
f"returning True - clock at 0:00 (clock='{clock}', period={period})"
|
f"returning True - clock at 0:00 (clock='{clock}', period={period})"
|
||||||
|
|||||||
@@ -465,6 +465,38 @@ class TestFontLoaderCwdIndependence:
|
|||||||
assert isinstance(font, ImageFont.FreeTypeFont)
|
assert isinstance(font, ImageFont.FreeTypeFont)
|
||||||
assert Path(font.path) == FONTS_DIR / TTF_NAME
|
assert Path(font.path) == FONTS_DIR / TTF_NAME
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("key", ["score", "time", "team", "status",
|
||||||
|
"detail", "rank"])
|
||||||
|
def test_load_fonts_survives_an_unrelated_cwd(self, monkeypatch, key):
|
||||||
|
"""`_load_fonts` had the same cwd-relative literals the seam exists to
|
||||||
|
remove; every scoreboard font silently became PIL's default bitmap face
|
||||||
|
when the process started outside the install root."""
|
||||||
|
monkeypatch.chdir("/")
|
||||||
|
fonts = SportsCore._load_fonts(probe())
|
||||||
|
assert isinstance(fonts[key], ImageFont.FreeTypeFont), (
|
||||||
|
f"fonts['{key}'] degraded to PIL's default face outside the "
|
||||||
|
"install root")
|
||||||
|
|
||||||
|
|
||||||
|
class TestShouldLogCooldown:
|
||||||
|
"""`_should_log` reads `self._last_warning_time` unguarded, so it must be
|
||||||
|
initialized in __init__ — otherwise the first warning of a run raises
|
||||||
|
AttributeError instead of logging."""
|
||||||
|
|
||||||
|
def test_cooldown_clock_is_initialized(self, build):
|
||||||
|
assert build()._last_warning_time == 0
|
||||||
|
|
||||||
|
def test_first_call_logs_then_cools_down(self, build):
|
||||||
|
manager = build()
|
||||||
|
assert manager._should_log("api", cooldown=60) is True
|
||||||
|
assert manager._should_log("api", cooldown=60) is False
|
||||||
|
|
||||||
|
def test_cooldown_expires(self, build):
|
||||||
|
manager = build()
|
||||||
|
assert manager._should_log("api", cooldown=60) is True
|
||||||
|
manager._last_warning_time -= 61
|
||||||
|
assert manager._should_log("api", cooldown=60) is True
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# 4. Seam guard rails
|
# 4. Seam guard rails
|
||||||
|
|||||||
@@ -254,12 +254,32 @@ class TestIsGameReallyOver:
|
|||||||
manager = build_manager(_LiveHarness)
|
manager = build_manager(_LiveHarness)
|
||||||
assert manager._is_game_really_over(game(clock="12:00", period=2)) is False
|
assert manager._is_game_really_over(game(clock="12:00", period=2)) is False
|
||||||
|
|
||||||
@pytest.mark.parametrize("clock", ["0:00", ":00", "00", "000", " 0:00 "])
|
@pytest.mark.parametrize(
|
||||||
|
"clock", ["0:00", ":00", "00", "000", " 0:00 ", "00:00", "0", "0000"])
|
||||||
def test_expired_clock_at_final_period_is_over(self, build_manager, clock):
|
def test_expired_clock_at_final_period_is_over(self, build_manager, clock):
|
||||||
|
"""Every spelling of a zeroed clock counts, not a hand-listed few.
|
||||||
|
|
||||||
|
"00:00" is the one that motivated comparing numerically: it normalizes
|
||||||
|
to "0000", which matched none of the literals the plugin copies listed,
|
||||||
|
so a two-digit-minute expired clock kept the game on screen forever.
|
||||||
|
"""
|
||||||
manager = build_manager(_LiveHarness)
|
manager = build_manager(_LiveHarness)
|
||||||
assert manager._is_game_really_over(
|
assert manager._is_game_really_over(
|
||||||
game(clock=clock, period=4, period_text="Q4")) is True
|
game(clock=clock, period=4, period_text="Q4")) is True
|
||||||
|
|
||||||
|
def test_none_period_at_expired_clock_does_not_raise(self, build_manager):
|
||||||
|
"""`period` present-but-None: `None >= FINAL_PERIOD` is a TypeError, and
|
||||||
|
`_detect_stale_games` has no try/except — the same failure shape as the
|
||||||
|
`period_text` case above."""
|
||||||
|
manager = build_manager(_LiveHarness)
|
||||||
|
assert manager._is_game_really_over(
|
||||||
|
game(clock="0:00", period=None, period_text="Q4")) is False
|
||||||
|
|
||||||
|
def test_none_period_with_running_clock_does_not_raise(self, build_manager):
|
||||||
|
manager = build_manager(_LiveHarness)
|
||||||
|
assert manager._is_game_really_over(
|
||||||
|
game(clock="8:12", period=None, period_text="Q2")) is False
|
||||||
|
|
||||||
def test_expired_clock_after_final_period_is_over(self, build_manager):
|
def test_expired_clock_after_final_period_is_over(self, build_manager):
|
||||||
manager = build_manager(_LiveHarness)
|
manager = build_manager(_LiveHarness)
|
||||||
assert manager._is_game_really_over(
|
assert manager._is_game_really_over(
|
||||||
|
|||||||
Reference in New Issue
Block a user