test(harness): make the stub scoping independent of file order (t44)
A subset of test files in an order the suite was not written for failed 3 test_init.py::TestImportStep tests with ModuleNotFoundError: voluptuous, while the full suite stayed green. Two order-only mechanisms, both now fixed at the source: 1. entity_stublib.install() guarded on ONE sentinel (`homeassistant.components.media_player`). A file whose teardown popped `voluptuous` but left the sentinel turned the next file's install() into a no-op, so importing config_flow failed. The guard now checks every face a file scopes on (`_STUB_FACES`, derived from ENTITIES_STUB_NAMES so the two cannot drift) and rebuilds the set when anything is missing. install() also registers the `homeassistant.helpers.selector` face itself, which the per-file installers had been adding separately — without it the derived list would never be complete. 2. Teardowns restored only the names their file had listed, so a module the file loaded (`nsh.diagnostics`, `nsh.media_player`) survived into the next file still holding stub objects — a stub exception class, the redact helper — that had since been replaced; comparisons against the new objects then failed (5 tests in test_command_routing on an unlucky order). Every file now ends through entity_stublib.snapshot_modules()/restore_modules(), which drops EVERYTHING the file created under the harness namespaces (homeassistant/voluptuous/nsh) and puts back everything it replaced. Hand-written lists were the bug, so there is none to keep in sync. The t7 no-leak discipline is unchanged: a file's stubs still never reach another file, and a file's loaded modules still do not survive it. A session-scoped baseline install was tried first and rejected: files whose own installer guards on "the stubs are already there" skipped installing their richer faces, which produced exactly the inconsistent state a baseline is meant to prevent (10 test_standalone_setup failures). tests/test_harness_scoping.py: the sentinel poisoning, the wholesale-rebuild invariant, a teardown that removes what the file loaded, the stale-object case (diagnostics vs the redact helper), and — because order is the thing under test — two subprocess runs of the reported orderings. Verified: the reported order (test_command_routing, test_protocol_transport, test_standalone_setup, test_init) 152 passed / 1 skipped; the minimal pair both ways; the previously failing 6-test order now 167 passed; every test file in REVERSE alphabetical order 369 passed / 2 skipped; two scrambled thirds green; full suite 369 / 2 and 370 / 1 with androidtvremote2; ruff clean.
This commit is contained in:
1 parent
fbc72dfd9e
commit
2e1d162ef4
9 files changed
+233
-63
No files matched your search
@@ -50,3 +50,4 @@ _load_module(
|
||||
# Stdlib-only integration modules.
|
||||
for _mod in ("const", "adb_session", "connection"):
|
||||
_load_module(f"{_PKG}.{_mod}", _SHIELD_DIR / f"{_mod}.py")
|
||||
|
||||
+34
-20
@@ -178,26 +178,11 @@ class RemoteEntity:
|
||||
#: behind while another name it popped — e.g. `voluptuous` — is gone. A sentinel
|
||||
#: guard then made install() a no-op in the next file, and whichever module needed
|
||||
#: the missing face failed with ModuleNotFoundError depending on file order (t44).
|
||||
_STUB_FACES = (
|
||||
"voluptuous",
|
||||
"homeassistant",
|
||||
"homeassistant.config_entries",
|
||||
"homeassistant.core",
|
||||
"homeassistant.const",
|
||||
"homeassistant.exceptions",
|
||||
"homeassistant.components",
|
||||
"homeassistant.components.media_player",
|
||||
"homeassistant.components.remote",
|
||||
"homeassistant.components.button",
|
||||
"homeassistant.components.sensor",
|
||||
"homeassistant.helpers",
|
||||
"homeassistant.helpers.event",
|
||||
"homeassistant.helpers.entity",
|
||||
"homeassistant.helpers.entity_platform",
|
||||
"homeassistant.helpers.device_registry",
|
||||
"homeassistant.helpers.entity_registry",
|
||||
"homeassistant.helpers.redact",
|
||||
"homeassistant.helpers.selector",
|
||||
#: The faces install() provides, derived from the list every test file scopes on.
|
||||
#: The ``nsh.*`` entries there are the integration modules the harness loads, not
|
||||
#: stub faces, so they are filtered out here.
|
||||
_STUB_FACES = tuple(
|
||||
name for name in ENTITIES_STUB_NAMES if not name.startswith("nsh.")
|
||||
)
|
||||
|
||||
|
||||
@@ -508,6 +493,35 @@ def install() -> None:
|
||||
sys.modules["homeassistant.helpers"] = helpers
|
||||
|
||||
|
||||
#: Namespaces the stub harness owns. Anything under these prefixes that a test
|
||||
#: file creates is harness state, so a file's teardown must remove it: a module
|
||||
#: left behind keeps the stub objects it captured (an exception class, the redact
|
||||
#: helper the diagnostics module holds) after the next file rebuilt them, which is
|
||||
#: how a file-order-dependent failure looked (t44).
|
||||
_TRACKED_PREFIXES = ("homeassistant", "voluptuous", "nsh")
|
||||
|
||||
|
||||
def _tracked(name: str) -> bool:
|
||||
return name.split(".", 1)[0] in _TRACKED_PREFIXES
|
||||
|
||||
|
||||
def snapshot_modules() -> dict:
|
||||
"""Every stub/module name live right now — for a leak-free teardown."""
|
||||
return {name: module for name, module in sys.modules.items() if _tracked(name)}
|
||||
|
||||
|
||||
def restore_modules(snapshot: dict) -> None:
|
||||
"""Undo everything the current test file did to those namespaces.
|
||||
|
||||
Two halves, and the first is what a hand-written list of names kept getting
|
||||
wrong: drop every tracked name the file created (so the next file loads it
|
||||
fresh, against ITS stubs), then put back every object it replaced.
|
||||
"""
|
||||
for name in [name for name in sys.modules if _tracked(name) and name not in snapshot]:
|
||||
del sys.modules[name]
|
||||
sys.modules.update(snapshot)
|
||||
|
||||
|
||||
def load_entity_modules() -> dict:
|
||||
"""Load the integration's HA-free-ish entity modules under nsh."""
|
||||
install()
|
||||
|
||||
@@ -13,7 +13,7 @@ import sys
|
||||
import entity_stublib
|
||||
import pytest
|
||||
from conftest import _SHIELD_DIR
|
||||
from entity_stublib import ENTITIES_STUB_NAMES, install, load_entity_modules
|
||||
from entity_stublib import install, load_entity_modules
|
||||
from nsh.connection import ShieldConnectionManager
|
||||
from nsh.const import TIVIMATE_PACKAGE
|
||||
|
||||
@@ -21,15 +21,10 @@ from nsh.const import TIVIMATE_PACKAGE
|
||||
@pytest.fixture(autouse=True)
|
||||
def scoped_entity_stubs():
|
||||
"""Container for the HA stubs + loaded entity modules (t7 discipline)."""
|
||||
snapshot = {name: sys.modules.get(name) for name in ENTITIES_STUB_NAMES}
|
||||
snapshot = entity_stublib.snapshot_modules()
|
||||
install()
|
||||
yield load_entity_modules()
|
||||
for name in ENTITIES_STUB_NAMES:
|
||||
original = snapshot[name]
|
||||
if original is None:
|
||||
sys.modules.pop(name, None)
|
||||
else:
|
||||
sys.modules[name] = original
|
||||
entity_stublib.restore_modules(snapshot)
|
||||
|
||||
|
||||
class FakeProtocolClient:
|
||||
|
||||
@@ -11,6 +11,7 @@ import sys
|
||||
import types
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import entity_stublib
|
||||
import pytest
|
||||
from conftest import _SHIELD_DIR, _load_module
|
||||
from nsh.const import (
|
||||
@@ -187,15 +188,10 @@ def scoped_stubs():
|
||||
teardown, so later tests never observe the fake homeassistant/voluptuous
|
||||
(nor the stub-loaded nsh.helpers/nsh.config_flow) modules.
|
||||
"""
|
||||
snapshot = {name: sys.modules.get(name) for name in _STUB_NAMES}
|
||||
snapshot = entity_stublib.snapshot_modules()
|
||||
_install_import_stubs()
|
||||
yield
|
||||
for name in _STUB_NAMES:
|
||||
original = snapshot[name]
|
||||
if original is None:
|
||||
sys.modules.pop(name, None)
|
||||
else:
|
||||
sys.modules[name] = original
|
||||
entity_stublib.restore_modules(snapshot)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
|
||||
@@ -11,6 +11,7 @@ import sys
|
||||
import types
|
||||
from types import SimpleNamespace
|
||||
|
||||
import entity_stublib
|
||||
import pytest
|
||||
from conftest import _SHIELD_DIR, _load_module
|
||||
from nsh.const import (
|
||||
@@ -278,15 +279,10 @@ def _entry_module():
|
||||
@pytest.fixture(autouse=True)
|
||||
def scoped_stubs():
|
||||
"""Keep the import stubs inside this module's tests (t7 discipline)."""
|
||||
snapshot = {name: sys.modules.get(name) for name in _STUB_NAMES}
|
||||
snapshot = entity_stublib.snapshot_modules()
|
||||
_install_stubs()
|
||||
yield
|
||||
for name in _STUB_NAMES:
|
||||
original = snapshot[name]
|
||||
if original is None:
|
||||
sys.modules.pop(name, None)
|
||||
else:
|
||||
sys.modules[name] = original
|
||||
entity_stublib.restore_modules(snapshot)
|
||||
|
||||
|
||||
def make_flow(tmp_dir: str, monkeypatch: pytest.MonkeyPatch, client: FakeProtocolClient):
|
||||
|
||||
@@ -26,6 +26,7 @@ from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import entity_stublib
|
||||
import pytest
|
||||
import voluptuous_mirror as vol_mirror
|
||||
from conftest import _SHIELD_DIR, _load_module
|
||||
@@ -331,17 +332,12 @@ def scoped_stubs():
|
||||
another file left behind from shadowing the faithful voluptuous mirror
|
||||
this file's schema assertions depend on.
|
||||
"""
|
||||
snapshot = {name: sys.modules.get(name) for name in _STUB_NAMES}
|
||||
snapshot = entity_stublib.snapshot_modules()
|
||||
for name in _STUB_NAMES:
|
||||
sys.modules.pop(name, None)
|
||||
_install_stubs()
|
||||
yield
|
||||
for name in _STUB_NAMES:
|
||||
original = snapshot[name]
|
||||
if original is None:
|
||||
sys.modules.pop(name, None)
|
||||
else:
|
||||
sys.modules[name] = original
|
||||
entity_stublib.restore_modules(snapshot)
|
||||
|
||||
|
||||
def _form_module():
|
||||
|
||||
@@ -0,0 +1,181 @@
|
||||
"""tests/test_harness_scoping.py — the stub harness must not depend on file order (t44).
|
||||
|
||||
The failure this pins, as reported: a subset of test files in an order the suite was
|
||||
not written for left three ``test_init.py::TestImportStep`` tests failing with
|
||||
``ModuleNotFoundError: No module named 'voluptuous'``, while the full suite stayed
|
||||
green. Two mechanisms produced it, and both are order-only:
|
||||
|
||||
1. ``entity_stublib.install()`` trusted a single sentinel
|
||||
(``homeassistant.components.media_player``). A file whose teardown popped
|
||||
``voluptuous`` while leaving the sentinel behind turned the next file's
|
||||
``install()`` into a no-op, so importing ``config_flow`` failed.
|
||||
2. A file's teardown restored only the names it had listed. A module it loaded
|
||||
(``nsh.diagnostics``, ``nsh.media_player``) therefore survived into the next
|
||||
file still holding stub objects — an exception class, the redact helper — that
|
||||
had since been replaced, and comparisons against the new objects failed.
|
||||
|
||||
The fix keeps the t7 discipline (no stub leaks across files): ``install()`` checks
|
||||
the whole face set and rebuilds when anything is missing, and every file's teardown
|
||||
uses ``snapshot_modules()``/``restore_modules()``, which drops *everything* the file
|
||||
created under the harness namespaces and puts back everything it replaced.
|
||||
|
||||
The in-process tests cover both mechanisms directly; the subprocess cases re-run the
|
||||
reported orderings end to end, because that is where the failure was visible.
|
||||
"""
|
||||
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import entity_stublib
|
||||
import pytest
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
TRACKED = ("homeassistant", "voluptuous", "nsh")
|
||||
|
||||
|
||||
def _scoped_faces() -> list[str]:
|
||||
"""The HA/voluptuous faces every file scopes on (the nsh.* names are modules)."""
|
||||
return [name for name in entity_stublib.ENTITIES_STUB_NAMES if not name.startswith("nsh.")]
|
||||
|
||||
|
||||
def _run(*paths: str) -> subprocess.CompletedProcess:
|
||||
return subprocess.run(
|
||||
[
|
||||
sys.executable,
|
||||
"-m",
|
||||
"pytest",
|
||||
*paths,
|
||||
"-q",
|
||||
"--no-header",
|
||||
"-p",
|
||||
"no:cacheprovider",
|
||||
],
|
||||
cwd=REPO_ROOT,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=300,
|
||||
)
|
||||
|
||||
|
||||
class TestStubFaceGuard:
|
||||
"""Mechanism 1: a partially popped face set must be repaired, not trusted."""
|
||||
|
||||
def test_install_provides_every_face_a_file_scopes_on(self):
|
||||
"""A face listed for scoping but never installed is a file-order trap."""
|
||||
snapshot = entity_stublib.snapshot_modules()
|
||||
try:
|
||||
entity_stublib.install()
|
||||
|
||||
missing = [name for name in _scoped_faces() if name not in sys.modules]
|
||||
|
||||
assert missing == []
|
||||
finally:
|
||||
entity_stublib.restore_modules(snapshot)
|
||||
|
||||
def test_a_complete_face_set_is_left_alone(self):
|
||||
"""The guard must still be an idempotence guard, not a rebuild."""
|
||||
snapshot = entity_stublib.snapshot_modules()
|
||||
try:
|
||||
entity_stublib.install()
|
||||
before = {name: sys.modules[name] for name in _scoped_faces()}
|
||||
|
||||
entity_stublib.install()
|
||||
|
||||
assert {name: sys.modules[name] for name in _scoped_faces()} == before
|
||||
finally:
|
||||
entity_stublib.restore_modules(snapshot)
|
||||
|
||||
def test_a_sentinel_left_behind_does_not_hide_a_missing_face(self):
|
||||
"""The reported poisoning: media_player present, voluptuous gone."""
|
||||
snapshot = entity_stublib.snapshot_modules()
|
||||
try:
|
||||
entity_stublib.install()
|
||||
kept = sys.modules["homeassistant.components.media_player"]
|
||||
del sys.modules["voluptuous"]
|
||||
|
||||
entity_stublib.install()
|
||||
|
||||
assert "voluptuous" in sys.modules
|
||||
# the rebuild is wholesale, so the sentinel is a real face again
|
||||
assert sys.modules["homeassistant.components.media_player"] is not None
|
||||
assert kept is not None
|
||||
finally:
|
||||
entity_stublib.restore_modules(snapshot)
|
||||
|
||||
|
||||
class TestTeardownLeaks:
|
||||
"""Mechanism 2: nothing a file created or replaced may survive its teardown."""
|
||||
|
||||
def test_teardown_removes_the_modules_a_file_loaded(self):
|
||||
snapshot = entity_stublib.snapshot_modules()
|
||||
try:
|
||||
entity_stublib.install()
|
||||
entity_stublib.load_entity_modules()
|
||||
created = [
|
||||
name
|
||||
for name in sys.modules
|
||||
if name.startswith("nsh.") and name not in snapshot
|
||||
]
|
||||
assert created, "premise: loading the entity modules creates nsh.* modules"
|
||||
|
||||
entity_stublib.restore_modules(snapshot)
|
||||
|
||||
assert [name for name in sys.modules if name.startswith("nsh.")] == [
|
||||
name for name in snapshot if name.startswith("nsh.")
|
||||
]
|
||||
assert {
|
||||
name: sys.modules.get(name)
|
||||
for name in snapshot
|
||||
if name.split(".", 1)[0] in TRACKED
|
||||
} == snapshot
|
||||
finally:
|
||||
entity_stublib.restore_modules(snapshot)
|
||||
|
||||
def test_a_reloaded_module_holds_the_current_stub_objects(self):
|
||||
"""The stale-object failure, exactly: diagnostics vs the redact helper."""
|
||||
snapshot = entity_stublib.snapshot_modules()
|
||||
try:
|
||||
entity_stublib.install()
|
||||
stale_module = entity_stublib.load_entity_modules()["diagnostics"]
|
||||
stale = stale_module.async_redact_data
|
||||
entity_stublib.restore_modules(snapshot) # ...the file ends
|
||||
|
||||
# ...the next file installs its own faces: a partial pop is how that
|
||||
# actually looked, and it is what forces the rebuild
|
||||
entity_stublib.install()
|
||||
del sys.modules["homeassistant.helpers.redact"]
|
||||
entity_stublib.install()
|
||||
fresh_modules = entity_stublib.load_entity_modules()
|
||||
|
||||
current = sys.modules["homeassistant.helpers.redact"].async_redact_data
|
||||
assert stale is not current, "premise: the rebuild created a new helper"
|
||||
assert fresh_modules["diagnostics"].async_redact_data is current
|
||||
# ...and the stale module is not the one sitting in the import cache
|
||||
assert sys.modules["nsh.diagnostics"] is fresh_modules["diagnostics"]
|
||||
finally:
|
||||
entity_stublib.restore_modules(snapshot)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"paths",
|
||||
[
|
||||
# the reported reproduction, cut to the two files that trigger it
|
||||
("tests/test_standalone_setup.py", "tests/test_init.py::TestImportStep"),
|
||||
# the other failure mode: a file that loads the entity modules before a
|
||||
# file that compares them against freshly installed faces
|
||||
(
|
||||
"tests/test_standalone_setup.py",
|
||||
"tests/test_config_flow_protocol.py",
|
||||
"tests/test_init.py",
|
||||
"tests/test_command_routing.py::TestAdbCommandService",
|
||||
"tests/test_command_routing.py::TestFaithfulHaStubs",
|
||||
"tests/test_command_routing.py::TestServiceRegistryKey",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_arbitrary_file_orders_pass(paths):
|
||||
"""Order is the thing under test, so it is executed, not reasoned about."""
|
||||
result = _run(*paths)
|
||||
|
||||
assert result.returncode == 0, result.stdout[-3000:]
|
||||
+2
-7
@@ -39,19 +39,14 @@ _INIT_STUB_NAMES = (
|
||||
@pytest.fixture(autouse=True)
|
||||
def _integration_module():
|
||||
"""HA stubs + the integration __init__ module under nsh.init."""
|
||||
snapshot = {name: sys.modules.get(name) for name in _INIT_STUB_NAMES}
|
||||
snapshot = entity_stublib.snapshot_modules()
|
||||
entity_stublib.install()
|
||||
if "nsh.init" not in sys.modules:
|
||||
import conftest
|
||||
|
||||
conftest._load_module("nsh.init", conftest._SHIELD_DIR / "__init__.py")
|
||||
yield sys.modules["nsh.init"]
|
||||
for name in _INIT_STUB_NAMES:
|
||||
original = snapshot[name]
|
||||
if original is None:
|
||||
sys.modules.pop(name, None)
|
||||
else:
|
||||
sys.modules[name] = original
|
||||
entity_stublib.restore_modules(snapshot)
|
||||
|
||||
|
||||
class _SeedEntriesStub:
|
||||
|
||||
@@ -17,6 +17,7 @@ from pathlib import Path # noqa: E402 (F1 test key staging)
|
||||
from types import SimpleNamespace
|
||||
from unittest import mock
|
||||
|
||||
import entity_stublib
|
||||
import pytest
|
||||
import voluptuous_mirror as vol_mirror
|
||||
from conftest import _SHIELD_DIR, _load_module
|
||||
@@ -262,15 +263,10 @@ class FakeHass:
|
||||
@pytest.fixture(autouse=True)
|
||||
def scoped_stubs():
|
||||
"""Keep the import stubs inside this module's tests (t7 discipline)."""
|
||||
snapshot = {name: sys.modules.get(name) for name in _STUB_NAMES}
|
||||
snapshot = entity_stublib.snapshot_modules()
|
||||
_install_stubs()
|
||||
yield
|
||||
for name in _STUB_NAMES:
|
||||
original = snapshot[name]
|
||||
if original is None:
|
||||
sys.modules.pop(name, None)
|
||||
else:
|
||||
sys.modules[name] = original
|
||||
entity_stublib.restore_modules(snapshot)
|
||||
|
||||
|
||||
def _form_module():
|
||||
|
||||
Reference in new issue
Block a user