Address review: prefs helper requires the redirect, visible stamp rejection, lock aware age re-probe, wider marker regexes

This commit is contained in:
J. Nick Koston
2026-09-03 21:01:49 +02:00
parent 19f5641706
commit 36468022ea
3 changed files with 25 additions and 11 deletions
+3 -3
View File
@@ -1104,8 +1104,8 @@ def get_components_per_integration_fixture() -> dict[str, set[str]]:
_TEST_FUNC_RE = re.compile(r"async def (test_\w+)")
_SHARED_YAML_RE = re.compile(r"@pytest\.mark\.shared_yaml\([\"'](\w+)[\"']\)")
_SHARED_YAML_DECORATOR_RE = re.compile(r"@pytest\.mark\.shared_yaml\(")
_SHARED_YAML_RE = re.compile(r"pytest\.mark\.shared_yaml\([\"'](\w+)[\"']\)")
_SHARED_YAML_USE_RE = re.compile(r"pytest\.mark\.shared_yaml\(")
@cache
@@ -1127,7 +1127,7 @@ def get_fixture_to_test_files() -> dict[str, frozenset[str]]:
result.setdefault(base_name, set()).add(rel_path)
# Shared fixtures are named by marker, not by a test function
names = _SHARED_YAML_RE.findall(content)
if len(names) != len(_SHARED_YAML_DECORATOR_RE.findall(content)):
if len(names) != len(_SHARED_YAML_USE_RE.findall(content)):
# A wrapped or non-literal marker would silently drop the mapping
# and CI would select no tests for that fixture
raise ValueError(
+15 -3
View File
@@ -342,7 +342,10 @@ def _read_stamp(stamp: Path, shared_dir: Path) -> Path | None:
return None
built = Path(text)
# Never trust a stamp pointing outside its own build dir as an unlink target
return built if shared_dir in built.parents else None
if shared_dir.resolve() in built.resolve().parents:
return built
warnings.warn(f"Ignoring stamp {stamp} pointing outside {shared_dir}", stacklevel=2)
return None
def _unused_since(stale: Path, cutoff: float) -> bool:
@@ -376,6 +379,9 @@ def _prune_stale_builds(name: str, keep: Path) -> None:
same_fixture = stale.name.startswith(prefix)
if not same_fixture and not _unused_since(stale, cutoff):
continue
# Creating .lock bumps the dir mtime, so remember whether the re-probe
# under the lock can trust it
lock_preexisting = (stale / ".lock").exists()
try:
lock_file = (stale / ".lock").open("w")
except FileNotFoundError:
@@ -392,8 +398,14 @@ def _prune_stale_builds(name: str, keep: Path) -> None:
except BlockingIOError:
continue # still in use by another run
# Re-probe under the lock: a worker freshens its dir before
# locking, so a just-claimed dir no longer looks unused
if not same_fixture and not _unused_since(stale, cutoff):
# locking, so a just-claimed dir no longer looks unused. A dir
# whose .lock we just created cannot be held by anyone, and our
# own open bumped its mtime, so its pre-open probe stands
if (
lock_preexisting
and not same_fixture
and not _unused_since(stale, cutoff)
):
continue
# rmtree tolerates races; a leftover partial tree only costs a
# rebuild, since the ELF is deleted before every compile
+7 -5
View File
@@ -1,7 +1,7 @@
"""Helpers for manipulating the host platform's preferences file.
ESPHome's host platform stores preferences in
``~/.esphome/prefs/<app_name>.prefs`` using a simple binary layout that
``$ESPHOME_PREFDIR/<app_name>.prefs`` using a simple binary layout that
mirrors ``HostPreferences::sync()``:
``[uint32_t key][uint8_t len][uint8_t data[len]]`` per entry.
@@ -19,11 +19,13 @@ import struct
def host_prefs_path(device_name: str) -> Path:
"""Return the on-disk prefs file path for a host-platform device.
Honors ESPHOME_PREFDIR, which the autouse isolated_preferences fixture
sets, so seeds land where the binary will look."""
Requires ESPHOME_PREFDIR, which the autouse isolated_preferences fixture
sets; refusing the ~/.esphome/prefs fallback keeps tests off real user
data if the fixture is ever bypassed."""
prefdir = os.environ.get("ESPHOME_PREFDIR")
base = Path(prefdir) if prefdir else Path.home() / ".esphome" / "prefs"
return base / f"{device_name}.prefs"
if not prefdir:
raise RuntimeError("ESPHOME_PREFDIR is not set; refusing the real prefs dir")
return Path(prefdir) / f"{device_name}.prefs"
def clear_host_prefs(device_name: str) -> None: