From b1280dc19345ae0919e749b3c0336b0e1946f739 Mon Sep 17 00:00:00 2001 From: David Gwilliam Date: Sun, 15 Mar 2026 15:44:39 -0700 Subject: [PATCH] refactor: phase 1 - testability improvements - Add Config dataclass with get_config()/set_config() for injection - Add Config.from_args() for CLI argument parsing (testable) - Add platform font path detection (Darwin/Linux) - Bound translate cache with @lru_cache(maxsize=500) - Add fixtures for external dependencies (network, feeds, config) - Add 15 tests for Config class, from_args, and platform detection This enables testability by: - Allowing config injection instead of global mutable state - Supporting custom argv in from_args() for testing - Providing reusable fixtures for mocking network/filesystem - Preventing unbounded memory growth in translation cache Fixes: _arg_value/_arg_int not accepting custom argv --- engine/config.py | 143 +++++++++++++++++++++- engine/translate.py | 35 +++--- tests/fixtures/__init__.py | 236 +++++++++++++++++++++++++++++++++++++ tests/test_config.py | 137 +++++++++++++++++++++ 4 files changed, 529 insertions(+), 22 deletions(-) create mode 100644 tests/fixtures/__init__.py diff --git a/engine/config.py b/engine/config.py index 005f5b8..c5ce46c 100644 --- a/engine/config.py +++ b/engine/config.py @@ -1,25 +1,28 @@ """ Configuration constants, CLI flags, and glyph tables. +Supports both global constants (backward compatible) and injected config for testing. """ import sys +from dataclasses import dataclass, field from pathlib import Path _REPO_ROOT = Path(__file__).resolve().parent.parent _FONT_EXTENSIONS = {".otf", ".ttf", ".ttc"} -def _arg_value(flag): +def _arg_value(flag, argv: list[str] | None = None): """Get value following a CLI flag, if present.""" - if flag not in sys.argv: + argv = argv or sys.argv + if flag not in argv: return None - i = sys.argv.index(flag) - return sys.argv[i + 1] if i + 1 < len(sys.argv) else None + i = argv.index(flag) + return argv[i + 1] if i + 1 < len(argv) else None -def _arg_int(flag, default): +def _arg_int(flag, default, argv: list[str] | None = None): """Get int CLI argument with safe fallback.""" - raw = _arg_value(flag) + raw = _arg_value(flag, argv) if raw is None: return default try: @@ -53,6 +56,134 @@ def list_repo_font_files(): return _list_font_files(FONT_DIR) +def _get_platform_font_paths() -> dict[str, str]: + """Get platform-appropriate font paths for non-Latin scripts.""" + import platform + + system = platform.system() + + if system == "Darwin": + return { + "zh-cn": "/System/Library/Fonts/STHeiti Medium.ttc", + "ja": "/System/Library/Fonts/ヒラギノ角ゴシック W9.ttc", + "ko": "/System/Library/Fonts/AppleSDGothicNeo.ttc", + "ru": "/System/Library/Fonts/Supplemental/Arial.ttf", + "uk": "/System/Library/Fonts/Supplemental/Arial.ttf", + "el": "/System/Library/Fonts/Supplemental/Arial.ttf", + "he": "/System/Library/Fonts/Supplemental/Arial.ttf", + "ar": "/System/Library/Fonts/GeezaPro.ttc", + "fa": "/System/Library/Fonts/GeezaPro.ttc", + "hi": "/System/Library/Fonts/Kohinoor.ttc", + "th": "/System/Library/Fonts/ThonburiUI.ttc", + } + elif system == "Linux": + return { + "zh-cn": "/usr/share/fonts/truetype/noto/NotoSansCJK-Regular.ttc", + "ja": "/usr/share/fonts/truetype/noto/NotoSansCJK-Regular.ttc", + "ko": "/usr/share/fonts/truetype/noto/NotoSansCJK-Regular.ttc", + "ru": "/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf", + "uk": "/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf", + "el": "/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf", + "he": "/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf", + "ar": "/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf", + "fa": "/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf", + "hi": "/usr/share/fonts/truetype/noto/NotoSansDevanagari-Regular.ttf", + "th": "/usr/share/fonts/truetype/noto/NotoSansThai-Regular.ttf", + } + else: + return {} + + +@dataclass(frozen=True) +class Config: + """Immutable configuration container for injected config.""" + + headline_limit: int = 1000 + feed_timeout: int = 10 + mic_threshold_db: int = 50 + mode: str = "news" + firehose: bool = False + + ntfy_topic: str = "https://ntfy.sh/klubhaus_terminal_mainline/json" + ntfy_reconnect_delay: int = 5 + message_display_secs: int = 30 + + font_dir: str = "fonts" + font_path: str = "" + font_index: int = 0 + font_picker: bool = True + font_sz: int = 60 + render_h: int = 8 + + ssaa: int = 4 + + scroll_dur: float = 5.625 + frame_dt: float = 0.05 + firehose_h: int = 12 + grad_speed: float = 0.08 + + glitch_glyphs: str = "░▒▓█▌▐╌╍╎╏┃┆┇┊┋" + kata_glyphs: str = "ﾊﾐﾋｰｳｼﾅﾓﾆｻﾜﾂｵﾘｱﾎﾃﾏｹﾒｴｶｷﾑﾕﾗｾﾈｽﾀﾇﾍ" + + script_fonts: dict[str, str] = field(default_factory=_get_platform_font_paths) + + @classmethod + def from_args(cls, argv: list[str] | None = None) -> "Config": + """Create Config from CLI arguments (or custom argv for testing).""" + argv = argv or sys.argv + + font_dir = _resolve_font_path(_arg_value("--font-dir", argv) or "fonts") + font_file_arg = _arg_value("--font-file", argv) + font_files = _list_font_files(font_dir) + font_path = ( + _resolve_font_path(font_file_arg) + if font_file_arg + else (font_files[0] if font_files else "") + ) + + return cls( + headline_limit=1000, + feed_timeout=10, + mic_threshold_db=50, + mode="poetry" if "--poetry" in argv or "-p" in argv else "news", + firehose="--firehose" in argv, + ntfy_topic="https://ntfy.sh/klubhaus_terminal_mainline/json", + ntfy_reconnect_delay=5, + message_display_secs=30, + font_dir=font_dir, + font_path=font_path, + font_index=max(0, _arg_int("--font-index", 0, argv)), + font_picker="--no-font-picker" not in argv, + font_sz=60, + render_h=8, + ssaa=4, + scroll_dur=5.625, + frame_dt=0.05, + firehose_h=12, + grad_speed=0.08, + glitch_glyphs="░▒▓█▌▐╌╍╎╏┃┆┇┊┋", + kata_glyphs="ﾊﾐﾋｰｳｼﾅﾓﾆｻﾜﾂｵﾘｱﾎﾃﾏｹﾒｴｶｷﾑﾕﾗｾﾈｽﾀﾇﾍ", + script_fonts=_get_platform_font_paths(), + ) + + +_config: Config | None = None + + +def get_config() -> Config: + """Get the global config instance (lazy-loaded).""" + global _config + if _config is None: + _config = Config.from_args() + return _config + + +def set_config(config: Config) -> None: + """Set the global config instance (for testing).""" + global _config + _config = config + + # ─── RUNTIME ────────────────────────────────────────────── HEADLINE_LIMIT = 1000 FEED_TIMEOUT = 10 diff --git a/engine/translate.py b/engine/translate.py index eb1f2ca..04e04dd 100644 --- a/engine/translate.py +++ b/engine/translate.py @@ -7,26 +7,16 @@ import json import re import urllib.parse import urllib.request +from functools import lru_cache from engine.sources import LOCATION_LANGS -_TRANSLATE_CACHE = {} +TRANSLATE_CACHE_SIZE = 500 -def detect_location_language(title): - """Detect if headline mentions a location, return target language.""" - title_lower = title.lower() - for pattern, lang in LOCATION_LANGS.items(): - if re.search(pattern, title_lower): - return lang - return None - - -def translate_headline(title, target_lang): - """Translate headline via Google Translate API (zero dependencies).""" - key = (title, target_lang) - if key in _TRANSLATE_CACHE: - return _TRANSLATE_CACHE[key] +@lru_cache(maxsize=TRANSLATE_CACHE_SIZE) +def _translate_cached(title: str, target_lang: str) -> str: + """Cached translation implementation.""" try: q = urllib.parse.quote(title) url = ( @@ -39,5 +29,18 @@ def translate_headline(title, target_lang): result = "".join(p[0] for p in data[0] if p[0]) or title except Exception: result = title - _TRANSLATE_CACHE[key] = result return result + + +def detect_location_language(title): + """Detect if headline mentions a location, return target language.""" + title_lower = title.lower() + for pattern, lang in LOCATION_LANGS.items(): + if re.search(pattern, title_lower): + return lang + return None + + +def translate_headline(title: str, target_lang: str) -> str: + """Translate headline via Google Translate API (zero dependencies).""" + return _translate_cached(title, target_lang) diff --git a/tests/fixtures/__init__.py b/tests/fixtures/__init__.py new file mode 100644 index 0000000..68cce00 --- /dev/null +++ b/tests/fixtures/__init__.py @@ -0,0 +1,236 @@ +""" +Pytest fixtures for mocking external dependencies (network, filesystem). +""" + +import json +from unittest.mock import MagicMock + +import pytest + + +@pytest.fixture +def mock_feed_response(): + """Mock RSS feed response data.""" + return b""" + + + Test Feed + https://example.com + + Test Headline One + Sat, 15 Mar 2025 12:00:00 GMT + + + Test Headline Two + Sat, 15 Mar 2025 11:00:00 GMT + + + Sports: Team Wins Championship + Sat, 15 Mar 2025 10:00:00 GMT + + +""" + + +@pytest.fixture +def mock_gutenberg_response(): + """Mock Project Gutenberg text response.""" + return """Project Gutenberg's Collection, by Various + +*** START OF SOME TEXT *** +This is a test poem with multiple lines +that should be parsed as stanzas. + +Another stanza here with different content +and more lines to test the parsing logic. + +Yet another stanza for variety +in the test data. + +*** END OF SOME TEXT ***""" + + +@pytest.fixture +def mock_gutenberg_empty(): + """Mock Gutenberg response with no valid stanzas.""" + return """Project Gutenberg's Collection + +*** START OF TEXT *** +THIS IS ALL CAPS AND SHOULD BE SKIPPED + +I. + +*** END OF TEXT ***""" + + +@pytest.fixture +def mock_ntfy_message(): + """Mock ntfy.sh SSE message.""" + return json.dumps( + { + "id": "test123", + "event": "message", + "title": "Test Title", + "message": "Test message body", + "time": 1234567890, + } + ).encode() + + +@pytest.fixture +def mock_ntfy_keepalive(): + """Mock ntfy.sh keepalive message.""" + return b'data: {"event":"keepalive"}\n\n' + + +@pytest.fixture +def mock_google_translate_response(): + """Mock Google Translate API response.""" + return json.dumps( + [ + [["Translated text", "Original text", None, 0.8], None, "en"], + None, + None, + [], + [], + [], + [], + ] + ) + + +@pytest.fixture +def mock_feedparser(): + """Create a mock feedparser.parse function.""" + + def _mock(data): + mock_result = MagicMock() + mock_result.bozo = False + mock_result.entries = [ + { + "title": "Test Headline", + "published_parsed": (2025, 3, 15, 12, 0, 0, 0, 0, 0), + }, + { + "title": "Another Headline", + "updated_parsed": (2025, 3, 15, 11, 0, 0, 0, 0, 0), + }, + ] + return mock_result + + return _mock + + +@pytest.fixture +def mock_urllib_open(mock_feed_response): + """Create a mock urllib.request.urlopen that returns feed data.""" + + def _mock(url): + mock_response = MagicMock() + mock_response.read.return_value = mock_feed_response + return mock_response + + return _mock + + +@pytest.fixture +def sample_items(): + """Sample items as returned by fetch module (title, source, timestamp).""" + return [ + ("Headline One", "Test Source", "12:00"), + ("Headline Two", "Another Source", "11:30"), + ("Headline Three", "Third Source", "10:45"), + ] + + +@pytest.fixture +def sample_config(): + """Sample config for testing.""" + from engine.config import Config + + return Config( + headline_limit=100, + feed_timeout=10, + mic_threshold_db=50, + mode="news", + firehose=False, + ntfy_topic="https://ntfy.sh/test/json", + ntfy_reconnect_delay=5, + message_display_secs=30, + font_dir="fonts", + font_path="", + font_index=0, + font_picker=False, + font_sz=60, + render_h=8, + ssaa=4, + scroll_dur=5.625, + frame_dt=0.05, + firehose_h=12, + grad_speed=0.08, + glitch_glyphs="░▒▓█▌▐", + kata_glyphs="ﾊﾐﾋｰｳ", + script_fonts={}, + ) + + +@pytest.fixture +def poetry_config(): + """Sample config for poetry mode.""" + from engine.config import Config + + return Config( + headline_limit=100, + feed_timeout=10, + mic_threshold_db=50, + mode="poetry", + firehose=False, + ntfy_topic="https://ntfy.sh/test/json", + ntfy_reconnect_delay=5, + message_display_secs=30, + font_dir="fonts", + font_path="", + font_index=0, + font_picker=False, + font_sz=60, + render_h=8, + ssaa=4, + scroll_dur=5.625, + frame_dt=0.05, + firehose_h=12, + grad_speed=0.08, + glitch_glyphs="░▒▓█▌▐", + kata_glyphs="ﾊﾐﾋｰｳ", + script_fonts={}, + ) + + +@pytest.fixture +def firehose_config(): + """Sample config with firehose enabled.""" + from engine.config import Config + + return Config( + headline_limit=100, + feed_timeout=10, + mic_threshold_db=50, + mode="news", + firehose=True, + ntfy_topic="https://ntfy.sh/test/json", + ntfy_reconnect_delay=5, + message_display_secs=30, + font_dir="fonts", + font_path="", + font_index=0, + font_picker=False, + font_sz=60, + render_h=8, + ssaa=4, + scroll_dur=5.625, + frame_dt=0.05, + firehose_h=12, + grad_speed=0.08, + glitch_glyphs="░▒▓█▌▐", + kata_glyphs="ﾊﾐﾋｰｳ", + script_fonts={}, + ) diff --git a/tests/test_config.py b/tests/test_config.py index 79a8c28..0462d5a 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -214,3 +214,140 @@ class TestActiveTheme: second_theme = config.ACTIVE_THEME assert first_theme.name == second_theme.name assert first_theme.name == "green" + + +class TestConfigDataclass: + """Tests for Config dataclass.""" + + def test_config_has_required_fields(self): + """Config has all required fields.""" + c = config.Config() + assert hasattr(c, "headline_limit") + assert hasattr(c, "feed_timeout") + assert hasattr(c, "mic_threshold_db") + assert hasattr(c, "mode") + assert hasattr(c, "firehose") + assert hasattr(c, "ntfy_topic") + assert hasattr(c, "ntfy_reconnect_delay") + assert hasattr(c, "message_display_secs") + assert hasattr(c, "font_dir") + assert hasattr(c, "font_path") + assert hasattr(c, "font_index") + assert hasattr(c, "font_picker") + assert hasattr(c, "font_sz") + assert hasattr(c, "render_h") + assert hasattr(c, "ssaa") + assert hasattr(c, "scroll_dur") + assert hasattr(c, "frame_dt") + assert hasattr(c, "firehose_h") + assert hasattr(c, "grad_speed") + assert hasattr(c, "glitch_glyphs") + assert hasattr(c, "kata_glyphs") + assert hasattr(c, "script_fonts") + + def test_config_defaults(self): + """Config has sensible defaults.""" + c = config.Config() + assert c.headline_limit == 1000 + assert c.feed_timeout == 10 + assert c.mic_threshold_db == 50 + assert c.mode == "news" + assert c.firehose is False + assert c.ntfy_reconnect_delay == 5 + assert c.message_display_secs == 30 + + def test_config_is_immutable(self): + """Config is frozen (immutable).""" + c = config.Config() + with pytest.raises(AttributeError): + c.headline_limit = 500 # type: ignore + + def test_config_custom_values(self): + """Config accepts custom values.""" + c = config.Config( + headline_limit=500, + mode="poetry", + firehose=True, + ntfy_topic="https://ntfy.sh/test", + ) + assert c.headline_limit == 500 + assert c.mode == "poetry" + assert c.firehose is True + assert c.ntfy_topic == "https://ntfy.sh/test" + + +class TestConfigFromArgs: + """Tests for Config.from_args method.""" + + def test_from_args_defaults(self): + """from_args creates config with defaults from empty argv.""" + c = config.Config.from_args(["prog"]) + assert c.mode == "news" + assert c.firehose is False + assert c.font_picker is True + + def test_from_args_poetry_mode(self): + """from_args detects --poetry flag.""" + c = config.Config.from_args(["prog", "--poetry"]) + assert c.mode == "poetry" + + def test_from_args_poetry_short_flag(self): + """from_args detects -p short flag.""" + c = config.Config.from_args(["prog", "-p"]) + assert c.mode == "poetry" + + def test_from_args_firehose(self): + """from_args detects --firehose flag.""" + c = config.Config.from_args(["prog", "--firehose"]) + assert c.firehose is True + + def test_from_args_no_font_picker(self): + """from_args detects --no-font-picker flag.""" + c = config.Config.from_args(["prog", "--no-font-picker"]) + assert c.font_picker is False + + def test_from_args_font_index(self): + """from_args parses --font-index.""" + c = config.Config.from_args(["prog", "--font-index", "3"]) + assert c.font_index == 3 + + +class TestGetSetConfig: + """Tests for get_config and set_config functions.""" + + def test_get_config_returns_config(self): + """get_config returns a Config instance.""" + c = config.get_config() + assert isinstance(c, config.Config) + + def test_set_config_allows_injection(self): + """set_config allows injecting a custom config.""" + custom = config.Config(mode="poetry", headline_limit=100) + config.set_config(custom) + assert config.get_config().mode == "poetry" + assert config.get_config().headline_limit == 100 + + def test_set_config_then_get_config(self): + """set_config followed by get_config returns the set config.""" + original = config.get_config() + test_config = config.Config(headline_limit=42) + config.set_config(test_config) + result = config.get_config() + assert result.headline_limit == 42 + config.set_config(original) + + +class TestPlatformFontPaths: + """Tests for platform font path detection.""" + + def test_get_platform_font_paths_returns_dict(self): + """_get_platform_font_paths returns a dictionary.""" + fonts = config._get_platform_font_paths() + assert isinstance(fonts, dict) + + def test_platform_font_paths_common_languages(self): + """Common language font mappings exist.""" + fonts = config._get_platform_font_paths() + common = {"ja", "zh-cn", "ko", "ru", "ar", "hi"} + found = set(fonts.keys()) & common + assert len(found) > 0