Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 42 additions & 7 deletions src/common/sports_shared.py
Original file line number Diff line number Diff line change
Expand Up @@ -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_<plugin_id>_<module>" 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)
Expand Down
55 changes: 55 additions & 0 deletions test/test_sports_shared.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Loading