mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 22:35:08 +00:00
* fix(errors): record the exception's own stack trace record_error() called traceback.format_exc(), which only sees an exception while its except block is running. plugin_executor records exceptions caught on a worker thread after that block has ended, so every trace on /errors read "NoneType: None". The trace is now built from the exception's __traceback__. The executor's log call had the same problem with exc_info=True and now passes the exception. record_error() also merged LEDMatrixError context into the caller's dict in place; it now works on a copy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(wifi): point at configure_wifi_permissions.sh instead of a sudoers list The module docstring told users to grant NOPASSWD sudo on iptables and ip. configure_wifi_permissions.sh refuses those grants on purpose: a wildcard rule for either runs an arbitrary program as root. Point at the script and say why it leaves them out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(wifi): disconnect finds the saved profile by SSID disconnect_from_network() asked `nmcli -f NAME,802-11-wireless.ssid connection show` for the profile to take down, but nmcli rejects that column for `connection show`, so the lookup always failed and only the device was disconnected. The per-profile lookup _connect_nmcli() already used is now _find_profile_for_ssid(), and both callers share it. It also splits terse output on the last colon and unescapes "\:", so a profile name containing a colon is found. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(wifi): write wifi_config.json atomically and report a failed save _save_config() opened the file for writing in place and swallowed any error, so a wifi_config.json left owned by root made the web toggle for auto-enabling AP mode report success while nothing was saved, and a crash mid-write could truncate the file. It now uses atomic_write_json, which also keeps the file's owner and shared group when root saves it, and returns False on failure. POST /wifi/ap/auto-enable answers 500 in that case. The file is now written with indent=4, like the other config files. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(fonts): resolve plugin:// fonts in the plugin's own directory FontManager looked for a plugin's bundled fonts under Path("plugins") / plugin_id: relative to the process cwd, and not the default install directory (plugin-repos/), so a manifest's plugin:// fonts never loaded. register_plugin_fonts() takes an optional plugin_dir, and PluginManager passes the directory it loaded the plugin from. Callers that omit it get a lookup in the configured plugin_system.plugins_directory, then plugins/, resolved against the install root. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(api-helper): cache responses for the requested cache_ttl APIHelper.get(cache_ttl=...) and set_cache(ttl=...) dropped the ttl on the claim that CacheManager does not support one, but CacheManager.set() takes a ttl, stores it with the entry, and both cache tiers honour it over a reader's max_age. Without it every response expired after the 300-second default read age, whatever the plugin asked for. The ttl is now passed through, and the cache read passes cache_ttl as max_age for entries written without one. The class docstring describes what the helper actually does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(style): one scale range for the schema, element_scale and LogoHelper The generated Scale field allowed 0.1 to 10, element_style's reader capped at 10 with no floor, and LogoHelper accepted 0.05 to 8 and reset anything else to 1.0. A logo scale of 9, which the form accepts, drew at the shipped size. MIN_ELEMENT_SCALE / MAX_ELEMENT_SCALE (0.1, 10.0) in src.element_style are now the schema bounds and the clamp every reader applies through coerce_scale(): a positive number outside the range is clamped, and anything that is not a finite positive number means the default. That also stops element_scale() passing NaN through, since min(nan, 10.0) is nan. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(logos): placeholder lands at the requested path; empty logos list download_missing_logo() wrote its fallback placeholder to <normalize_abbreviation(abbr)>.png in the logo directory rather than to the logo_path the caller passed, so it could return True while nothing existed where the plugin looks (e.g. "TA&M.png" vs "TAANDM.png"). create_placeholder_logo() takes an optional filepath, and download_missing_logo passes the requested one. download_missing_logo_for_team() only caught KeyError, so a team whose "logos" list is empty raised IndexError; it now treats KeyError, IndexError and TypeError as "no logo URL". The placeholder is drawn with PLACEHOLDER_SIZE / PLACEHOLDER_BG, the constants is_placeholder_logo() recognises it by, instead of repeated literals. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(fonts): resolve bundled font paths against the install root TextHelper's default font_dir, the logo placeholder's font and FontManager's font_overrides.json were all relative to the process cwd, so a process started anywhere but the install root (the plugin safety harness, a manual run, a unit without WorkingDirectory) drew with PIL's default face and read no overrides. They now go through font_layout.resolve_asset_path; the overrides file sits in the install root's config/. The resolver docstrings described an order the code does not follow: resolve_asset_path never consults the cwd, and sports_shared's _resolve_font_path tries the cwd first. Both docstrings now say what the code does, and _resolve_font_path calls resolve_asset_path instead of probing FontManager for it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(sync): the web UI reads the sync status file the display writes sync_manager writes its status to tempfile.gettempdir(), but GET /api/v3/sync/status read a hardcoded /tmp/led_matrix_sync_status.json and defaulted the port to a literal 5765. Wherever TMPDIR is set (or on any non-/tmp host) the page only ever showed "starting". The endpoint now uses sync_manager.STATUS_FILE and SYNC_PORT. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(http): the rankings resolver sends the project's User-Agent DynamicTeamResolver fetched ESPN rankings with a bare requests.get, so it sent python-requests' default User-Agent, which ESPN rejects; the AP_TOP_N favourites then resolved to nothing. It now sends DEFAULT_HTTP_HEADERS. BaseOddsManager carried its own copy of the User-Agent string and now uses the same shared headers (which also adds Accept-Language). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(backup): record the core release and read the configured plugin dir The manifest's ledmatrix_version came from a VERSION file that does not exist, then from .git/HEAD: a 12-character sha, or "ref: refs/he" when the branch's ref was packed. It is now src.__version__. list_installed_plugins() scanned a hardcoded plugin-repos/, so on an install whose plugin_system.plugins_directory points elsewhere, plugins missing from plugin_state.json were left out of the backup. It now reads the configured directory from config/config.json, defaulting to plugin-repos. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(startup): report a missing display section once A config without a display section produced three errors for the one problem ("Missing required configuration key: display", "Display configuration is missing or empty" and "Display configuration is missing"), and an empty one produced two. _validate_config now reports it once, as a missing key or an empty section, and _validate_display_config leaves it to that. The module docstring said the validator fails fast; nothing in the display service calls raise_on_errors(), so it now says the errors are reported and startup continues. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(wifi): share the copied blocks and name the AP constants - _parse_nmcli_wifi_list() is the one parser behind _scan_nmcli and _scan_nmcli_cached. - _verify_connected(), _wait_for_device_idle(), _failsafe_ap() and _mark_forced() replace blocks that were pasted two or three times in the connect and enable-AP paths. The device-idle wait now checks before its first one-second sleep instead of after it. - _check_command() calls _find_command_path() instead of repeating it. - AP_IP, PORTAL_PORT, AP_PROFILE_NAME and AP_PROFILE_NAMES name values that were spelled out 14, 12, 8 and 2 times; the two deletion loops now walk the same tuple. The iwconfig status path compares the AP address exactly: startswith() also skipped 192.168.4.10-19. - Dropped a second WIFI.SIGNAL query that repeated the first, a no-op "if ssid: continue", the try/except around _connect_wpa_supplicant's constant return, and a second save of a scan scan_networks already saves. - _ensure_wifi_radio_enabled's docstring says it returns True when the radio state cannot be read at all. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(config): drop dead branches and history comments in ConfigManager - The module docstring pointed plugin authors at update_plugin_config(), which does not exist; it now names save_config_atomic() and save_raw_file_content(). - load_config's FileNotFoundError handler tested the message for "config_secrets.json", but a missing secrets file is handled where it is read, so only config.json reaches it; the check is gone. - save_raw_file_content's `file_type == "main" or "secrets"` guard was always true (anything else raised earlier). - get_raw_file_content('secrets') already returns {} for a missing file, so the os.path.exists() in front of two calls to it is gone. - Comments that narrated earlier behaviour are rewritten as what the code does now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(background-data): present-tense comments, drop unused API - Comments that told the history of each fix (what "used to" happen, "the old per-delivery release") now state the invariant the code keeps. - get_statistics() no longer reports a constant 'queue_size': 0, and the uncalled clear_completed_requests() is gone (_cleanup_completed_requests does that job on every completion). Neither is referenced in core, the web UI or the plugin monorepo. shutdown_background_service() has no production caller either, but it is the only way to tear down the get_background_service() singleton, which the tests rely on, so it stays. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(odds): drop the unread cache_ttl and merge the odds_data branches BaseOddsManager loaded base_odds_manager.cache_ttl from config and never used it: cached odds live for the update interval (get_odds' ttl=interval). No core or monorepo code reads the attribute, so it is gone along with its log line. The two consecutive `if odds_data:` blocks are one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(backup): one table for the single-file sections config, secrets, wifi and ytm_auth were each spelled out in create, preview, validate and restore. _SINGLE_FILE_SECTIONS lists them once, with the RestoreOptions flag that restores each, and all four walk it. Restore error messages keep their wording ("Failed to restore <file name>"). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(fonts): drop FontManager's write-only state and duplicate logs - fonts_config, font_metadata and font_dependencies were written and never read; the performance_stats keys font_load_times, render_times, total_renders and the per-call "resolve" timings (_record_performance_metric) likewise. get_performance_stats() reads only the counters that remain. Nothing in core or the plugin monorepo references any of them. - A failed BDF load was logged twice, by _load_bdf_font and again by get_font; get_font's line is the one kept. - Removed "NEW:" and commented-out cozette entries, the "Copy font to assets/fonts" comment on code that copies nothing, and local imports of names the module already imports. The deprecated add_font() now resolves assets/fonts against the install root. The @deprecated methods stay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(text-helper): cache loaded fonts; drop the pre-textlength fallback TextHelper declared _font_cache, cleared it and reported its size, but never stored anything in it. load_fonts() now keeps each (file, size) it loads there, so clear_font_cache() and get_font_cache_stats() mean what they say and repeated load_fonts() calls reuse the fonts. get_text_width() no longer catches AttributeError for Pillow releases without ImageDraw.textlength; requirements.txt pins Pillow>=12.2. The class docstring describes what the helper does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(common): fix wrong docstrings in api_helper, permission_utils, snapshot_policy - permission_utils called 0o2775 "sticky bit"; the 2 is setgid, which is what makes new files take the directory's group. - snapshot_policy pointed at web_interface/blueprints/api_v3.py, which is a package now; the health check is in api_v3/misc.py. - APIHelper.clear_cache() lost a history note and a fallback to a clear() method that neither CacheManager nor the testing MockCacheManager has. The session headers are built from DEFAULT_HTTP_HEADERS instead of a copy of them, and the module docstring says what the module offers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(sports): present-tense comments in the shared scoreboard renderers - sports_scroll and sports_game_renderer comments that referred to "this PR", "the old flat 128px card" or what the renderer "previously" did now describe the current behaviour and its reason. - The block explaining why non-finite settings are rejected sat above _score_reserve_width; it describes _center_gap_width and now lives in it. - unshare_element_fonts wrapped its import of font_layout.load_truetype in an `except ImportError` that cannot fire inside core; the import stays at call time so tests can spy on the pinned loader. - sports_card docstrings that told the history of a fix say what the code does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(sports-shared): drop dead code, name the ESPN limit - _get_weeks_data asked for limit=1000, which fetch_espn_scoreboard clamps to ESPN_MAX_LIMIT anyway; it now names that constant. Its unused `immediate_events = []` is gone. - _get_season_schedule_dates() returned ("", "") and has no caller in core or the plugin monorepo. - _should_log keeps its warning_type parameter (part of the inherited signature, though nothing in core or the monorepo calls it) and its docstring says the cooldown is shared across types. - An unused ImageFont import is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(sync): one follower-mode switch, shared panel defaults - The class docstring said the leader sends PNG frames. Frames go over UDP as raw RGB; PNG is only the Vegas scroll image sent over TCP. It now describes both paths. - _enter_follower_mode() replaces the two copies of "note the leader, switch from standalone to follower, log, write status" in the frame and scroll-position handlers. - The rows/cols fallbacks use DEFAULT_ROWS / DEFAULT_COLS from src.display_geometry, as chain_length already did. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(style): drop _layout_axis, name the layout group title - ElementStyleResolver._layout_axis() had no caller in core or the plugin monorepo. - _element_block_from_spec checked spec['size'] was a dict again after size_spec already had; it reads size_spec. - The "Layout Offsets" title written into three generated schema blocks is _LAYOUT_TITLE. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(logo-helper): say what the placeholder draws; name the 1.5 box factor - _create_placeholder_logo's docstring said it draws the team abbreviation; it draws an outlined grey box and nothing else. The docstring says so, and the "in a real implementation you'd want text" comments are gone. - The 1.5 x panel default logo box, written out six times, is DEFAULT_LOGO_BOX_FACTOR. - ImageDraw is imported with Image at the top of the module. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(logos): drop dead code and a duplicate regex in logo_downloader - _SAFE_LEAGUE_CODE_RE was the same pattern as _SAFE_LEAGUE_RE; both checks use the one. - get_logo_filename_variations reassigned the TA&M case to the list it already had; the function returns the two names directly. - _get_team_name_variations() had no caller in core or the plugin monorepo. - fetch_single_team's docstring was copied from fetch_teams_data; a log message read "for{team_id}". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor: drop the Pillow<9.1 resample shim and a catch-and-reraise - adaptive_images fell back to Image.LANCZOS/NEAREST for Pillow < 9.1; requirements.txt pins Pillow>=12.2. RESAMPLE_LANCZOS and RESAMPLE_NEAREST keep their names (src.common re-exports them). - CacheManager.save_cache caught CacheError only to re-raise it; the disk write is now called directly, with the same result. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(api-helper): stop the real CacheManager's cleanup thread The cache-lifetime tests built a CacheManager and left its cleanup thread's class-wide claim on the directory in place, which broke test_cache_cleanup_thread_ownership when it ran later in the session. The fixture now stops the thread on teardown. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(changelog): core-common Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
648 lines
28 KiB
Python
648 lines
28 KiB
Python
"""
|
|
Tests for src/logo_downloader.py
|
|
|
|
Focuses on the pure/static methods that don't require network calls:
|
|
normalize_abbreviation, get_logo_filename_variations, get_logo_directory,
|
|
ensure_logo_directory, and the download_missing_logo function path
|
|
(with HTTP mocked).
|
|
"""
|
|
|
|
import io
|
|
import os
|
|
import threading
|
|
import time
|
|
|
|
import pytest
|
|
import requests
|
|
from pathlib import Path
|
|
from unittest.mock import patch, Mock, MagicMock
|
|
|
|
from PIL import Image
|
|
from PIL.PngImagePlugin import PngInfo
|
|
|
|
import src.logo_downloader as logo_downloader_module
|
|
from src.logo_downloader import (
|
|
PLACEHOLDER_BG,
|
|
PLACEHOLDER_MARKER,
|
|
PLACEHOLDER_RETRY_SECONDS,
|
|
PLACEHOLDER_SIZE,
|
|
LogoDownloader,
|
|
download_missing_logo,
|
|
is_placeholder_logo,
|
|
placeholder_age_seconds,
|
|
refresh_placeholder_timestamp,
|
|
should_attempt_download,
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# normalize_abbreviation
|
|
# ---------------------------------------------------------------------------
|
|
|
|
class TestNormalizeAbbreviation:
|
|
def test_basic_lowercase(self):
|
|
result = LogoDownloader.normalize_abbreviation("lal")
|
|
assert result == "LAL"
|
|
|
|
def test_uppercases(self):
|
|
result = LogoDownloader.normalize_abbreviation("bos")
|
|
assert result == "BOS"
|
|
|
|
def test_ampersand_replaced(self):
|
|
result = LogoDownloader.normalize_abbreviation("TA&M")
|
|
assert "&" not in result
|
|
assert "AND" in result
|
|
|
|
def test_forward_slash_replaced(self):
|
|
result = LogoDownloader.normalize_abbreviation("A/B")
|
|
assert "/" not in result
|
|
|
|
def test_empty_returns_empty(self):
|
|
result = LogoDownloader.normalize_abbreviation("")
|
|
assert result == ""
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# get_logo_filename_variations
|
|
# ---------------------------------------------------------------------------
|
|
|
|
class TestGetLogoFilenameVariations:
|
|
def test_returns_list(self):
|
|
result = LogoDownloader.get_logo_filename_variations("LAL")
|
|
assert isinstance(result, list)
|
|
assert len(result) > 0
|
|
|
|
def test_includes_png(self):
|
|
result = LogoDownloader.get_logo_filename_variations("KC")
|
|
filenames = " ".join(result)
|
|
assert ".png" in filenames
|
|
|
|
def test_includes_original(self):
|
|
result = LogoDownloader.get_logo_filename_variations("LAL")
|
|
assert any("LAL" in f for f in result)
|
|
|
|
def test_ampersand_variation(self):
|
|
result = LogoDownloader.get_logo_filename_variations("TA&M")
|
|
# Should produce at least the normalized version
|
|
assert len(result) > 0
|
|
|
|
def test_empty_string_no_crash(self):
|
|
result = LogoDownloader.get_logo_filename_variations("")
|
|
assert isinstance(result, list)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# get_logo_directory
|
|
# ---------------------------------------------------------------------------
|
|
|
|
class TestGetLogoDirectory:
|
|
def test_known_sport_returns_string(self):
|
|
downloader = LogoDownloader()
|
|
result = downloader.get_logo_directory("nfl")
|
|
assert isinstance(result, str)
|
|
assert len(result) > 0
|
|
|
|
def test_known_sport_nba(self):
|
|
downloader = LogoDownloader()
|
|
result = downloader.get_logo_directory("nba")
|
|
assert "nba" in result.lower() or "sports" in result.lower()
|
|
|
|
def test_unknown_sport_returns_string(self):
|
|
downloader = LogoDownloader()
|
|
result = downloader.get_logo_directory("unknown_sport_xyz")
|
|
assert isinstance(result, str)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# ensure_logo_directory
|
|
# ---------------------------------------------------------------------------
|
|
|
|
class TestEnsureLogoDirectory:
|
|
def test_creates_writable_directory(self, tmp_path):
|
|
downloader = LogoDownloader()
|
|
test_dir = str(tmp_path / "logos" / "nfl")
|
|
result = downloader.ensure_logo_directory(test_dir)
|
|
assert result is True
|
|
assert Path(test_dir).is_dir()
|
|
|
|
def test_existing_writable_directory(self, tmp_path):
|
|
downloader = LogoDownloader()
|
|
test_dir = str(tmp_path)
|
|
result = downloader.ensure_logo_directory(test_dir)
|
|
assert result is True
|
|
|
|
def test_returns_false_when_write_test_fails(self, tmp_path):
|
|
"""Simulate a directory that exists but raises PermissionError on write."""
|
|
downloader = LogoDownloader()
|
|
test_dir = str(tmp_path / "logos")
|
|
|
|
import builtins
|
|
original_open = builtins.open
|
|
|
|
def mock_open(path, *args, **kwargs):
|
|
if ".write_test" in str(path):
|
|
raise PermissionError("no write access")
|
|
return original_open(path, *args, **kwargs)
|
|
|
|
with patch("builtins.open", side_effect=mock_open):
|
|
result = downloader.ensure_logo_directory(test_dir)
|
|
assert result is False
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Placeholder detection and retry
|
|
#
|
|
# A failed download used to be cached as a placeholder wearing the real logo's
|
|
# filename, and download_missing_logo returned early on "the file exists". One
|
|
# transient failure therefore pinned a team to a grey box permanently.
|
|
# ---------------------------------------------------------------------------
|
|
|
|
class TestPlaceholderLogos:
|
|
def _placeholder(self, tmp_path, abbrev="COLL"):
|
|
downloader = LogoDownloader()
|
|
assert downloader.create_placeholder_logo(abbrev, str(tmp_path)) is True
|
|
return tmp_path / f"{abbrev}.png"
|
|
|
|
def test_generated_placeholder_is_recognised(self, tmp_path):
|
|
assert is_placeholder_logo(self._placeholder(tmp_path)) is True
|
|
|
|
def test_real_logo_is_not_a_placeholder(self, tmp_path):
|
|
real = tmp_path / "REAL.png"
|
|
Image.new("RGBA", (500, 500), (12, 34, 56, 255)).save(real)
|
|
assert is_placeholder_logo(real) is False
|
|
|
|
def test_legacy_unmarked_placeholder_is_recognised(self, tmp_path):
|
|
"""Placeholders written before the marker existed must still be caught.
|
|
|
|
They are already sitting on users' disks; if they were not recognised
|
|
those teams would stay grey boxes forever even after this fix.
|
|
"""
|
|
legacy = tmp_path / "LEGACY.png"
|
|
Image.new("RGBA", PLACEHOLDER_SIZE, PLACEHOLDER_BG).save(legacy)
|
|
assert is_placeholder_logo(legacy) is True
|
|
|
|
def test_same_size_but_different_colour_is_not_a_placeholder(self, tmp_path):
|
|
real = tmp_path / "SMALL.png"
|
|
Image.new("RGBA", PLACEHOLDER_SIZE, (10, 200, 10, 255)).save(real)
|
|
assert is_placeholder_logo(real) is False
|
|
|
|
def test_missing_file_is_not_a_placeholder(self, tmp_path):
|
|
assert is_placeholder_logo(tmp_path / "nope.png") is False
|
|
|
|
def test_existing_real_logo_short_circuits_without_downloading(self, tmp_path):
|
|
real = tmp_path / "REAL.png"
|
|
Image.new("RGBA", (500, 500), (1, 2, 3, 255)).save(real)
|
|
with patch.object(LogoDownloader, "download_logo") as download:
|
|
assert download_missing_logo(
|
|
"afl", "1", "REAL", real, logo_url="http://example/x.png") is True
|
|
download.assert_not_called()
|
|
|
|
def _age_placeholder(self, path, seconds):
|
|
"""Rewrite a placeholder's marker so it reads as `seconds` old."""
|
|
metadata = PngInfo()
|
|
metadata.add_text(PLACEHOLDER_MARKER, str(time.time() - seconds))
|
|
with Image.open(path) as img:
|
|
img.copy().save(path, "PNG", pnginfo=metadata)
|
|
|
|
def test_stale_placeholder_triggers_a_retry(self, tmp_path):
|
|
path = self._placeholder(tmp_path)
|
|
self._age_placeholder(path, PLACEHOLDER_RETRY_SECONDS + 60)
|
|
assert placeholder_age_seconds(path) > PLACEHOLDER_RETRY_SECONDS
|
|
|
|
with patch.object(LogoDownloader, "download_logo", return_value=True) as download:
|
|
assert download_missing_logo(
|
|
"afl", "1", "COLL", path,
|
|
logo_url="http://example/coll.png") is True
|
|
download.assert_called_once()
|
|
|
|
def test_placeholder_age_survives_an_mtime_touch(self, tmp_path):
|
|
"""The age comes from the stamp, not the filesystem.
|
|
|
|
Anything that rewrites file times -- a backup restore, an rsync, a
|
|
permissions fix script -- would otherwise reset the retry clock.
|
|
"""
|
|
path = self._placeholder(tmp_path)
|
|
self._age_placeholder(path, PLACEHOLDER_RETRY_SECONDS + 60)
|
|
now = time.time()
|
|
os.utime(path, (now, now))
|
|
assert placeholder_age_seconds(path) > PLACEHOLDER_RETRY_SECONDS
|
|
|
|
def test_fresh_placeholder_does_not_retry(self, tmp_path):
|
|
"""Rate limiting: a placeholder written seconds ago must not re-download.
|
|
|
|
Without this the fix would trade a permanent grey box for an ESPN
|
|
request on every frame.
|
|
"""
|
|
path = self._placeholder(tmp_path)
|
|
with patch.object(LogoDownloader, "download_logo") as download:
|
|
assert download_missing_logo(
|
|
"afl", "1", "COLL", path,
|
|
logo_url="http://example/coll.png") is True
|
|
download.assert_not_called()
|
|
|
|
|
|
class TestDownloadEligibility:
|
|
"""One rule, shared by every download site.
|
|
|
|
The two bulk loops and the single-logo path each had their own idea of what
|
|
counted as "already have it", which is how one of them ended up retrying
|
|
fresh placeholders and the other skipping stale ones forever.
|
|
"""
|
|
|
|
def _placeholder(self, tmp_path, abbrev="COLL"):
|
|
assert LogoDownloader().create_placeholder_logo(abbrev, str(tmp_path))
|
|
return tmp_path / f"{abbrev}.png"
|
|
|
|
def _age(self, path, seconds):
|
|
metadata = PngInfo()
|
|
metadata.add_text(PLACEHOLDER_MARKER, str(time.time() - seconds))
|
|
with Image.open(path) as img:
|
|
img.copy().save(path, "PNG", pnginfo=metadata)
|
|
|
|
def test_missing_file_is_eligible(self, tmp_path):
|
|
assert should_attempt_download(tmp_path / "nope.png") is True
|
|
|
|
def test_real_logo_is_not_eligible(self, tmp_path):
|
|
real = tmp_path / "REAL.png"
|
|
Image.new("RGBA", (500, 500), (1, 2, 3, 255)).save(real)
|
|
assert should_attempt_download(real) is False
|
|
|
|
def test_force_download_beats_a_real_logo(self, tmp_path):
|
|
real = tmp_path / "REAL.png"
|
|
Image.new("RGBA", (500, 500), (1, 2, 3, 255)).save(real)
|
|
assert should_attempt_download(real, force_download=True) is True
|
|
|
|
def test_fresh_placeholder_is_not_eligible(self, tmp_path):
|
|
assert should_attempt_download(self._placeholder(tmp_path)) is False
|
|
|
|
def test_stale_placeholder_is_eligible(self, tmp_path):
|
|
path = self._placeholder(tmp_path)
|
|
self._age(path, PLACEHOLDER_RETRY_SECONDS + 60)
|
|
assert should_attempt_download(path) is True
|
|
|
|
def test_league_bulk_loop_skips_a_fresh_placeholder(self, tmp_path):
|
|
"""A bulk pass honours the same back-off as everything else."""
|
|
self._placeholder(tmp_path, "AAA")
|
|
downloader = LogoDownloader()
|
|
teams = [{"abbreviation": "AAA", "display_name": "A", "logo_url": "http://x/a.png"}]
|
|
with patch.object(LogoDownloader, "get_logo_directory", return_value=str(tmp_path)):
|
|
with patch.object(LogoDownloader, "fetch_teams_data", return_value={"sports": [{}]}):
|
|
with patch.object(LogoDownloader, "extract_teams_from_data", return_value=teams):
|
|
with patch.object(LogoDownloader, "download_logo") as download:
|
|
downloader.download_missing_logos_for_league("nfl")
|
|
download.assert_not_called()
|
|
|
|
def test_league_bulk_loop_retries_a_stale_placeholder(self, tmp_path):
|
|
path = self._placeholder(tmp_path, "AAA")
|
|
self._age(path, PLACEHOLDER_RETRY_SECONDS + 60)
|
|
downloader = LogoDownloader()
|
|
teams = [{"abbreviation": "AAA", "display_name": "A", "logo_url": "http://x/a.png"}]
|
|
with patch.object(LogoDownloader, "get_logo_directory", return_value=str(tmp_path)):
|
|
with patch.object(LogoDownloader, "fetch_teams_data", return_value={"sports": [{}]}):
|
|
with patch.object(LogoDownloader, "extract_teams_from_data", return_value=teams):
|
|
with patch.object(LogoDownloader, "download_logo", return_value=True) as download:
|
|
downloader.download_missing_logos_for_league("nfl")
|
|
download.assert_called_once()
|
|
|
|
def test_ncaa_bulk_loop_retries_a_stale_placeholder(self, tmp_path):
|
|
"""This loop skipped placeholders forever; it now shares the rule."""
|
|
path = self._placeholder(tmp_path, "AAA")
|
|
self._age(path, PLACEHOLDER_RETRY_SECONDS + 60)
|
|
downloader = LogoDownloader()
|
|
teams = [{"abbreviation": "AAA", "display_name": "A",
|
|
"logo_url": "http://x/a.png", "category": "FBS",
|
|
"conference": "SEC"}]
|
|
with patch.object(LogoDownloader, "get_logo_directory", return_value=str(tmp_path)):
|
|
with patch.object(LogoDownloader, "fetch_teams_data", return_value={"sports": [{}]}):
|
|
with patch.object(LogoDownloader, "extract_teams_from_data", return_value=teams):
|
|
with patch.object(LogoDownloader, "download_logo", return_value=True) as download:
|
|
downloader.download_all_ncaa_football_logos()
|
|
download.assert_called_once()
|
|
|
|
def test_ncaa_bulk_loop_skips_a_fresh_placeholder(self, tmp_path):
|
|
self._placeholder(tmp_path, "AAA")
|
|
downloader = LogoDownloader()
|
|
teams = [{"abbreviation": "AAA", "display_name": "A",
|
|
"logo_url": "http://x/a.png", "category": "FBS",
|
|
"conference": "SEC"}]
|
|
with patch.object(LogoDownloader, "get_logo_directory", return_value=str(tmp_path)):
|
|
with patch.object(LogoDownloader, "fetch_teams_data", return_value={"sports": [{}]}):
|
|
with patch.object(LogoDownloader, "extract_teams_from_data", return_value=teams):
|
|
with patch.object(LogoDownloader, "download_logo") as download:
|
|
downloader.download_all_ncaa_football_logos()
|
|
download.assert_not_called()
|
|
|
|
|
|
class TestRefreshPlaceholderTimestamp:
|
|
def test_restarts_the_back_off(self, tmp_path):
|
|
assert LogoDownloader().create_placeholder_logo("COLL", str(tmp_path))
|
|
path = tmp_path / "COLL.png"
|
|
metadata = PngInfo()
|
|
metadata.add_text(PLACEHOLDER_MARKER, str(time.time() - (PLACEHOLDER_RETRY_SECONDS + 60)))
|
|
with Image.open(path) as img:
|
|
img.copy().save(path, "PNG", pnginfo=metadata)
|
|
assert should_attempt_download(path) is True
|
|
|
|
assert refresh_placeholder_timestamp(path) is True
|
|
assert should_attempt_download(path) is False
|
|
|
|
def test_refuses_to_touch_a_real_logo(self, tmp_path):
|
|
real = tmp_path / "REAL.png"
|
|
Image.new("RGBA", (500, 500), (1, 2, 3, 255)).save(real)
|
|
before = real.read_bytes()
|
|
assert refresh_placeholder_timestamp(real) is False
|
|
assert real.read_bytes() == before
|
|
|
|
def test_missing_file_is_not_an_error(self, tmp_path):
|
|
assert refresh_placeholder_timestamp(tmp_path / "nope.png") is False
|
|
|
|
|
|
class TestFailurePaths:
|
|
def test_a_team_without_logos_is_a_failed_download(self, tmp_path):
|
|
downloader = LogoDownloader()
|
|
with patch.object(downloader, "fetch_single_team",
|
|
return_value={"team": {"logos": []}}):
|
|
assert downloader.download_missing_logo_for_team(
|
|
"nfl", "1", "XYZ", tmp_path / "XYZ.png") is False
|
|
|
|
def test_placeholder_is_written_where_the_caller_looks(self, tmp_path):
|
|
"""A path that is not <normalized abbreviation>.png (the plugin's own
|
|
file naming, or an abbreviation normalize_abbreviation rewrites)
|
|
still ends up holding the placeholder, so True means it exists."""
|
|
logo_path = tmp_path / "TA&M.png"
|
|
with patch.object(LogoDownloader, "download_logo", return_value=False):
|
|
assert download_missing_logo(
|
|
"ncaa_fb", "245", "TA&M", logo_path,
|
|
logo_url="http://example/tamu.png") is True
|
|
assert is_placeholder_logo(logo_path)
|
|
assert not (tmp_path / "TAANDM.png").exists()
|
|
|
|
def test_placeholder_uses_the_placeholder_geometry(self, tmp_path):
|
|
assert LogoDownloader().create_placeholder_logo("AB", str(tmp_path))
|
|
with Image.open(tmp_path / "AB.png") as img:
|
|
assert img.size == PLACEHOLDER_SIZE
|
|
assert img.convert("RGBA").getpixel((0, 0)) == PLACEHOLDER_BG
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# download_logo: the download the scoreboard plugins actually use
|
|
#
|
|
# It used to read response.content with no size cap, write straight to the
|
|
# final path (so a failed or corrupt download could be left there and then be
|
|
# cached as the logo), and build a fresh Session for every logo. It now goes
|
|
# through fetch_logo, the hardened download LogoHelper also uses.
|
|
# ---------------------------------------------------------------------------
|
|
|
|
def _png(img: Image.Image) -> bytes:
|
|
buf = io.BytesIO()
|
|
img.save(buf, "PNG")
|
|
return buf.getvalue()
|
|
|
|
|
|
def _stream(body: bytes, content_type: str = "image/png", chunk: int = 1024,
|
|
die_after: int | None = None):
|
|
"""A streamed requests.Response stand-in.
|
|
|
|
``die_after`` makes the transfer fail after that many bytes, the way a
|
|
reset connection does mid-body.
|
|
"""
|
|
response = MagicMock()
|
|
response.__enter__.return_value = response
|
|
response.__exit__.return_value = False
|
|
response.raise_for_status = MagicMock()
|
|
response.headers = {"content-type": content_type}
|
|
|
|
def _iter_content(*_args, **_kwargs):
|
|
sent = 0
|
|
for i in range(0, len(body), chunk):
|
|
if die_after is not None and sent >= die_after:
|
|
raise requests.exceptions.ChunkedEncodingError("connection reset")
|
|
piece = body[i:i + chunk]
|
|
sent += len(piece)
|
|
yield piece
|
|
|
|
response.iter_content = _iter_content
|
|
return response
|
|
|
|
|
|
@pytest.fixture
|
|
def fresh_thread_state(monkeypatch):
|
|
"""Each test gets its own per-thread downloader cache."""
|
|
monkeypatch.setattr(logo_downloader_module, "_thread_state", threading.local())
|
|
|
|
|
|
@pytest.fixture
|
|
def downloader():
|
|
return LogoDownloader()
|
|
|
|
|
|
@pytest.fixture
|
|
def old_logo(tmp_path):
|
|
"""A logo already on disk that a failed re-download must not damage."""
|
|
path = tmp_path / "PHI.png"
|
|
Image.new("RGBA", (30, 30), (1, 2, 3, 255)).save(path)
|
|
return path, path.read_bytes()
|
|
|
|
|
|
def _leftovers(directory: Path, keep: str | None = None):
|
|
return sorted(p.name for p in directory.iterdir() if p.name != keep)
|
|
|
|
|
|
class TestDownloadLogoHardening:
|
|
def test_valid_logo_is_saved_as_rgba_png(self, downloader, tmp_path):
|
|
target = tmp_path / "PHI.png"
|
|
downloader.session.get = MagicMock(
|
|
return_value=_stream(_png(Image.new("RGB", (20, 10), (9, 8, 7)))))
|
|
assert downloader.download_logo("http://x/phi.png", target, "PHI") is True
|
|
with Image.open(target) as img:
|
|
assert img.format == "PNG" and img.mode == "RGBA"
|
|
assert img.getpixel((0, 0)) == (9, 8, 7, 255)
|
|
assert _leftovers(tmp_path, keep="PHI.png") == []
|
|
# Streamed, so the size cap applies before the body is buffered.
|
|
assert downloader.session.get.call_args.kwargs["stream"] is True
|
|
|
|
def test_oversized_response_is_rejected_and_leaves_no_file(
|
|
self, downloader, tmp_path, monkeypatch):
|
|
monkeypatch.setattr(logo_downloader_module, "MAX_LOGO_BYTES", 4096)
|
|
target = tmp_path / "BIG.png"
|
|
body = _png(Image.new("RGB", (8, 8))) + b"\x00" * 8192
|
|
downloader.session.get = MagicMock(return_value=_stream(body))
|
|
assert downloader.download_logo("http://x/big.png", target, "BIG") is False
|
|
assert list(tmp_path.iterdir()) == []
|
|
|
|
def test_oversized_response_does_not_replace_an_existing_logo(
|
|
self, downloader, old_logo, monkeypatch):
|
|
path, before = old_logo
|
|
monkeypatch.setattr(logo_downloader_module, "MAX_LOGO_BYTES", 4096)
|
|
downloader.session.get = MagicMock(
|
|
return_value=_stream(b"\x89PNG" + b"\x00" * 8192))
|
|
assert downloader.download_logo("http://x/big.png", path, "PHI") is False
|
|
assert path.read_bytes() == before
|
|
assert _leftovers(path.parent, keep=path.name) == []
|
|
|
|
def test_mid_download_failure_leaves_no_partial_file(self, downloader, tmp_path):
|
|
target = tmp_path / "CUT.png"
|
|
body = _png(Image.new("RGB", (200, 200), (5, 5, 5))) + b"\x00" * 4096
|
|
downloader.session.get = MagicMock(
|
|
return_value=_stream(body, chunk=256, die_after=512))
|
|
assert downloader.download_logo("http://x/cut.png", target, "CUT") is False
|
|
assert list(tmp_path.iterdir()) == []
|
|
|
|
def test_mid_download_failure_keeps_the_previous_logo(self, downloader, old_logo):
|
|
# The old code opened the final path for writing before the body
|
|
# arrived, so a dropped connection truncated the logo it was replacing.
|
|
path, before = old_logo
|
|
body = _png(Image.new("RGB", (200, 200), (5, 5, 5))) + b"\x00" * 4096
|
|
downloader.session.get = MagicMock(
|
|
return_value=_stream(body, chunk=256, die_after=512))
|
|
assert downloader.download_logo("http://x/cut.png", path, "PHI") is False
|
|
assert path.read_bytes() == before
|
|
assert _leftovers(path.parent, keep=path.name) == []
|
|
|
|
def test_non_image_content_type_is_rejected(self, downloader, old_logo):
|
|
# Rejected on the label alone, before the body is trusted: these bytes
|
|
# would decode, so only the content-type check stops them.
|
|
path, before = old_logo
|
|
body = _png(Image.new("RGB", (8, 8), (250, 0, 0)))
|
|
downloader.session.get = MagicMock(
|
|
return_value=_stream(body, content_type="text/html"))
|
|
assert downloader.download_logo("http://x/404", path, "PHI") is False
|
|
assert path.read_bytes() == before
|
|
assert _leftovers(path.parent, keep=path.name) == []
|
|
|
|
def test_bytes_that_do_not_decode_are_rejected(self, downloader, old_logo):
|
|
# A server that labels an error page image/png must not get it cached.
|
|
path, before = old_logo
|
|
downloader.session.get = MagicMock(
|
|
return_value=_stream(b"<html>oops</html>", content_type="image/png"))
|
|
assert downloader.download_logo("http://x/lie.png", path, "PHI") is False
|
|
assert path.read_bytes() == before
|
|
assert _leftovers(path.parent, keep=path.name) == []
|
|
|
|
def test_http_error_keeps_the_previous_logo(self, downloader, old_logo):
|
|
path, before = old_logo
|
|
response = _stream(b"")
|
|
response.raise_for_status.side_effect = requests.exceptions.HTTPError("503")
|
|
downloader.session.get = MagicMock(return_value=response)
|
|
assert downloader.download_logo("http://x/phi.png", path, "PHI") is False
|
|
assert path.read_bytes() == before
|
|
assert _leftovers(path.parent, keep=path.name) == []
|
|
|
|
def test_unwritable_directory_returns_false(self, downloader, tmp_path):
|
|
downloader.session.get = MagicMock()
|
|
with patch("src.logo_downloader.tempfile.mkstemp",
|
|
side_effect=PermissionError("read-only")):
|
|
assert downloader.download_logo(
|
|
"http://x/phi.png", tmp_path / "PHI.png", "PHI") is False
|
|
downloader.session.get.assert_not_called()
|
|
|
|
|
|
class TestDownloadLogoTransparency:
|
|
"""Plugins paste logos with the image as its own mask; alpha must survive."""
|
|
|
|
def _download(self, downloader, tmp_path, body, content_type="image/png"):
|
|
target = tmp_path / "LOGO.png"
|
|
downloader.session.get = MagicMock(return_value=_stream(body, content_type))
|
|
assert downloader.download_logo("http://x/logo", target, "LOGO") is True
|
|
with Image.open(target) as img:
|
|
img.load()
|
|
return img.copy(), img.format
|
|
|
|
def test_rgba_alpha_is_kept_exactly(self, downloader, tmp_path):
|
|
src = Image.new("RGBA", (16, 16), (0, 0, 0, 0))
|
|
for x in range(16):
|
|
src.putpixel((x, 3), (200, 100, 50, x * 16))
|
|
out, _ = self._download(downloader, tmp_path, _png(src))
|
|
assert out.mode == "RGBA"
|
|
assert list(out.getdata()) == list(src.getdata())
|
|
|
|
def test_palette_transparency_becomes_alpha(self, downloader, tmp_path):
|
|
src = Image.new("P", (8, 8), 0)
|
|
src.putpalette([0, 0, 0, 255, 0, 0] + [0] * (254 * 3))
|
|
src.putpixel((4, 4), 1)
|
|
buf = io.BytesIO()
|
|
src.save(buf, "PNG", transparency=0)
|
|
out, _ = self._download(downloader, tmp_path, buf.getvalue())
|
|
assert out.mode == "RGBA"
|
|
assert out.getpixel((0, 0))[3] == 0
|
|
assert out.getpixel((4, 4)) == (255, 0, 0, 255)
|
|
|
|
def test_greyscale_transparency_becomes_alpha(self, downloader, tmp_path):
|
|
src = Image.new("L", (8, 8), 0)
|
|
src.putpixel((2, 2), 255)
|
|
buf = io.BytesIO()
|
|
src.save(buf, "PNG", transparency=0)
|
|
out, _ = self._download(downloader, tmp_path, buf.getvalue())
|
|
assert out.getpixel((0, 0))[3] == 0
|
|
assert out.getpixel((2, 2)) == (255, 255, 255, 255)
|
|
|
|
def test_jpeg_is_stored_as_opaque_rgba_png(self, downloader, tmp_path):
|
|
buf = io.BytesIO()
|
|
Image.new("RGB", (8, 8), (10, 200, 30)).save(buf, "JPEG", quality=95)
|
|
out, fmt = self._download(downloader, tmp_path, buf.getvalue(), "image/jpeg")
|
|
assert fmt == "PNG" and out.mode == "RGBA"
|
|
assert out.getchannel("A").getextrema() == (255, 255)
|
|
|
|
|
|
class TestSharedDownloader:
|
|
def test_download_missing_logo_reuses_one_session(self, tmp_path, fresh_thread_state):
|
|
sessions = []
|
|
real_session = requests.Session
|
|
|
|
def counting_session(*args, **kwargs):
|
|
s = real_session(*args, **kwargs)
|
|
sessions.append(s)
|
|
return s
|
|
|
|
with patch("src.logo_downloader.requests.Session", side_effect=counting_session):
|
|
with patch.object(LogoDownloader, "download_logo", return_value=True) as dl:
|
|
for abbr in ("AAA", "BBB", "CCC"):
|
|
assert download_missing_logo(
|
|
"nfl", "1", abbr, tmp_path / f"{abbr}.png",
|
|
logo_url=f"http://x/{abbr}.png",
|
|
create_placeholder=False) is True
|
|
assert dl.call_count == 3
|
|
assert len(sessions) == 1
|
|
|
|
def test_each_thread_gets_its_own_downloader(self, fresh_thread_state):
|
|
here = logo_downloader_module.shared_downloader()
|
|
assert logo_downloader_module.shared_downloader() is here
|
|
seen = []
|
|
t = threading.Thread(target=lambda: seen.append(
|
|
logo_downloader_module.shared_downloader()))
|
|
t.start()
|
|
t.join()
|
|
assert seen and seen[0] is not here
|
|
assert seen[0].session is not here.session
|
|
|
|
|
|
class TestPlaceholderWrite:
|
|
def test_no_write_probe_file_is_created(self, tmp_path):
|
|
created = []
|
|
real_touch = Path.touch
|
|
|
|
def spy_touch(self, *args, **kwargs):
|
|
created.append(self.name)
|
|
return real_touch(self, *args, **kwargs)
|
|
|
|
with patch.object(Path, "touch", spy_touch):
|
|
assert LogoDownloader().create_placeholder_logo("COLL", str(tmp_path)) is True
|
|
assert "test_write.tmp" not in created
|
|
assert sorted(p.name for p in tmp_path.iterdir()) == ["COLL.png"]
|
|
|
|
def test_unwritable_directory_returns_false(self, tmp_path):
|
|
with patch.object(LogoDownloader, "ensure_logo_directory", return_value=True), \
|
|
patch("src.logo_downloader.tempfile.mkstemp",
|
|
side_effect=PermissionError("read-only")):
|
|
assert LogoDownloader().create_placeholder_logo("COLL", str(tmp_path)) is False
|
|
assert list(tmp_path.iterdir()) == []
|
|
|
|
def test_failed_save_keeps_the_previous_file(self, tmp_path):
|
|
path = tmp_path / "COLL.png"
|
|
Image.new("RGBA", (30, 30), (1, 2, 3, 255)).save(path)
|
|
before = path.read_bytes()
|
|
with patch("src.logo_downloader.os.replace", side_effect=OSError("disk full")):
|
|
assert LogoDownloader().create_placeholder_logo("COLL", str(tmp_path)) is False
|
|
assert path.read_bytes() == before
|
|
assert sorted(p.name for p in tmp_path.iterdir()) == ["COLL.png"]
|