diff --git a/CHANGELOG.md b/CHANGELOG.md index 1dc13afd..1b10be37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,7 +29,8 @@ this): `_setting_int`, `_reset_dwell_on_reentry`, `_next_switch_index`, `_spread_weighted_order`, `_odds_color`, `_upcoming_date_and_time_text` under the plugins' names and signatures, plus the `_favorite_key` override point. - No `__init__` and no state; the host contract is in the module docstring. + Constructor-free; keeps lazy state on its host (see the module docstring, + which also gives the host contract). A new module rather than more methods on `sports_shared`: a plugin that deletes a copy and leans on an older module having grown the method fails at runtime with `AttributeError`, which no load-time check sees, while a missing diff --git a/src/common/sports_helpers.py b/src/common/sports_helpers.py index 53b11482..6ae1c517 100644 --- a/src/common/sports_helpers.py +++ b/src/common/sports_helpers.py @@ -187,8 +187,8 @@ def spread_weighted_order(weights: List[int]) -> List[int]: class SportsHelpersMixin: """The ``SportsCore`` methods identical across the scoreboards' sports.py. - Stateless and constructor-free; see the module docstring for the host - contract and base-order guidance. + Constructor-free; keeps lazy state on its host (see the module docstring + for that state, the host contract and base-order guidance). """ #: Longest gap between two display() calls that still counts as one @@ -386,7 +386,7 @@ class SportsHelpersMixin: if getter is None: return (0, 255, 0) try: - return getter("odds_text", (0, 255, 0)) + return getter("odds_text", (0, 255, 0)) # pylint: disable=not-callable except Exception: return (0, 255, 0) diff --git a/test/test_common_is_hardware_free.py b/test/test_common_is_hardware_free.py index 0b73e939..72cdf512 100644 --- a/test/test_common_is_hardware_free.py +++ b/test/test_common_is_hardware_free.py @@ -45,7 +45,7 @@ def test_sports_modules_import_without_hardware(): for name in {modules!r}: importlib.import_module("src.common." + name) loaded = sorted(m for m in sys.modules - if m.startswith(("src.display_manager", "src.base_classes"))) + if m.startswith({FORBIDDEN!r})) print(json.dumps(loaded)) """) result = subprocess.run( @@ -73,14 +73,23 @@ def _module_level_imports(tree): stack.extend(handler.body) -def _targets(node): +def _targets(node, package="src.common"): + """Absolute dotted names an import can load. Relative imports resolve + against ``package``, the scanned module's package.""" if isinstance(node, ast.Import): return [alias.name for alias in node.names] - if node.module is None: + base = node.module + if node.level: + parts = package.split(".") + if node.level > len(parts): + return [] # beyond the top-level package; Python rejects it too + anchor = ".".join(parts[:len(parts) - node.level + 1]) + base = f"{anchor}.{node.module}" if node.module else anchor + if base is None: return [] - if node.module == "src": - return [f"src.{alias.name}" for alias in node.names] - return [node.module] + # `from pkg import name` may load the submodule pkg.name. + return [base] + [f"{base}.{alias.name}" for alias in node.names + if alias.name != "*"] def _violations(): @@ -88,7 +97,7 @@ def _violations(): for path in sorted(COMMON.glob("*.py")): tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) for node in _module_level_imports(tree): - for target in _targets(node): + for target in _targets(node, "src.common"): if target == FORBIDDEN or any( target == f or target.startswith(f + ".") for f in FORBIDDEN): found.setdefault(path.name, []).append(f"line {node.lineno}: {target}") @@ -120,5 +129,35 @@ def test_the_scan_sees_a_direct_import(tmp_path): def later(): from src.display_manager import DisplayManager """)) - targets = sorted(t for n in _module_level_imports(tree) for t in _targets(n)) - assert targets == ["src.base_classes.sports", "src.display_manager", "src.plugin_system"] + targets = {t for n in _module_level_imports(tree) for t in _targets(n)} + assert {"src.base_classes.sports", "src.display_manager", + "src.plugin_system"} <= targets + + +def test_the_scan_resolves_relative_imports(): + # A module in src/common reaches src.plugin_system through `..`. + tree = ast.parse(textwrap.dedent(""" + from .. import plugin_system + from ..plugin_system import plugin_manager + from . import sports_shared + """)) + targets = {t for n in _module_level_imports(tree) + for t in _targets(n, "src.common")} + assert {"src.plugin_system", "src.plugin_system.plugin_manager", + "src.common.sports_shared"} <= targets + # The package decides where `..` points. + node = ast.parse("from .. import plugin_system").body[0] + assert _targets(node, "src.common") == ["src", "src.plugin_system"] + assert _targets(node, "a.b") == ["a", "a.plugin_system"] + + +def test_the_scan_flags_relative_imports_in_src_common(monkeypatch, tmp_path): + # End to end through _violations(): both relative forms are reported. + (tmp_path / "bad.py").write_text( + "from .. import plugin_system\nfrom ..plugin_system import x\n", + encoding="utf-8") + monkeypatch.setitem(globals(), "COMMON", tmp_path) + found = {name: sorted(hits) for name, hits in _violations().items()} + assert found == {"bad.py": ["line 1: src.plugin_system", + "line 2: src.plugin_system", + "line 2: src.plugin_system.x"]} diff --git a/test/test_sports_helpers.py b/test/test_sports_helpers.py index 3bfe2aab..f2ef99a6 100644 --- a/test/test_sports_helpers.py +++ b/test/test_sports_helpers.py @@ -8,8 +8,9 @@ loses those tests with it. 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 compared, as a docstring-stripped AST, against every plugin copy that carries -it. Without the variable it skips rather than fails, since CI has no plugins -checkout. +it. Without the variable it skips rather than fails, since core CI has no +plugins checkout; ledmatrix-plugins CI runs the same comparison against core +(scripts/check_sports_helpers_parity.py, ledmatrix-plugins#495). """ import ast @@ -505,7 +506,9 @@ def _plugins_root(): raw = os.environ.get("LEDMATRIX_PLUGINS") if not raw: pytest.skip("set LEDMATRIX_PLUGINS to a ledmatrix-plugins checkout to " - "compare sports_helpers against the plugin copies") + "compare sports_helpers against the plugin copies; in CI " + "this parity check runs in ledmatrix-plugins " + "scripts/check_sports_helpers_parity.py (#495)") root = Path(raw) if (root / "plugins").is_dir(): root = root / "plugins"