diff --git a/conftest.py b/conftest.py index a5c85373..2e04c864 100644 --- a/conftest.py +++ b/conftest.py @@ -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() diff --git a/pyproject.toml b/pyproject.toml index 1ce0b7b5..c3907707 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -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.", ] diff --git a/tests/test_conftest_engine_state_isolation.py b/tests/test_conftest_engine_state_isolation.py new file mode 100644 index 00000000..5f675e83 --- /dev/null +++ b/tests/test_conftest_engine_state_isolation.py @@ -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" + )