Merge pull request 'fix(tests): isolate xdist polluters blocking -n auto default (W3)' (#46) from fix/xdist-polluter-isolation into main
Reviewed-on: #46
This commit is contained in:
commit
b0156406dd
8 changed files with 3294 additions and 26 deletions
1
.gitignore
vendored
1
.gitignore
vendored
|
|
@ -15,7 +15,6 @@ workbench_data/*
|
|||
core-rs/target/
|
||||
core-rs/Cargo.lock
|
||||
|
||||
uv.lock
|
||||
|
||||
# Environment secrets — never commit real keys
|
||||
.env
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ single scoring path.
|
|||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
|
@ -33,7 +34,13 @@ from evals.gsm8k_math.runner import _score_one_candidate_graph
|
|||
|
||||
_HERE = Path(__file__).resolve().parent
|
||||
_CASES_PATH = _HERE / "cases.jsonl"
|
||||
_REPORT_PATH = _HERE / "report.json"
|
||||
# Overridable so concurrent test invocations (xdist workers) can each point
|
||||
# the CLI entrypoint at an isolated tmp path instead of racing on the shared
|
||||
# committed report.json. Unset (the default) preserves the real operator
|
||||
# contract: `python -m ...runner` writes the tracked repo file.
|
||||
_REPORT_PATH = Path(
|
||||
os.environ.get("CORE_GSM8K_TRAIN_SAMPLE_REPORT_PATH", str(_HERE / "report.json"))
|
||||
)
|
||||
_SAMPLE_REL = "evals/gsm8k_math/train_sample/v1/cases.jsonl"
|
||||
_EXPECTED_COUNT = 50
|
||||
_CORRECT_MIN = 10
|
||||
|
|
|
|||
|
|
@ -10,6 +10,24 @@ import pytest
|
|||
from core import cli
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _isolate_demo_results_dir(tmp_path, monkeypatch):
|
||||
"""Redirect the demo results dir to an isolated per-test tmp dir.
|
||||
|
||||
``core.cli._DEMO_RESULTS_DIR`` is a module-level constant pointing at the
|
||||
shared repo dir ``evals/forward_semantic_control/results/`` that every
|
||||
``core demo`` subcommand reads, writes, and glob-scans (including the
|
||||
``index.json`` rebuild in ``_write_results_index``). Multiple tests below
|
||||
invoke ``cli.main(["demo", ...])``; left unpatched they race on that one
|
||||
shared directory under ``-n auto`` (concurrent report writes plus a
|
||||
scan-then-rewrite of ``index.json``). Follows the same monkeypatch-the-
|
||||
module-attribute isolation idiom as the root-conftest engine-state fixture
|
||||
(#782). ``TestResultsReadme`` reads the real checked-in README by a
|
||||
hardcoded path, so it is unaffected by this redirection.
|
||||
"""
|
||||
monkeypatch.setattr(cli, "_DEMO_RESULTS_DIR", tmp_path / "results")
|
||||
|
||||
|
||||
class TestADR0024SuiteAliases:
|
||||
"""Layer 1: pin the new suite aliases so they don't drift."""
|
||||
|
||||
|
|
@ -93,7 +111,7 @@ class TestDemoSubcommand:
|
|||
assert "ALL THREE CONDITIONS" in captured.out
|
||||
assert "PASS" in captured.out
|
||||
# Report file present and well-formed.
|
||||
report = Path("evals/forward_semantic_control/results/phase6_demo_report.json")
|
||||
report = cli._DEMO_RESULTS_DIR / "phase6_demo_report.json"
|
||||
assert report.exists()
|
||||
data = json.loads(report.read_text())
|
||||
assert data["metrics"]["all_three_conditions_pass"] is True
|
||||
|
|
@ -109,6 +127,12 @@ class TestDemoSubcommand:
|
|||
assert "all_three_conditions_pass" in payload["metrics"]
|
||||
|
||||
def test_demo_list_results_indexes_reports(self, capsys) -> None:
|
||||
# Self-contained: write a report before indexing rather than relying
|
||||
# on a sibling test's shared-directory side effect (that implicit
|
||||
# ordering coupling only "worked" by accident of file-declaration
|
||||
# order and would break under `-n auto` reordering).
|
||||
cli.main(["demo", "phase6"])
|
||||
capsys.readouterr()
|
||||
rc = cli.main(["demo", "list-results"])
|
||||
assert rc == 0
|
||||
captured = capsys.readouterr()
|
||||
|
|
@ -116,6 +140,10 @@ class TestDemoSubcommand:
|
|||
assert "phase6_demo_report.json" in captured.out
|
||||
|
||||
def test_demo_list_results_json_well_formed(self, capsys) -> None:
|
||||
# Self-contained for the same reason as
|
||||
# test_demo_list_results_indexes_reports above.
|
||||
cli.main(["demo", "phase6"])
|
||||
capsys.readouterr()
|
||||
rc = cli.main(["demo", "list-results", "--json"])
|
||||
assert rc == 0
|
||||
captured = capsys.readouterr()
|
||||
|
|
@ -127,7 +155,7 @@ class TestDemoSubcommand:
|
|||
|
||||
def test_demo_index_file_refreshed_after_run(self) -> None:
|
||||
cli.main(["demo", "phase6"])
|
||||
index_path = Path("evals/forward_semantic_control/results/index.json")
|
||||
index_path = cli._DEMO_RESULTS_DIR / "index.json"
|
||||
assert index_path.exists()
|
||||
data = json.loads(index_path.read_text())
|
||||
names = [e["file"] for e in data["reports"]]
|
||||
|
|
|
|||
|
|
@ -114,19 +114,19 @@ def test_canonical_pack_admits_cross_sentence_composition_when_seeded():
|
|||
)
|
||||
|
||||
|
||||
def test_wrong_zero_preserved_on_train_sample():
|
||||
def test_wrong_zero_preserved_on_train_sample(tmp_path):
|
||||
"""The full train_sample eval must preserve wrong == 0 after RAT-1."""
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
report_path = tmp_path / "report.json"
|
||||
result = subprocess.run(
|
||||
[sys.executable, "-m", "evals.gsm8k_math.train_sample.v1.runner"],
|
||||
cwd=_repo_root(),
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
report_path = (
|
||||
_repo_root() / "evals" / "gsm8k_math" / "train_sample" / "v1" / "report.json"
|
||||
env={**os.environ, "CORE_GSM8K_TRAIN_SAMPLE_REPORT_PATH": str(report_path)},
|
||||
)
|
||||
assert report_path.exists(), "train_sample runner must emit report.json"
|
||||
report = json.loads(report_path.read_text())
|
||||
|
|
@ -134,18 +134,18 @@ def test_wrong_zero_preserved_on_train_sample():
|
|||
assert counts["wrong"] == 0, f"wrong-zero invariant violated: {counts}"
|
||||
|
||||
|
||||
def test_case_0050_remains_refused_after_rat1():
|
||||
def test_case_0050_remains_refused_after_rat1(tmp_path):
|
||||
"""Hazard pin: case 0050 must not admit after any RAT-1 wiring."""
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
report_path = tmp_path / "report.json"
|
||||
subprocess.run(
|
||||
[sys.executable, "-m", "evals.gsm8k_math.train_sample.v1.runner"],
|
||||
cwd=_repo_root(),
|
||||
capture_output=True,
|
||||
)
|
||||
report_path = (
|
||||
_repo_root() / "evals" / "gsm8k_math" / "train_sample" / "v1" / "report.json"
|
||||
env={**os.environ, "CORE_GSM8K_TRAIN_SAMPLE_REPORT_PATH": str(report_path)},
|
||||
)
|
||||
report = json.loads(report_path.read_text())
|
||||
case = next(
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ from unittest.mock import patch
|
|||
|
||||
import pytest
|
||||
|
||||
import engine_state
|
||||
from core.cli import main
|
||||
from teaching.discovery import DiscoveryCandidate, EvidencePointer
|
||||
from teaching.proposals import ProposalLog, ReplayEvidence, build_proposal
|
||||
|
|
@ -515,11 +516,21 @@ def snapshot_dir(directory: Path) -> dict[Path, bytes]:
|
|||
|
||||
|
||||
def test_read_only_invariant():
|
||||
"""Directories the hitl-queue commands must not mutate.
|
||||
|
||||
``engine_state`` is read via ``engine_state._DEFAULT_DIR`` (not a
|
||||
hardcoded ``project_root / "engine_state"``) because the root-conftest
|
||||
``_isolate_engine_state_default`` autouse fixture repoints it at a
|
||||
per-test tmp dir; a hardcoded real path would instead snapshot the
|
||||
shared repo dir and false-fail under ``-n auto`` whenever an unrelated
|
||||
concurrent worker wrote to it (observed: AssertionError on this exact
|
||||
directory under a full `-n auto` fast-lane run).
|
||||
"""
|
||||
project_root = Path(__file__).resolve().parent.parent
|
||||
dirs = [
|
||||
project_root / "teaching" / "proposals",
|
||||
project_root / "packs",
|
||||
project_root / "engine_state",
|
||||
Path(engine_state._DEFAULT_DIR),
|
||||
project_root / "contemplation" / "runs",
|
||||
]
|
||||
|
||||
|
|
|
|||
|
|
@ -171,41 +171,43 @@ def _has_wave_a_seed() -> bool:
|
|||
)
|
||||
|
||||
|
||||
def test_wrong_zero_preserved():
|
||||
def test_wrong_zero_preserved(tmp_path):
|
||||
"""The full train_sample eval keeps wrong == 0 after WAVE-A."""
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
here = Path(__file__).resolve()
|
||||
while here.parent != here and not (here / "pyproject.toml").exists():
|
||||
here = here.parent
|
||||
report_path = tmp_path / "report.json"
|
||||
subprocess.run(
|
||||
[sys.executable, "-m", "evals.gsm8k_math.train_sample.v1.runner"],
|
||||
cwd=here,
|
||||
capture_output=True,
|
||||
env={**os.environ, "CORE_GSM8K_TRAIN_SAMPLE_REPORT_PATH": str(report_path)},
|
||||
)
|
||||
report = json.loads(
|
||||
(here / "evals" / "gsm8k_math" / "train_sample" / "v1" / "report.json").read_text()
|
||||
)
|
||||
report = json.loads(report_path.read_text())
|
||||
assert report["counts"]["wrong"] == 0
|
||||
|
||||
|
||||
def test_case_0050_remains_refused():
|
||||
def test_case_0050_remains_refused(tmp_path):
|
||||
"""Hazard pin."""
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
here = Path(__file__).resolve()
|
||||
while here.parent != here and not (here / "pyproject.toml").exists():
|
||||
here = here.parent
|
||||
report_path = tmp_path / "report.json"
|
||||
subprocess.run(
|
||||
[sys.executable, "-m", "evals.gsm8k_math.train_sample.v1.runner"],
|
||||
cwd=here,
|
||||
capture_output=True,
|
||||
env={**os.environ, "CORE_GSM8K_TRAIN_SAMPLE_REPORT_PATH": str(report_path)},
|
||||
)
|
||||
report = json.loads(
|
||||
(here / "evals" / "gsm8k_math" / "train_sample" / "v1" / "report.json").read_text()
|
||||
)
|
||||
report = json.loads(report_path.read_text())
|
||||
case_0050 = next(
|
||||
(c for c in report["per_case"] if c["case_id"].endswith("-0050")),
|
||||
None,
|
||||
|
|
|
|||
|
|
@ -12,14 +12,13 @@ from pathlib import Path
|
|||
|
||||
import pytest
|
||||
|
||||
import engine_state
|
||||
from chat.runtime import ChatRuntime
|
||||
from workbench import api as workbench_api
|
||||
from workbench.api import WorkbenchApi, _run_sealed_chat_turn, _with_turn_cost_and_id
|
||||
from workbench.journal import TurnJournal, TurnJournalEntry
|
||||
from workbench.replay import CRITICAL_FIELDS, INFORMATIONAL_FIELDS, replay_turn
|
||||
|
||||
_ENGINE_STATE_DIR = Path("engine_state")
|
||||
|
||||
|
||||
def _snapshot(root: Path) -> dict[str, bytes]:
|
||||
snap: dict[str, bytes] = {}
|
||||
|
|
@ -150,19 +149,26 @@ def test_hashless_legacy_turn_is_not_replayable(tmp_path, recorded, monkeypatch)
|
|||
|
||||
|
||||
def test_replay_leaves_no_trace(tmp_path, recorded) -> None:
|
||||
"""Obligation 4: GET /replay writes nothing — journal and engine state."""
|
||||
"""Obligation 4: GET /replay writes nothing — journal and engine state.
|
||||
|
||||
``engine_state._DEFAULT_DIR`` is read fresh (not a module-level constant)
|
||||
because the root-conftest ``_isolate_engine_state_default`` autouse
|
||||
fixture repoints it at a per-test tmp dir; a hardcoded ``Path("engine_state")``
|
||||
would instead snapshot the real shared repo dir and false-fail under
|
||||
``-n auto`` whenever an unrelated concurrent worker wrote to it.
|
||||
"""
|
||||
entry, _ = recorded
|
||||
journal = TurnJournal(journal_dir=tmp_path / "workbench_data")
|
||||
journal.append(entry)
|
||||
api = WorkbenchApi(journal=journal)
|
||||
journal_before = _snapshot(tmp_path / "workbench_data")
|
||||
engine_state_before = _snapshot(_ENGINE_STATE_DIR)
|
||||
engine_state_before = _snapshot(Path(engine_state._DEFAULT_DIR))
|
||||
|
||||
response = api.handle("GET", "/replay/1", b"")
|
||||
|
||||
assert response.status == 200
|
||||
assert _snapshot(tmp_path / "workbench_data") == journal_before
|
||||
assert _snapshot(_ENGINE_STATE_DIR) == engine_state_before
|
||||
assert _snapshot(Path(engine_state._DEFAULT_DIR)) == engine_state_before
|
||||
|
||||
|
||||
def test_wall_clock_divergence_does_not_break_equivalence(tmp_path, recorded) -> None:
|
||||
|
|
|
|||
Loading…
Reference in a new issue