refactor(processing): replace inline effect dispatch with @_effect_renderer registry
EffectColorStripStream._animate_loop used to rebuild a 12-entry dict
``renderers = {"fire": self._render_fire, ...}`` on every frame, then
look up ``renderers.get(self._effect_type, self._render_fire)``. Two
audit smells (H1) at once: per-frame dict-rebuild churn and a silent
fallback to fire whenever ``self._effect_type`` was a typo or any
``_render_*`` method got renamed without updating the dict.
Fix:
* ``@_effect_renderer("fire")`` stamps an attribute on the unbound
method.
* ``@_collect_effect_renderers`` (applied to the class) walks
members at class-creation, gathers the marked ones into
``cls._RENDERERS``, and raises ``RuntimeError`` on duplicate
registration.
The loop now reads ``type(self)._RENDERERS`` once and calls the
unbound method with explicit ``self``. An unknown ``_effect_type``
logs a warning and skips the frame (sleep one frame_time) instead of
silently rendering fire — louder failure mode without crashing the
animation thread.
Tests: 5 new tests cover the 12-effect coverage set, callable shape,
class-level (not per-instance) dict identity, duplicate-name
rejection, and the marker stamp contract.
343 existing processing / storage / API tests stay green; ruff clean.
This commit is contained in:
@@ -0,0 +1,82 @@
|
||||
"""Tests for the EffectColorStripStream renderer registry.
|
||||
|
||||
Lock in the per-class decorator-built _RENDERERS dict so:
|
||||
|
||||
* every render method is registered exactly once,
|
||||
* the registry is built at class-creation (not per-frame), and
|
||||
* duplicate registration is caught early.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from ledgrab.core.processing.effect_stream import (
|
||||
EffectColorStripStream,
|
||||
_collect_effect_renderers,
|
||||
_effect_renderer,
|
||||
)
|
||||
|
||||
|
||||
EXPECTED_EFFECTS = frozenset(
|
||||
{
|
||||
"fire",
|
||||
"meteor",
|
||||
"plasma",
|
||||
"noise",
|
||||
"aurora",
|
||||
"rain",
|
||||
"comet",
|
||||
"bouncing_ball",
|
||||
"fireworks",
|
||||
"sparkle_rain",
|
||||
"lava_lamp",
|
||||
"wave_interference",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def test_registry_contains_every_known_effect():
|
||||
assert set(EffectColorStripStream._RENDERERS.keys()) == EXPECTED_EFFECTS
|
||||
|
||||
|
||||
def test_each_registered_method_is_callable_and_unbound():
|
||||
for name, method in EffectColorStripStream._RENDERERS.items():
|
||||
assert callable(method), f"renderer for {name!r} is not callable"
|
||||
# Unbound: bind requires self argument, name should start with _render_
|
||||
assert method.__name__.startswith(
|
||||
"_render_"
|
||||
), f"renderer for {name!r} has unexpected name {method.__name__}"
|
||||
|
||||
|
||||
def test_registry_is_one_per_class_not_per_instance():
|
||||
"""The _RENDERERS dict lives on the CLASS, not on each instance."""
|
||||
assert isinstance(EffectColorStripStream._RENDERERS, dict)
|
||||
# Should be the exact same object across attribute access
|
||||
assert EffectColorStripStream._RENDERERS is EffectColorStripStream._RENDERERS
|
||||
|
||||
|
||||
def test_duplicate_registration_is_rejected():
|
||||
"""A class that registers two methods under the same name fails at decoration."""
|
||||
|
||||
with pytest.raises(RuntimeError, match="Duplicate"):
|
||||
|
||||
@_collect_effect_renderers
|
||||
class _Bad: # noqa: D401
|
||||
@_effect_renderer("fire")
|
||||
def a(self):
|
||||
pass
|
||||
|
||||
@_effect_renderer("fire")
|
||||
def b(self):
|
||||
pass
|
||||
|
||||
|
||||
def test_renderer_decorator_stamps_name_attribute():
|
||||
"""The marker attribute is what the class decorator reads."""
|
||||
|
||||
@_effect_renderer("my_effect")
|
||||
def fn(self):
|
||||
pass
|
||||
|
||||
assert fn._effect_renderer_name == "my_effect"
|
||||
Reference in New Issue
Block a user