test: isolate default engine-state dir per test (env var + opt-out marker) (#782)

Completes the recommended fix in docs/issues/default-engine-state-test-hygiene.md.
The root conftest autouse fixture already redirected engine_state._DEFAULT_DIR
per test (the in-process half, req #1); this adds the two missing halves:

- monkeypatch.setenv("CORE_ENGINE_STATE_DIR", isolated) so subprocess / CLI
  tests that re-import engine_state in a child process inherit the same
  isolation (req #2). A child that sets its own env still overrides and wins.
- @pytest.mark.uses_default_engine_state opt-out (registered in pyproject.toml)
  for tests that intentionally exercise the real process-default dir (req #3).

Adds tests/test_conftest_engine_state_isolation.py: non-vacuous proof that each
half fires and that the marker opts out; each assertion fails if its half is
removed (CLAUDE.md "Schema-Defined Proof Obligations").

Validation: full serial baseline on origin/main (95a06a20) = 38 pre-existing
reds (21 failed + 17 error). The only new behavior over baseline is the setenv,
whose sole live effect is child processes; a full re-run of the subprocess/CLI
blast radius shows zero new failures, and the documented test_achat
identity-continuity warning is gone. Does NOT add -n auto adoption or the
fresh-env-dict subprocess hermeticity fix (tracked separately).
This commit is contained in:
Shay 2026-06-15 14:46:20 -07:00 committed by GitHub
parent 1ff06726a6
commit 01966ef92e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 77 additions and 1 deletions

View file

@ -28,8 +28,11 @@ import pytest
import engine_state
_USES_DEFAULT_ENGINE_STATE_MARKER = "uses_default_engine_state"
@pytest.fixture(autouse=True)
def _isolate_engine_state_default(tmp_path_factory, monkeypatch):
def _isolate_engine_state_default(request, tmp_path_factory, monkeypatch):
"""Isolate the default engine-state checkpoint dir per test.
A bare ``ChatRuntime()`` (no ``engine_state_path``) falls back to
@ -41,9 +44,27 @@ def _isolate_engine_state_default(tmp_path_factory, monkeypatch):
boots under a different identity over the same dir). Point the default at a
fresh per-test temp dir. Tests passing an explicit ``engine_state_path`` are
unaffected; within one test, repeated ``ChatRuntime()`` share this dir.
Two redirections, both pointed at the same per-test dir:
1. ``engine_state._DEFAULT_DIR`` is monkeypatched directly. It is bound at
import (``engine_state/__init__.py``), so an env var alone would NOT
redirect an already-imported in-process runtime.
2. ``CORE_ENGINE_STATE_DIR`` is set in the environment so subprocess / CLI
tests that re-import ``engine_state`` in a child process inherit the same
isolation. A child that sets its own ``CORE_ENGINE_STATE_DIR`` (e.g.
``tests/test_l10_always_on_daemon.py::test_real_sigterm_stops_the_daemon_cleanly``)
still overrides this and wins.
A test that intentionally exercises the real process-default dir (default-dir
semantics, CLI fallback, legacy-flat migration) can opt out with
``@pytest.mark.uses_default_engine_state``; it then sees neither redirection.
"""
if request.node.get_closest_marker(_USES_DEFAULT_ENGINE_STATE_MARKER):
return
isolated = tmp_path_factory.mktemp("engine_state_default")
monkeypatch.setattr(engine_state, "_DEFAULT_DIR", isolated)
monkeypatch.setenv("CORE_ENGINE_STATE_DIR", str(isolated))
QUARANTINE: frozenset[str] = frozenset()

View file

@ -61,4 +61,5 @@ asyncio_mode = "auto"
testpaths = ["tests"]
markers = [
"quarantine: pre-existing failure tracked in conftest.py QUARANTINE registry; excluded from full-pytest CI gate. See docs/test-debt-quarantine.md.",
"uses_default_engine_state: test intentionally exercises the process default engine-state directory; opt out of per-test default isolation. See docs/issues/default-engine-state-test-hygiene.md.",
]

View file

@ -0,0 +1,54 @@
"""Proof tests for the root-conftest engine-state isolation fixture.
Covers ``conftest.py::_isolate_engine_state_default`` and the recommended fix in
``docs/issues/default-engine-state-test-hygiene.md``. Each assertion fails if the
corresponding half of the fixture is removed, so these are load-bearing, not
decoration (CLAUDE.md "Schema-Defined Proof Obligations"):
- drop the ``monkeypatch.setattr`` ``test_default_dir_is_redirected_*`` fails,
- drop the ``monkeypatch.setenv`` ``test_env_var_matches_*`` fails,
- drop the marker opt-out check ``test_opt_out_marker_*`` fails.
"""
from __future__ import annotations
import os
from pathlib import Path
import pytest
import engine_state
def test_default_dir_is_redirected_to_isolated_tmp() -> None:
"""(req #1) the import-time-bound module attribute is redirected per test."""
default_dir = Path(engine_state._DEFAULT_DIR)
assert "engine_state_default" in str(default_dir), (
"fixture did not monkeypatch engine_state._DEFAULT_DIR to an isolated dir"
)
repo_default = Path(engine_state.__file__).resolve().parents[1] / "engine_state"
assert default_dir.resolve() != repo_default.resolve(), (
"isolated default dir must not be the in-repo engine_state/ dir"
)
def test_env_var_matches_the_isolated_default_dir() -> None:
"""(req #2) CORE_ENGINE_STATE_DIR is set for child processes, to the SAME dir."""
env_dir = os.environ.get("CORE_ENGINE_STATE_DIR")
assert env_dir is not None, "fixture did not set CORE_ENGINE_STATE_DIR"
assert env_dir == str(engine_state._DEFAULT_DIR), (
"CORE_ENGINE_STATE_DIR and engine_state._DEFAULT_DIR must agree so an "
"in-process runtime and a subprocess child resolve the same isolated dir"
)
@pytest.mark.uses_default_engine_state
def test_opt_out_marker_skips_isolation() -> None:
"""(req #3) the marker opts a test out of both redirections.
Asserted as a contrast (not an absolute path) so it is robust whether or not
CORE_ENGINE_STATE_DIR is set in the ambient environment: an opted-out test must
NOT receive the per-test ``mktemp("engine_state_default")`` dir.
"""
assert "engine_state_default" not in str(engine_state._DEFAULT_DIR), (
"uses_default_engine_state marker did not opt out of the isolation fixture"
)