fix(tests): relocated pytest basetemps stop piling up in $HOME; runner sweeps roots killed runs left
Two leaks from the test temp plumbing: tests/conftest.py relocates pytest's basetemp out of the native Hermes home (#111101) with mkdtemp(dir=native.parent), which is the operator's $HOME, and nothing removed it: 123 hermes-pytest-basetemp-* dirs (552 MB) appeared there in a day, one per bare pytest process. The relocated basetemp now goes into one prunable root (/var/tmp/hermes-pytest on POSIX, a non-dotted sibling of the native home elsewhere), is removed at pytest_unconfigure, and idle siblings from killed runs are swept on entry. scripts/run_tests_parallel.py deletes each per-file temp root in finally, but a SIGKILLed runner (tool timeout, stray pkill) never gets there and leaks one root per in-flight worker: 983 r-* roots (3.4 GB) in three days. The runner now sweeps 24h-idle roots at start and forces read-only permission fixtures writable before rmtree instead of skipping them.
This commit is contained in:
@@ -58,6 +58,29 @@ from concurrent.futures import ThreadPoolExecutor, Future
|
||||
from pathlib import Path
|
||||
from typing import Dict, List, Optional, Tuple
|
||||
|
||||
def _sweep_killed_run_roots(root: str) -> None:
|
||||
"""Remove per-file temp roots older runs left behind. Each attempt deletes its own root
|
||||
in ``finally``, but a runner that is SIGKILLed (a tool timeout, a stray pkill) never gets
|
||||
there and leaks one root per in-flight worker; nothing else looks at this directory, so
|
||||
983 of them (3.4 GB) accumulated on one host in three days. Idle for a day = dead."""
|
||||
try:
|
||||
from hermes_constants_scratch import prune_idle_entries
|
||||
except ImportError: # runner invoked from outside the repo root
|
||||
return
|
||||
prune_idle_entries(Path(root), 24, frozenset())
|
||||
|
||||
|
||||
def _rmtree_force(path: str) -> None:
|
||||
def _chmod_retry(fn, p, _exc):
|
||||
try:
|
||||
os.chmod(os.path.dirname(p) if fn is os.rmdir or fn is os.listdir else p, 0o700)
|
||||
os.chmod(p, 0o700)
|
||||
fn(p)
|
||||
except OSError:
|
||||
pass
|
||||
shutil.rmtree(path, onerror=_chmod_retry)
|
||||
|
||||
|
||||
def _runner_scratch_root() -> str:
|
||||
"""Per-run temp roots live on DISK, never the system temp dir: a full-suite run writes
|
||||
gigabytes of tmp_path fixtures and /tmp is RAM-backed tmpfs on many Linux hosts. /var/tmp is
|
||||
@@ -555,8 +578,9 @@ def _run_one_file_once(
|
||||
finally:
|
||||
# Delete the temp root for this attempt. Nothing reads it after the
|
||||
# subprocess exits. More than 3000 of them fill the disk of the
|
||||
# runner over one suite.
|
||||
shutil.rmtree(temproot, ignore_errors=True)
|
||||
# runner over one suite. Permission fixtures leave read-only dirs
|
||||
# behind; make them writable and retry instead of skipping them.
|
||||
_rmtree_force(temproot)
|
||||
|
||||
if rc == 5:
|
||||
# No tests collected in THIS file — legitimate per-file: a
|
||||
@@ -1329,6 +1353,10 @@ def main() -> int:
|
||||
if rc != 0:
|
||||
_print_inline_failure(fpath, output, repo_root, pytest_passthrough)
|
||||
|
||||
if str(repo_root) not in sys.path:
|
||||
sys.path.insert(0, str(repo_root))
|
||||
_sweep_killed_run_roots(_runner_scratch_root())
|
||||
|
||||
with ThreadPoolExecutor(max_workers=args.jobs) as pool:
|
||||
# Duration cache for the timeout scaler: known-slow files get
|
||||
# proportional headroom instead of a false timeout-kill under
|
||||
|
||||
@@ -1317,16 +1317,41 @@ def _relocate_basetemp_outside_operator_home(config) -> None:
|
||||
return
|
||||
# The system temp dir may itself be inside the home (Windows TEMP under the
|
||||
# Hermes home). The repo is no escape either: the default install checks it
|
||||
# out *inside* the home (~/.hermes/hermes-agent). A sibling of the native
|
||||
# home is outside it by construction.
|
||||
safe_root = None if not Path(tempfile.gettempdir()).resolve().is_relative_to(native) else native.parent
|
||||
safe = Path(tempfile.mkdtemp(prefix="hermes-pytest-basetemp-", dir=safe_root))
|
||||
# out *inside* the home (~/.hermes/hermes-agent). The relocated basetemp goes
|
||||
# into ONE prunable root outside the home, never loose into the operator's
|
||||
# $HOME (123 ``hermes-pytest-basetemp-*`` dirs piled up there in a day, one per
|
||||
# test file the per-file runner spawned). It is removed when this pytest exits
|
||||
# and, for runs that were killed before that, swept once it is 24h idle.
|
||||
safe = Path(tempfile.mkdtemp(prefix="b-", dir=_pytest_disk_temp_root(native)))
|
||||
assert not safe.resolve().is_relative_to(native), (
|
||||
f"pytest basetemp {safe} still resolves inside the operator's Hermes home {native}; "
|
||||
"refusing to run the suite against the live install (pass --basetemp outside it)"
|
||||
)
|
||||
factory._given_basetemp = safe
|
||||
config.option.basetemp = str(safe)
|
||||
config._hermes_relocated_basetemp = safe
|
||||
|
||||
|
||||
def _pytest_disk_temp_root(native: Path) -> Path:
|
||||
"""The root for relocated basetemps: the disk-backed runner root when the host has
|
||||
one (``scripts/run_tests_parallel.py::_runner_scratch_root``), else a plain (not
|
||||
dot-prefixed — hidden-dir search tests would see every fixture as hidden) sibling of
|
||||
the native home. Entries idle for a day are swept on the way in."""
|
||||
from hermes_constants_scratch import prune_idle_entries
|
||||
|
||||
if os.name != "nt" and os.path.isdir("/var/tmp"): # no-tmp: ok — disk-backed FHS root
|
||||
root = Path("/var/tmp/hermes-pytest") # no-tmp: ok — /var/tmp is disk-backed by FHS, never tmpfs
|
||||
else:
|
||||
root = native.parent / "hermes-pytest"
|
||||
root.mkdir(parents=True, exist_ok=True)
|
||||
prune_idle_entries(root, 24, frozenset())
|
||||
return root
|
||||
|
||||
|
||||
def _remove_relocated_basetemp(config) -> None:
|
||||
safe = getattr(config, "_hermes_relocated_basetemp", None)
|
||||
if safe is not None:
|
||||
shutil.rmtree(safe, ignore_errors=True)
|
||||
|
||||
|
||||
def _pinned_mcp_sdk_version() -> str:
|
||||
@@ -1363,6 +1388,10 @@ def require_mcp_2_sdk():
|
||||
pytest.skip(f"requires mcp=={pinned} (found {found}); install the [mcp] extra")
|
||||
|
||||
|
||||
def pytest_unconfigure(config): # noqa: D401 — pytest hook
|
||||
_remove_relocated_basetemp(config)
|
||||
|
||||
|
||||
@pytest.hookimpl(trylast=True) # after _pytest.tmpdir has built config._tmp_path_factory
|
||||
def pytest_configure(config): # noqa: D401 — pytest hook
|
||||
"""Register markers used by hermetic conftest."""
|
||||
|
||||
@@ -15,6 +15,16 @@ import hermes_constants
|
||||
from tests import conftest as suite_conftest
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _sibling_root_only(monkeypatch):
|
||||
"""Relocate into the sibling-of-native root, never the host's shared disk root: these
|
||||
tests use a fake native home under tmp_path and must not create or prune anything real."""
|
||||
import os
|
||||
|
||||
real_isdir = os.path.isdir
|
||||
monkeypatch.setattr(suite_conftest.os.path, "isdir", lambda p: False if p == "/var/tmp" else real_isdir(p)) # no-tmp: ok — disables the disk-root branch
|
||||
|
||||
|
||||
def _config_with_basetemp(given: Path | None) -> SimpleNamespace:
|
||||
return SimpleNamespace(
|
||||
_tmp_path_factory=SimpleNamespace(_given_basetemp=given),
|
||||
@@ -33,6 +43,11 @@ def test_basetemp_inside_the_native_home_is_relocated_outside_it(tmp_path, monke
|
||||
relocated = config._tmp_path_factory._given_basetemp
|
||||
assert relocated is not None and not relocated.resolve().is_relative_to(native.resolve())
|
||||
assert config.option.basetemp == str(relocated)
|
||||
# One shared, prunable root (never a loose dir in the operator's $HOME) and gone when
|
||||
# this pytest exits; a run killed before that is swept as soon as the root is idle.
|
||||
assert relocated.parent.name == "hermes-pytest" and relocated.parent != Path.home()
|
||||
suite_conftest.pytest_unconfigure(config)
|
||||
assert not relocated.exists()
|
||||
# The sandbox derived from it no longer resolves to the native root.
|
||||
monkeypatch.setenv("HERMES_HOME", str(relocated / "t0" / "hermes_test"))
|
||||
assert hermes_constants.get_default_hermes_root() == relocated / "t0" / "hermes_test"
|
||||
@@ -65,3 +80,22 @@ def test_fallback_root_escapes_a_repo_checked_out_inside_the_native_home(tmp_pat
|
||||
|
||||
relocated = config._tmp_path_factory._given_basetemp
|
||||
assert relocated is not None and not relocated.resolve().is_relative_to(native.resolve())
|
||||
|
||||
|
||||
def test_relocation_root_sweeps_basetemps_of_killed_runs_and_keeps_live_ones(tmp_path, monkeypatch):
|
||||
import os
|
||||
import time
|
||||
|
||||
native = tmp_path / "native-home"
|
||||
native.mkdir()
|
||||
root = native.parent / "hermes-pytest"
|
||||
dead, live = root / "b-dead", root / "b-live"
|
||||
dead.mkdir(parents=True)
|
||||
live.mkdir()
|
||||
(dead / "f").write_text("x", encoding="utf-8")
|
||||
(live / "f").write_text("x", encoding="utf-8")
|
||||
ancient = time.time() - 30 * 3600
|
||||
for p in (dead, dead / "f", live):
|
||||
os.utime(p, (ancient, ancient))
|
||||
assert suite_conftest._pytest_disk_temp_root(native) == root
|
||||
assert not dead.exists() and live.exists()
|
||||
|
||||
Reference in New Issue
Block a user