diff --git a/src/common/sports_shared.py b/src/common/sports_shared.py index f427817d..1187b846 100644 --- a/src/common/sports_shared.py +++ b/src/common/sports_shared.py @@ -211,17 +211,52 @@ def _crisp_size(cls, font_file, desired): return desired return max(grid, int(round(float(desired) / grid)) * grid) + #: Absolute path of this plugin's directory, declared by the plugin + #: itself. The mixin cannot work it out -- see _plugin_dir. + _PLUGIN_DIR: ClassVar[Optional[str]] = None + def _plugin_dir(self) -> Optional[str]: - """Directory of the plugin that defines this class. + """Directory of the plugin that owns this instance. In sports.py these methods could just use ``__file__``. Here that is - src/common/, so the plugin's own directory has to be recovered from the - instance. ``type(self).__module__`` alone is not enough: SportsCore is - an ABC, so a subclass built with ``type(name, bases, ns)`` -- which the - plugins' own tests do -- reports its module as "abc". Walking the MRO - steps past those synthetic classes to the first one whose module sits - next to a config_schema.json, which is the real plugin. + src/common/, so the directory has to come from the plugin. + + It is TOLD, not deduced. The first version walked the MRO for a class + whose module sits beside a config_schema.json. That works when a test + imports the plugin itself, and returns None under the real loader, for + a specific reason worth recording: + + PluginLoader._namespace_plugin_modules renames every bare module a + plugin brought in (sports, game_renderer, ...) to + "_plg__" and REMOVES the bare entry, so two + plugins owning a module of the same name cannot collide. + + The class still reports ``__module__ == "sports"``, but + ``sys.modules["sports"]`` no longer exists, so the walk finds no + __file__ and falls off the end. The failure was silent and expensive: + + _plugin_dir() -> None + _schema_font_size() -> None for every element + -> a configured size equal to the schema default stops looking + like a default and is treated as a deliberate user choice + -> the snap to the font's pixel grid is skipped + -> 4x6-font.ttf renders at 6 instead of 7: 3px-wide glyphs + instead of 4px + + On a 256x64 panel that made the odds, the team records and the date row + hard to read. It was found by a user counting pixels on the panel. No + gate here caught it: the tests imported plugins directly and the + safety harness loads them its own way, so neither reproduced the + loader's renaming. + + The MRO walk stays as a fallback for hosts that declare no + _PLUGIN_DIR -- the plugins' own probe harnesses build classes with + ``type()`` -- but it is no longer the primary answer. """ + declared = getattr(self, "_PLUGIN_DIR", None) + if declared and os.path.isfile(os.path.join(declared, "config_schema.json")): + return declared + for cls in type(self).__mro__: module = sys.modules.get(getattr(cls, "__module__", ""), None) path = getattr(module, "__file__", None) diff --git a/test/test_sports_shared.py b/test/test_sports_shared.py index f18fe64d..bd402ae3 100644 --- a/test/test_sports_shared.py +++ b/test/test_sports_shared.py @@ -319,3 +319,58 @@ def test_the_streak_starts_from_absent_state(self): assert not hasattr(h, "_empty_live_streak") h._note_live_fetch(False) assert h._empty_live_streak == 1 + + +class TestPluginDirIsToldNotDeduced: + """The regression that shipped: _plugin_dir returned None in production. + + The first version walked the MRO for a class whose module sits beside a + config_schema.json. That passes when a test imports the plugin directly -- + which is how it was verified -- and returns None under the real plugin + loader, which imports modules by a path that leaves no such entry on the + MRO. + + Silent, and expensive: no schema means _schema_font_size returns None for + every element, so a configured size equal to the schema default stops + looking like a default, is treated as a deliberate choice, and skips the + grid snap. 4x6-font.ttf then renders at 6 rather than 7 -- 3px-wide glyphs + instead of 4px. On a 256x64 panel that made the odds, the records and the + date row illegible. A user counted the pixels; no gate here caught it. + """ + + def test_a_declared_directory_is_used(self, tmp_path): + d = _write_plugin(tmp_path, "declared") + host = type("H", (SportsCoreSharedMixin,), {"_PLUGIN_DIR": str(d)})() + assert host._plugin_dir() == str(d) + + def test_it_works_when_no_module_on_the_mro_helps(self, tmp_path, monkeypatch): + """The production case: nothing on the MRO sits beside a schema.""" + d = _write_plugin(tmp_path, "loaderstyle") + # A class whose module is not importable by name, as the loader produces. + cls = type("Loaded", (SportsCoreSharedMixin,), {"_PLUGIN_DIR": str(d)}) + cls.__module__ = "a.module.name.that.is.not.in.sys.modules" + assert cls.__new__(cls)._plugin_dir() == str(d), ( + "the declared directory must win when the MRO walk cannot help") + + def test_without_it_the_mro_walk_would_have_failed(self): + # Pin the precondition, so this test still means something if the + # fallback is ever changed. + cls = type("Orphan", (SportsCoreSharedMixin,), {}) + cls.__module__ = "not.a.real.module" + assert cls.__new__(cls)._plugin_dir() is None + + def test_a_declared_directory_without_a_schema_is_not_trusted(self, tmp_path): + # A stale or wrong path must not shadow the fallback. + empty = tmp_path / "noschema" + empty.mkdir() + d = _write_plugin(tmp_path, "realone") + monkey = type("H", (SportsCoreSharedMixin,), {"_PLUGIN_DIR": str(empty)}) + assert monkey.__new__(monkey)._plugin_dir() != str(empty) + + def test_the_font_size_consequence(self, tmp_path): + """End to end: a declared directory restores the schema lookup.""" + d = _write_plugin(tmp_path, "sizeconseq") + host = type("H", (SportsCoreSharedMixin,), {"_PLUGIN_DIR": str(d)})() + assert host._schema_font_size("score") == 16, ( + "without the schema this is None, which is what made a configured " + "size look user-chosen and skipped the grid snap")