Skip to content
40 changes: 40 additions & 0 deletions conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@
テストは、実行した人の設定に関わらずその場で落ちる
4. テストの実行中だけ実行の要約の置き場所(`NDF_METRICS_DIR`)を一時ディレクトリへ向ける。
状態を保存するテストが、実行した人の状態ディレクトリへ要約を書かない(#662 の AC72)
5. テストの実行中だけ監視の上限を指す環境変数(接頭辞 `MONITOR_`)を外す。上限を延ばした
シェルから起動しても、既定値を前提にするテストが同じ結果になる(#678)

どの束のディレクトリを起点にしても読まれるよう、テストの基準のディレクトリ(rootdir)は
根の設定ファイル(`pytest.ini`)がリポジトリの根へ固定する。

`playwright-kit-ops` のディレクトリを起点にした実行では、このファイルは読まれない。
`pytester` はそのディレクトリの `pyproject.toml` の `addopts` が読み込む。
Expand Down Expand Up @@ -82,6 +87,41 @@ def _missing(bundles: set[str]) -> dict[str, list[str]]:
return found


# 監視の上限を指す環境変数の接頭辞(#678)。担当ごとの指定・共通の指定のどちらもこの
# 接頭辞を持つため、接頭辞だけで一致させる。名前を並べると、上限の種類が増えるたびに
# ここへ足し忘れる。
MONITOR_ENV_PREFIX = "MONITOR_"

# `pytest_configure` で外した値の控え。実行が終わったときに戻す。
_saved_monitor_env: dict[str, str] = {}


def _strip_monitor_env() -> dict[str, str]:
"""接頭辞の環境変数を外し、外した値を返す。"""
return {k: os.environ.pop(k) for k in list(os.environ) if k.startswith(MONITOR_ENV_PREFIX)}


def pytest_configure(config) -> None:
Comment thread
takemi-ohama marked this conversation as resolved.
"""テストの実行中だけ、監視の上限を指す環境変数を外す(#678)。

無進捗の許容と打ち切りの上限は環境変数で延ばせる。運用で延ばしたシェルから起動すると、
表の既定値を前提にするテストが既定値ではなくその値を読み、変更の中身と関係なく落ちる。
収束ループの初期化は着手前のテストの通過を条件にするため、そこで止まる。

**収集より前に外す。** テストの本体を読み込む時点で上限を決めてしまう実装があり、
セッションの前提(fixture)では間に合わない。子プロセスは環境変数を受け継ぐため、
テストが起動する別プロセスにも同じ切り離しが効く。**個別に設定するテストは打ち消さない。**
`monkeypatch` も、別プロセスへ渡す上書きも、この後に効く。
"""
_saved_monitor_env.update(_strip_monitor_env())


def pytest_unconfigure(config) -> None:
"""実行が終わったら、外した環境変数を戻す。"""
os.environ.update(_saved_monitor_env)
_saved_monitor_env.clear()


def pytest_collection_modifyitems(config, items) -> None:
bundles = {b for item in items if (b := _bundle_of(Path(str(item.fspath)))) is not None}
missing = _missing(bundles)
Expand Down
89 changes: 89 additions & 0 deletions issues/issue-678-requirements.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# テストの前提: 監視の上限を環境変数で延ばしたシェルでは既定値を前提にするテストが落ち、収束ループの初期化が中断する → テストの実行中は監視の環境変数を共通の前提で外す(#678)

## 目的

無進捗の許容と打ち切りの上限は、環境変数で延ばせる。運用でこれを延ばしたシェルから全体の
テストを起動すると、既定値を前提にするテストが落ちる。落ちた原因はテストの実行環境にあり、
変更の中身にはない。

収束ループ(`cross-refactoring`)の初期化は着手前のテストの通過を条件にするため、上限を
延ばしたシェルでは初期化がそこで止まる。

**テストの実行中だけ、監視の環境変数を利用者の環境から切り離す。** 切り離しの置き場所は
リポジトリ直下の共通の前提(`conftest.py`)とし、テストごとに散った除去をそこへ寄せる。

## 対象範囲

**含む**

- リポジトリ直下の共通の前提へ、監視の環境変数(接頭辞 `MONITOR_`)を外す仕組みを足す
- テストごとに散った同じ除去を取り除く(1 変数ずつ外す箇所と、接頭辞でまとめて外す箇所)
- 共通の前提が働いていることを確かめるテストを足す

**含まない**

- 上限の解決順(担当ごとの指定 → 共通の指定 → 表の既定)の変更
- 上限の表の値の変更
- 監視の本体・起動スクリプト・Skill 本文の変更

## 前提

- 本番の振る舞いも本番コードの構造も変えない。変えるのはテストの前提だけである
- 子プロセスは実行中の環境変数を受け継ぐため、共通の前提で外せば、テストが起動する
別プロセスにも同じ切り離しが効く
- テストの中で監視の環境変数を設定する箇所(`monkeypatch.setenv`・別プロセスへ渡す
上書き)は、共通の前提より後に効くため、そのまま働く

## 着手前の実測(2026-09-21)

同じコマンドを、監視の環境変数を設定したシェルと、していないシェルで実行した。

```console
$ MONITOR_TIMEOUT_AGY=1800 MONITOR_STALL_AGY=1800 uv run --with pytest pytest scripts/tests plugins/ndf -q
FAILED plugins/ndf/skills/cross-review/tests/test_launch_agy.py::test_the_print_timeout_defaults_to_the_longest_phase
FAILED plugins/ndf/skills/cross-review/tests/test_monitor_agy.py::test_the_new_name_has_a_stall_default
FAILED plugins/ndf/skills/cross-review/tests/test_monitor_import_safety.py::test_import_succeeds_with_non_numeric_monitor_stall
3 failed, 4600 passed in 182.10s

$ uv run --with pytest pytest scripts/tests plugins/ndf -q
4603 passed in 179.67s
```

| 観測 | 値 |
| --- | --- |
| 収集した件数 | 4603(どちらのシェルでも同じ) |
| 設定したシェルで落ちる件数 | 3 |
| 設定していないシェルで落ちる件数 | 0 |

課題の本文が記録した 2026-09-15 の観測では落ちるのが 2 件、その後の追記で 3 件だった。
件数は着手時点の実測で 3 件のまま変わらない。

## 受け入れ条件

- [x] 受け入れ条件 1: 監視の環境変数(`MONITOR_TIMEOUT_AGY` と `MONITOR_STALL_AGY`)を設定した
シェルで全体のテストを実行すると、失敗が 0 件になる
- [x] 受け入れ条件 2: 設定したシェルと設定していないシェルで、通過した件数と失敗した件数が
一致する
- [x] 受け入れ条件 3: 共通の前提が接頭辞 `MONITOR_` の環境変数を外していることを、テストが
直接確かめる(実行中に該当する環境変数が 1 つも残らない)
- [x] 受け入れ条件 4: テストの中で監視の環境変数を設定する箇所は、共通の前提を足した後も
同じ値を観測できる(共通の前提が個別の設定を打ち消さない)
- [x] 受け入れ条件 5: 接頭辞でまとめて外していた箇所と、1 変数ずつ外していた箇所が、
共通の前提へ寄る(対象のファイルに同じ除去が残らない)
- [x] 受け入れ条件 6: 共通の前提は、テストの実行が終わった後に元の環境変数を戻す

## 検証手段

| 条件 | 確かめ方 |
| --- | --- |
| 1 / 2 | `MONITOR_TIMEOUT_AGY=1800 MONITOR_STALL_AGY=1800 uv run --with pytest pytest scripts/tests plugins/ndf -q` と、設定しない同じコマンドの 2 回を実行し、件数を突き合わせる |
| 3 / 4 / 6 | 共通の前提を確かめるテスト(`scripts/tests/test_root_conftest.py`)を実行する |
| 5 | `grep -rn "MONITOR_" --include="*.py" <テストのディレクトリ>` の結果に、接頭辞での除去と 1 変数ずつの除去が残らないことを確かめる |

## 境界

```text
常に行う … 共通の前提の追加、散った除去の削除、両方のシェルでの全体テスト
確認してから行う … 上限の解決順・表の既定値に触れる変更(この変更では行わない)
行わない … 監視の本体・起動スクリプト・Skill 本文の変更、依頼範囲外の整形
```
15 changes: 4 additions & 11 deletions plugins/ndf/scripts/tests/test_limits.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,19 @@


@pytest.fixture()
def limits(monkeypatch):
# **表の既定値を読むテストである。** 実行した人の環境の `MONITOR_*` を外す(#678)。
for key in [k for k in os.environ if k.startswith("MONITOR_")]:
monkeypatch.delenv(key)
def limits():
# 表の既定値を読むテストである。実行した人の環境の `MONITOR_*` は、根の
# `conftest.py` が実行中だけ外す(#678)。
spec = importlib.util.spec_from_file_location("ndf_lib_limits", LIMITS)
mod = importlib.util.module_from_spec(spec)
spec.loader.exec_module(mod)
return mod


def _clean_env(**over: str) -> dict[str, str]:
env = {k: v for k, v in os.environ.items() if not k.startswith("MONITOR_")}
env.update(over)
return env


def _run(*args: str, **env: str) -> subprocess.CompletedProcess[str]:
return subprocess.run(
[sys.executable, str(LIMITS), *args],
env=_clean_env(**env), capture_output=True, text=True,
env={**os.environ, **env}, capture_output=True, text=True,
Comment thread
takemi-ohama marked this conversation as resolved.
)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ def _launch(tmp_path: pathlib.Path, phase: str) -> tuple[list[str], pathlib.Path
subprocess.run(
[str(LAUNCH), RUNTIME, phase, "130", "1"],
env={
# 実行した人の `MONITOR_*` で上限が変わらないよう外す(#678)。
**{k: v for k, v in os.environ.items() if not k.startswith("MONITOR_")},
**os.environ,
"CROSS_REFACTORING_TMP_DIR": str(state_path.parent),
"PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}",
"NDF_TEST_ARGS_FILE": str(args_file),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ def _env(tmp_path: pathlib.Path, **over: str) -> dict[str, str]:
stub = bin_dir / "agy"
stub.write_text(STUB, encoding="utf-8")
stub.chmod(0o755)
# 実行した人の `MONITOR_*` で値が変わらないよう外す(#678)。
env = {k: v for k, v in os.environ.items() if not k.startswith("MONITOR_")}
env = dict(os.environ)
env.pop("NDF_CRITIQUE_PRINT_TIMEOUT", None)
env.update({
"PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,8 @@ def test_claude_stdout_scan_ignores_missing_file(monitor_mod, tmp_path):

# ---------- 5. 追加ランタイムの stall 既定 ----------

def test_stall_defaults_cover_claude_and_kiro(monitor_mod, monkeypatch):
def test_stall_defaults_cover_claude_and_kiro(monitor_mod):
"""`claude -p` は完了まで無出力なので、最も長い既定を持つこと。"""
monkeypatch.delenv("MONITOR_STALL", raising=False)
monkeypatch.delenv("MONITOR_STALL_CLAUDE", raising=False)
monkeypatch.delenv("MONITOR_STALL_KIRO", raising=False)
assert monitor_mod._agent_stall_default("claude") == 900
assert monitor_mod._agent_stall_default("kiro") == 480

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,10 @@ def _dead_pid() -> int:

def _run_monitor(tmp_dir: pathlib.Path, *extra: str, script: pathlib.Path = _MONITOR_LIB,
pr: int = 7, agents: str = "codex") -> subprocess.CompletedProcess:
env = {k: v for k, v in os.environ.items() if not k.startswith("MONITOR_")}
return subprocess.run(
[sys.executable, str(script), str(pr), "--agents", agents,
"--tmp-dir", str(tmp_dir), "--poll", "1", *extra],
capture_output=True, text=True, env=env, timeout=60,
capture_output=True, text=True, timeout=60,
)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,10 @@ def _run(tmp_dir: pathlib.Path, *extra: str, agents: str = "agy",
for agent in agents.split(","):
(tmp_dir / f"{agent}-review-pr7.pid").write_text(str(_dead_pid()))
(tmp_dir / f"{agent}-review-pr7-result.json").write_text('{"event": "APPROVE"}')
base = {k: v for k, v in os.environ.items() if not k.startswith("MONITOR_")}
return subprocess.run(
[sys.executable, str(_MONITOR_LIB), "7", "--agents", agents,
"--tmp-dir", str(tmp_dir), "--poll", "1", *extra],
capture_output=True, text=True, env={**base, **(env or {})}, timeout=60,
capture_output=True, text=True, env={**os.environ, **(env or {})}, timeout=60,
)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,31 +7,26 @@

agy は err.log にほぼ進捗を出さないため、ビルトイン既定を 480s と大きめに
取って 1 度目の STALLED 誤検知を避ける。codex は従来通り 180s で変更なし。

実行した人の `MONITOR_*` は、根の `conftest.py` が実行中だけ外す(#678)。
"""
from __future__ import annotations

import pytest


def test_builtin_default_codex(monkeypatch, monitor_mod):
def test_builtin_default_codex(monitor_mod):
"""codex のビルトイン既定は 180s。"""
monkeypatch.delenv("MONITOR_STALL", raising=False)
monkeypatch.delenv("MONITOR_STALL_CODEX", raising=False)
monkeypatch.delenv("MONITOR_STALL_AGY", raising=False)
assert monitor_mod._agent_stall_default("codex") == 180


def test_builtin_default_agy(monkeypatch, monitor_mod):
def test_builtin_default_agy(monitor_mod):
"""agy のビルトイン既定は 480s (codex より大きい)。"""
monkeypatch.delenv("MONITOR_STALL", raising=False)
monkeypatch.delenv("MONITOR_STALL_CODEX", raising=False)
monkeypatch.delenv("MONITOR_STALL_AGY", raising=False)
assert monitor_mod._agent_stall_default("agy") == 480


def test_per_agent_env_overrides_builtin(monkeypatch, monitor_mod):
"""env `MONITOR_STALL_AGY` 設定で agy 既定が上書きされる。"""
monkeypatch.delenv("MONITOR_STALL", raising=False)
monkeypatch.setenv("MONITOR_STALL_AGY", "600")
assert monitor_mod._agent_stall_default("agy") == 600
# codex は影響を受けない
Expand All @@ -46,8 +41,6 @@ def test_shared_env_applies_to_both(monkeypatch, monitor_mod):
本テストは monkeypatch で `MONITOR_STALL=240` に書き換え、両 agent が 240 を
返すことを確認する (= 共通 env が実際に反映されることの検証)。
"""
monkeypatch.delenv("MONITOR_STALL_CODEX", raising=False)
monkeypatch.delenv("MONITOR_STALL_AGY", raising=False)
monkeypatch.setenv("MONITOR_STALL", "240")
# 共通 env が両 agent に効く (per-agent 上書きなしの場合)
assert monitor_mod._agent_stall_default("codex") == 240
Expand All @@ -58,16 +51,13 @@ def test_per_agent_env_takes_precedence_over_shared(monkeypatch, monitor_mod):
"""per-agent env > 共通 env の優先順位を確認する。"""
monkeypatch.setenv("MONITOR_STALL", "240")
monkeypatch.setenv("MONITOR_STALL_AGY", "777")
monkeypatch.delenv("MONITOR_STALL_CODEX", raising=False)
assert monitor_mod._agent_stall_default("agy") == 777
# codex 側は per-agent env が無いので 共通 env (= 240) にフォールバック
assert monitor_mod._agent_stall_default("codex") == 240


def test_unknown_agent_falls_back_to_default_stall(monkeypatch, monitor_mod):
def test_unknown_agent_falls_back_to_default_stall(monitor_mod):
"""ビルトインに無い agent 名は `DEFAULT_STALL` にフォールバックする。"""
monkeypatch.delenv("MONITOR_STALL", raising=False)
monkeypatch.delenv("MONITOR_STALL_UNKNOWN", raising=False)
assert monitor_mod._agent_stall_default("unknown") == monitor_mod.DEFAULT_STALL


Expand All @@ -80,8 +70,6 @@ def test_shared_env_non_numeric_falls_back_to_builtin(monkeypatch, monitor_mod,
gemini round 4 指摘: `int(os.environ[...])` は非数値で ValueError を出す。
監視プロセスを env 設定ミスでクラッシュさせないため、try/except で builtin に戻す。
"""
monkeypatch.delenv("MONITOR_STALL_CODEX", raising=False)
monkeypatch.delenv("MONITOR_STALL_AGY", raising=False)
monkeypatch.setenv("MONITOR_STALL", "abc")
# codex / agy とも builtin 既定 (180 / 480) に戻る
assert monitor_mod._agent_stall_default("codex") == 180
Expand All @@ -96,9 +84,7 @@ def test_per_agent_env_non_numeric_falls_back_to_builtin(
monkeypatch, monitor_mod, capsys
):
"""env `MONITOR_STALL_<AGENT>` が非数値なら builtin にフォールバック。"""
monkeypatch.delenv("MONITOR_STALL", raising=False)
monkeypatch.setenv("MONITOR_STALL_AGY", "not-a-number")
monkeypatch.delenv("MONITOR_STALL_CODEX", raising=False)
# agy は builtin (480) にフォールバック
assert monitor_mod._agent_stall_default("agy") == 480
# codex は env 未設定なので builtin (180)
Expand All @@ -112,7 +98,6 @@ def test_per_agent_env_non_numeric_does_not_affect_other_agent(
monkeypatch, monitor_mod
):
"""non-numeric な per-agent env は対象 agent だけに影響する。"""
monkeypatch.delenv("MONITOR_STALL", raising=False)
monkeypatch.setenv("MONITOR_STALL_AGY", "xxx")
monkeypatch.setenv("MONITOR_STALL_CODEX", "200") # codex 側は正常
assert monitor_mod._agent_stall_default("codex") == 200
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
from __future__ import annotations

import json
import os
import pathlib
import subprocess
import sys
Expand Down Expand Up @@ -170,11 +169,10 @@ def _dead_pid() -> int:


def _run_monitor(tmp_dir: pathlib.Path, agent: str, *extra: str) -> subprocess.CompletedProcess:
env = {k: v for k, v in os.environ.items() if not k.startswith("MONITOR_")}
return subprocess.run(
[sys.executable, str(_MONITOR_LIB), "7", "--agents", agent,
"--tmp-dir", str(tmp_dir), "--poll", "1", *extra],
capture_output=True, text=True, env=env, timeout=60,
capture_output=True, text=True, timeout=60,
)


Expand Down
8 changes: 8 additions & 0 deletions pytest.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# 起点のディレクトリに関わらず、テストの基準のディレクトリ(rootdir)をリポジトリの根へ
# 解決させるために置く。pytest は起点から上へ設定ファイルを探し、見つかったところで止まる。
# 根に設定ファイルが 1 つも無いと、テストの束のディレクトリを起点にした実行では基準が
# そこで止まり、根の共通の前提(`conftest.py`)が読み込まれない。監視の上限を指す環境変数
# (接頭辞 `MONITOR_`)の除去のように、どの起点でも効かなければならない前提がここに載る。
#
# **設定値は足さない。** `testpaths` などを書くと、既存の実行が対象にする範囲が変わる。
[pytest]
Loading
Loading