Compare commits

..
Author SHA1 Message Date
ChuckandClaude Opus 5.5 e4f5e49ff7 test(sports): treat an adopted sports_helpers copy as parity, not missing (#777)
* test(sports): treat an adopted sports_helpers copy as parity, not missing

The scoreboards deleted their copies of the sports_helpers bodies and
constants when they adopted SportsHelpersMixin (ledmatrix-plugins
#563/#564), so the 19 parity tests in test/test_sports_helpers.py failed
whenever LEDMATRIX_PLUGINS pointed at a plugins checkout. A copy that is
gone now counts as adopted when the plugin imports
src.common.sports_helpers, as the stage 3/4 and game-over parity tests
already do; a copy that remains must still match.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(sports): _adopted checks for a real import via the AST, not a text match

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 17:42:27 -04:00
4 changed files with 53 additions and 33 deletions
+8 -5
View File
@@ -19,12 +19,15 @@ accepts both, but the store flags the old spelling as deprecated
## Unreleased ## Unreleased
### Fixes ### Tooling
- A failed on-demand request no longer comes back after a restart as the - `test/test_sports_helpers.py`'s parity tests pass again with
session it ended. A failed request ends any running session, but the `LEDMATRIX_PLUGINS` set. The scoreboards deleted their copies of the
saved copy of that session (`display_on_demand_config`) was left behind, `sports_helpers` bodies and constants when they adopted `SportsHelpersMixin`
so the next restart of the display resumed it. (ledmatrix-plugins #563/#564), and the 19 tests still expected them. A copy
that is gone now counts as adopted when the plugin imports
`src.common.sports_helpers`, as the stage 3/4 and game-over parity tests
already do; a copy that remains must still match.
## 3.8.2 ## 3.8.2
+3 -7
View File
@@ -668,6 +668,7 @@ class DisplayController:
except Exception: # pylint: disable=broad-except except Exception: # pylint: disable=broad-except
cached_session = None cached_session = None
if self.on_demand_active or cached_session: if self.on_demand_active or cached_session:
self.cache_manager.clear_cache('display_on_demand_config')
self._set_on_demand_error('restore-failed') self._set_on_demand_error('restore-failed')
# Its state machine no longer describes what runs; let the last # Its state machine no longer describes what runs; let the last
# snapshot go stale (readers then say unknown) rather than keep # snapshot go stale (readers then say unknown) rather than keep
@@ -1816,14 +1817,8 @@ class DisplayController:
logger.error("Failed to publish on-demand state: %s", err, exc_info=True) logger.error("Failed to publish on-demand state: %s", err, exc_info=True)
def _set_on_demand_error(self, message: str) -> None: def _set_on_demand_error(self, message: str) -> None:
"""Set on-demand state to error and publish. """Set on-demand state to error and publish."""
Ends any running session, so its saved copy goes too: a failed
request that replaced a session left display_on_demand_config
behind, and the next restart resumed the session that had ended.
"""
self._reset_on_demand_fields() self._reset_on_demand_fields()
self.cache_manager.clear_cache('display_on_demand_config')
self.on_demand_status = 'error' self.on_demand_status = 'error'
self.on_demand_last_error = message self.on_demand_last_error = message
self.on_demand_last_event = None self.on_demand_last_event = None
@@ -2761,6 +2756,7 @@ class DisplayController:
logger.error("On-demand session for plugin '%s' cannot resume after the " logger.error("On-demand session for plugin '%s' cannot resume after the "
"restart: the plugin has no loaded display modes (did it " "restart: the plugin has no loaded display modes (did it "
"fail to load?); ending it", plugin_id) "fail to load?); ending it", plugin_id)
self.cache_manager.clear_cache('display_on_demand_config')
self._set_on_demand_error('restore-failed') self._set_on_demand_error('restore-failed')
return return
-9
View File
@@ -217,15 +217,6 @@ class TestReleasingThePlugin:
assert controller.current_display_mode == 'clock' assert controller.current_display_mode == 'clock'
assert controller.force_change is True assert controller.force_change is True
def test_a_failed_request_that_ends_the_session_drops_its_saved_copy(self, controller):
"""Otherwise the next restart resumes the session that just ended."""
_start(controller, plugin_id='clock')
controller.cache_manager.clear_cache.reset_mock()
_start(controller, plugin_id='uninstalled')
controller.cache_manager.clear_cache.assert_called_once_with('display_on_demand_config')
def test_a_plugin_enabled_during_the_session_stays_loaded(self, controller): def test_a_plugin_enabled_during_the_session_stays_loaded(self, controller):
_start(controller) _start(controller)
controller.test_config['preview-me'] = {'enabled': True} controller.test_config['preview-me'] = {'enabled': True}
+41 -11
View File
@@ -8,7 +8,9 @@ loses those tests with it.
The parity class is what keeps "byte-identical" true after this lands. Point The parity class is what keeps "byte-identical" true after this lands. Point
LEDMATRIX_PLUGINS at a ledmatrix-plugins checkout and every promoted body is LEDMATRIX_PLUGINS at a ledmatrix-plugins checkout and every promoted body is
compared, as a docstring-stripped AST, against every plugin copy that carries compared, as a docstring-stripped AST, against every plugin copy that carries
it. Without the variable it skips rather than fails, since core CI has no it. A copy that is gone counts as adopted when the plugin imports
src.common.sports_helpers (plugins#563/#564 did that for every scoreboard).
Without the variable it skips rather than fails, since core CI has no
plugins checkout; ledmatrix-plugins CI runs the same comparison against core plugins checkout; ledmatrix-plugins CI runs the same comparison against core
(scripts/check_sports_helpers_parity.py, ledmatrix-plugins#495). (scripts/check_sports_helpers_parity.py, ledmatrix-plugins#495).
""" """
@@ -572,6 +574,24 @@ def _core_definitions():
return out return out
def _sports_source(root, sport):
return (root / f"{sport}-scoreboard" / "sports.py").read_text(encoding="utf-8")
def _adopted(source):
"""Gone is fine once the plugin uses the module; otherwise the finder is
not seeing its copy."""
name = sports_helpers.__name__
for node in ast.walk(ast.parse(source)):
if isinstance(node, ast.ImportFrom):
if node.module == name or any(
f"{node.module}.{a.name}" == name for a in node.names):
return True
elif isinstance(node, ast.Import) and any(a.name == name for a in node.names):
return True
return False
class TestParityWithPlugins: class TestParityWithPlugins:
@pytest.mark.parametrize("name", sorted(PROMOTED)) @pytest.mark.parametrize("name", sorted(PROMOTED))
def test_body_matches_every_plugin_copy(self, name): def test_body_matches_every_plugin_copy(self, name):
@@ -580,10 +600,10 @@ class TestParityWithPlugins:
ours = _dump(_core_definitions()[name]) ours = _dump(_core_definitions()[name])
drifted, missing = [], [] drifted, missing = [], []
for sport in carriers: for sport in carriers:
defs = _definitions(ast.parse( source = _sports_source(root, sport)
(root / f"{sport}-scoreboard" / "sports.py").read_text(encoding="utf-8"))) theirs = _definitions(ast.parse(source))[where].get(plugin_name)
theirs = defs[where].get(plugin_name)
if theirs is None: if theirs is None:
if not _adopted(source):
missing.append(sport) missing.append(sport)
elif _dump(theirs) != ours: elif _dump(theirs) != ours:
drifted.append(sport) drifted.append(sport)
@@ -594,10 +614,20 @@ class TestParityWithPlugins:
@pytest.mark.parametrize("sport", SCOREBOARDS) @pytest.mark.parametrize("sport", SCOREBOARDS)
def test_constants_match(self, sport): def test_constants_match(self, sport):
root = _plugins_root() source = _sports_source(_plugins_root(), sport)
defs = _definitions(ast.parse( defs = _definitions(ast.parse(source))
(root / f"{sport}-scoreboard" / "sports.py").read_text(encoding="utf-8"))) expected = {
assert ast.literal_eval(defs["module"]["_MIN_WINDOW_DAYS"].value) == MIN_WINDOW_DAYS ("module", "_MIN_WINDOW_DAYS"): MIN_WINDOW_DAYS,
assert ast.literal_eval(defs["module"]["_MAX_WINDOW_DAYS"].value) == MAX_WINDOW_DAYS ("module", "_MAX_WINDOW_DAYS"): MAX_WINDOW_DAYS,
gap = defs["SportsCore"]["_DWELL_REENTRY_GAP_SECONDS"].value ("SportsCore", "_DWELL_REENTRY_GAP_SECONDS"):
assert math.isclose(ast.literal_eval(gap), SportsHelpersMixin._DWELL_REENTRY_GAP_SECONDS) SportsHelpersMixin._DWELL_REENTRY_GAP_SECONDS,
}
missing = []
for (where, name), value in expected.items():
node = defs[where].get(name)
if node is None:
if not _adopted(source):
missing.append(name)
else:
assert math.isclose(ast.literal_eval(node.value), value), name
assert missing == [], f"not found in {sport}: {missing}"