255 lines
12 KiB
Python
255 lines
12 KiB
Python
"""KIN-145 — Regression tests for merge_worktree retry/sleep blocking fix.
|
||
|
||
Root cause: agents/runner.py was calling merge_worktree(worktree_path, p_path,
|
||
max_retries=3, retry_delay_s=15). Git conflicts are deterministic and cannot
|
||
be resolved by retrying. Each retry causes time.sleep(15) → 3 retries = 45s
|
||
blocking sleep. The pipeline thread blocks, the parent web-server times out,
|
||
_check_parent_alive catches ESRCH → all active pipelines marked as failed →
|
||
mass failure across all projects.
|
||
|
||
Fix: Removed max_retries=3, retry_delay_s=15 kwargs from runner.py:2076.
|
||
merge_worktree now uses default max_retries=0 → conflict returns immediately.
|
||
|
||
Tests verify:
|
||
A. merge_worktree with default max_retries=0 does NOT call time.sleep on conflict
|
||
B. merge_worktree with default args returns {success: False} immediately on conflict
|
||
C. merge_worktree with explicit max_retries=N DOES sleep N times (feature intact)
|
||
D. Old kwargs (max_retries=3, retry_delay_s=15) would produce 45s total blocking
|
||
E. runner.py source code does NOT pass max_retries or retry_delay_s (static guard)
|
||
"""
|
||
|
||
import re
|
||
from pathlib import Path
|
||
from unittest.mock import MagicMock, patch
|
||
|
||
import pytest
|
||
|
||
|
||
# ─────────────────────────────────────────────────────────────────────────────
|
||
# Helpers
|
||
# ─────────────────────────────────────────────────────────────────────────────
|
||
|
||
def _ok_run():
|
||
m = MagicMock()
|
||
m.returncode = 0
|
||
m.stdout = ""
|
||
m.stderr = ""
|
||
return m
|
||
|
||
|
||
def _fail_run(stderr="CONFLICT (content): merge conflict in src/app.py"):
|
||
m = MagicMock()
|
||
m.returncode = 1
|
||
m.stdout = ""
|
||
m.stderr = stderr
|
||
return m
|
||
|
||
|
||
def _conflict_diff():
|
||
m = MagicMock()
|
||
m.returncode = 0
|
||
m.stdout = "src/conflict.py\n"
|
||
m.stderr = ""
|
||
return m
|
||
|
||
|
||
def _side_effects_for_n_conflict_attempts(n_attempts: int) -> list:
|
||
"""Build subprocess.run side_effects for n conflict attempts.
|
||
|
||
Structure: git-add, git-commit, then per-attempt: merge-fail + diff + abort.
|
||
"""
|
||
effects = [_ok_run(), _ok_run()] # git add -A, git commit
|
||
for _ in range(n_attempts):
|
||
effects.extend([_fail_run(), _conflict_diff(), _ok_run()]) # merge-fail, diff, abort
|
||
return effects
|
||
|
||
|
||
# ─────────────────────────────────────────────────────────────────────────────
|
||
# A & B. Default max_retries=0: no sleep, immediate failure return
|
||
# ─────────────────────────────────────────────────────────────────────────────
|
||
|
||
class TestMergeWorktreeNoRetryDefault:
|
||
@patch("core.worktree.time.sleep")
|
||
@patch("subprocess.run")
|
||
def test_merge_worktree_default_args_does_not_sleep_on_conflict(
|
||
self, mock_run, mock_sleep, tmp_path
|
||
):
|
||
"""KIN-145 fix A: merge_worktree с аргументами по умолчанию НЕ вызывает sleep при конфликте."""
|
||
from core.worktree import merge_worktree
|
||
|
||
worktree_path = tmp_path / ".kin_worktrees" / "T-145-dev"
|
||
worktree_path.mkdir(parents=True)
|
||
mock_run.side_effect = _side_effects_for_n_conflict_attempts(1)
|
||
|
||
merge_worktree(str(worktree_path), str(tmp_path))
|
||
|
||
mock_sleep.assert_not_called()
|
||
|
||
@patch("core.worktree.time.sleep")
|
||
@patch("subprocess.run")
|
||
def test_merge_worktree_default_args_returns_failure_immediately_on_conflict(
|
||
self, mock_run, mock_sleep, tmp_path
|
||
):
|
||
"""KIN-145 fix B: merge_worktree без retry немедленно возвращает success=False при конфликте."""
|
||
from core.worktree import merge_worktree
|
||
|
||
worktree_path = tmp_path / ".kin_worktrees" / "T-145-dev"
|
||
worktree_path.mkdir(parents=True)
|
||
mock_run.side_effect = _side_effects_for_n_conflict_attempts(1)
|
||
|
||
result = merge_worktree(str(worktree_path), str(tmp_path))
|
||
|
||
assert result["success"] is False
|
||
assert "src/conflict.py" in result["conflicts"]
|
||
|
||
@patch("core.worktree.time.sleep")
|
||
@patch("subprocess.run")
|
||
def test_merge_worktree_explicit_max_retries_0_no_sleep(
|
||
self, mock_run, mock_sleep, tmp_path
|
||
):
|
||
"""KIN-145: explicit max_retries=0 тоже не вызывает sleep при конфликте."""
|
||
from core.worktree import merge_worktree
|
||
|
||
worktree_path = tmp_path / ".kin_worktrees" / "T-145-b-dev"
|
||
worktree_path.mkdir(parents=True)
|
||
mock_run.side_effect = _side_effects_for_n_conflict_attempts(1)
|
||
|
||
merge_worktree(str(worktree_path), str(tmp_path), max_retries=0)
|
||
|
||
mock_sleep.assert_not_called()
|
||
|
||
|
||
# ─────────────────────────────────────────────────────────────────────────────
|
||
# C. Retry feature still works when explicitly requested
|
||
# ─────────────────────────────────────────────────────────────────────────────
|
||
|
||
class TestMergeWorktreeRetryBehaviorIntact:
|
||
@patch("core.worktree.time.sleep")
|
||
@patch("subprocess.run")
|
||
def test_merge_worktree_max_retries_2_sleeps_twice(
|
||
self, mock_run, mock_sleep, tmp_path
|
||
):
|
||
"""KIN-145 fix C: retry-функциональность не сломана — max_retries=2 вызывает sleep 2 раза."""
|
||
from core.worktree import merge_worktree
|
||
|
||
worktree_path = tmp_path / ".kin_worktrees" / "T-145-retry-dev"
|
||
worktree_path.mkdir(parents=True)
|
||
# max_retries=2 → 3 total attempts (attempt 0,1,2)
|
||
# sleep happens after attempt 0 and 1 (not after final attempt 2)
|
||
mock_run.side_effect = _side_effects_for_n_conflict_attempts(3)
|
||
|
||
merge_worktree(str(worktree_path), str(tmp_path), max_retries=2, retry_delay_s=5)
|
||
|
||
assert mock_sleep.call_count == 2
|
||
for c in mock_sleep.call_args_list:
|
||
assert c.args[0] == 5
|
||
|
||
@patch("core.worktree.time.sleep")
|
||
@patch("subprocess.run")
|
||
def test_merge_worktree_retry_with_eventual_success_no_failure(
|
||
self, mock_run, mock_sleep, tmp_path
|
||
):
|
||
"""KIN-145: retry завершается успехом если второй attempt проходит."""
|
||
from core.worktree import merge_worktree
|
||
|
||
worktree_path = tmp_path / ".kin_worktrees" / "T-145-retry-ok"
|
||
worktree_path.mkdir(parents=True)
|
||
|
||
diff_ok = MagicMock()
|
||
diff_ok.returncode = 0
|
||
diff_ok.stdout = "src/fixed.py\n"
|
||
diff_ok.stderr = ""
|
||
|
||
mock_run.side_effect = [
|
||
_ok_run(), # git add -A
|
||
_ok_run(), # git commit
|
||
_fail_run(), # merge attempt 0: fail
|
||
_conflict_diff(), # git diff --diff-filter=U
|
||
_ok_run(), # git merge --abort
|
||
_ok_run(), # merge attempt 1: success
|
||
diff_ok, # git diff HEAD~1 HEAD --name-only
|
||
]
|
||
|
||
result = merge_worktree(str(worktree_path), str(tmp_path), max_retries=1, retry_delay_s=1)
|
||
|
||
assert result["success"] is True
|
||
assert mock_sleep.call_count == 1
|
||
|
||
|
||
# ─────────────────────────────────────────────────────────────────────────────
|
||
# D. Old buggy kwargs would have caused 45s total blocking sleep
|
||
# ─────────────────────────────────────────────────────────────────────────────
|
||
|
||
class TestOldBuggyBehaviorDocumented:
|
||
@patch("core.worktree.time.sleep")
|
||
@patch("subprocess.run")
|
||
def test_old_kwargs_max_retries_3_delay_15_sleeps_45s_total(
|
||
self, mock_run, mock_sleep, tmp_path
|
||
):
|
||
"""KIN-145 doc D: старые kwargs max_retries=3, retry_delay_s=15 = 45s суммарного blocking.
|
||
|
||
Этот тест документирует удалённое поведение.
|
||
Если этот тест когда-либо упадёт (sleep_count != 3), значит поведение изменилось.
|
||
"""
|
||
from core.worktree import merge_worktree
|
||
|
||
worktree_path = tmp_path / ".kin_worktrees" / "T-145-old-dev"
|
||
worktree_path.mkdir(parents=True)
|
||
# max_retries=3 → 4 total attempts → 3 sleeps
|
||
mock_run.side_effect = _side_effects_for_n_conflict_attempts(4)
|
||
|
||
merge_worktree(str(worktree_path), str(tmp_path), max_retries=3, retry_delay_s=15)
|
||
|
||
assert mock_sleep.call_count == 3
|
||
total_sleep_s = sum(c.args[0] for c in mock_sleep.call_args_list)
|
||
assert total_sleep_s == 45
|
||
|
||
|
||
# ─────────────────────────────────────────────────────────────────────────────
|
||
# E. Static guard: runner.py must not pass retry kwargs to merge_worktree
|
||
# ─────────────────────────────────────────────────────────────────────────────
|
||
|
||
class TestRunnerCallSiteStaticGuard:
|
||
def test_runner_merge_worktree_call_has_no_max_retries_kwarg(self):
|
||
"""KIN-145 guard E: runner.py НЕ передаёт max_retries в merge_worktree."""
|
||
runner_path = Path(__file__).parent.parent / "agents" / "runner.py"
|
||
source = runner_path.read_text()
|
||
|
||
calls = re.findall(r"merge_worktree\([^)]*\)", source, re.DOTALL)
|
||
assert calls, "merge_worktree не найден в agents/runner.py — проверь путь"
|
||
|
||
for call_src in calls:
|
||
assert "max_retries" not in call_src, (
|
||
f"runner.py вызывает merge_worktree с max_retries: {call_src!r}\n"
|
||
"Это баг KIN-145 — retry kwargs вызывают 45s blocking sleep!"
|
||
)
|
||
|
||
def test_runner_merge_worktree_call_has_no_retry_delay_kwarg(self):
|
||
"""KIN-145 guard E: runner.py НЕ передаёт retry_delay_s в merge_worktree."""
|
||
runner_path = Path(__file__).parent.parent / "agents" / "runner.py"
|
||
source = runner_path.read_text()
|
||
|
||
calls = re.findall(r"merge_worktree\([^)]*\)", source, re.DOTALL)
|
||
assert calls, "merge_worktree не найден в agents/runner.py — проверь путь"
|
||
|
||
for call_src in calls:
|
||
assert "retry_delay_s" not in call_src, (
|
||
f"runner.py вызывает merge_worktree с retry_delay_s: {call_src!r}\n"
|
||
"Это баг KIN-145 — retry kwargs вызывают 45s blocking sleep!"
|
||
)
|
||
|
||
def test_runner_merge_worktree_call_uses_exactly_two_positional_args(self):
|
||
"""KIN-145 guard E: вызов merge_worktree в runner.py имеет ровно 2 аргумента."""
|
||
runner_path = Path(__file__).parent.parent / "agents" / "runner.py"
|
||
source = runner_path.read_text()
|
||
|
||
calls = re.findall(r"merge_worktree\(([^)]*)\)", source, re.DOTALL)
|
||
assert calls, "merge_worktree не найден в agents/runner.py"
|
||
|
||
for call_args in calls:
|
||
args = [a.strip() for a in call_args.split(",") if a.strip()]
|
||
assert len(args) == 2, (
|
||
f"runner.py вызывает merge_worktree с {len(args)} аргументами: {call_args!r}\n"
|
||
f"Ожидается ровно 2 (worktree_path, p_path). "
|
||
f"Лишние kwargs — источник бага KIN-145!"
|
||
)
|