ed
dc397db7ed
TIER-3 READ AGENTS.md + conductor/workflow.md + conductor/code_styleguides/error_handling.md + the 4 source files + 3 test files before this commit. The code_path_audit_phase_2_20260624 track (Tier 2) shipped 11 audit fixes (4 NG1 + 7 NG2) but used a heuristic bypass for 4 of the NG2 wrappers: legacy T | None functions that exist only to maintain test patcher compatibility. Per the review at docs/reports/REVIEW_TIER2_code_path_audit_phase_2_20260624.md Finding 8, this track eliminates the legacy wrappers properly. 11 wrappers eliminated (8 main + 3 _legacy_compat inner): - src/ai_client.py: get_current_tier (1 src + 1 test consumer) - src/ai_client.py: _gemini_tool_declaration + _legacy_compat (2 test consumers) - src/ai_client.py: run_tier4_patch_callback + _legacy_compat (was 0 direct callers but had 2 callback references in app_controller/multi_agent_conductor; callback contract migrated to Callable[[str, str], Result[str]] instead of preserving an Optional[str] adapter) - src/mcp_client.py: _get_symbol_node + _legacy_compat (8 in-file consumers) - src/mcp_client.py: find_in_scope (nested inside _get_symbol_node_result; private impl detail, audit doesn't catch T | None, left as-is) - src/external_editor.py: launch_diff (1 src + 3 test + 1 live_gui test consumer) - src/external_editor.py: launch_editor (no consumers; deleted) - src/session_logger.py: log_tool_output (2 src + 3 test consumers) - src/project_manager.py: parse_ts (no consumers; deleted) For each consumer: replace legacy_fn(args) with legacy_fn_result(args).data. For T | None checks: replace if x is None: with if not result.ok: or if not result.ok or not isinstance(result.data, ...) (depending on pattern). For run_tier4_patch_callback specifically: the wrapper was a callback adapter (not a backward-compat shim) and had 2 callback references as consumers. Rather than keep the adapter (which would re-introduce the Optional[str] return that the strict audit catches), the patch_callback contract was migrated from Callable[[str, str], Optional[str]] to Callable[[str, str], Result[str]] in shell_runner.py + app_controller.py + 9 _send_<vendor>_result signatures in ai_client.py. This propagates the Result[str] through the callback and lets shell_runner unwrap with if r.ok and r.data instead of if patch_text. Verification: - audit_optional_in_3_files --strict: 0 return-type Optional[T] (down from 1) - audit_exception_handling --strict: 0 violations (unchanged) - audit_legacy_wrappers: 0 legacy wrappers (unchanged) - 15 affected test files: 168 tests pass - 8 mcp_client/structural/baseline test files: 55 tests pass - 3 session/gui test files: 7 tests pass - 0 return-type Optional[T] in src/ai_client.py (was 1: run_tier4_patch_callback)
133 lines
4.3 KiB
Python
133 lines
4.3 KiB
Python
"""Tests for external editor integration."""
|
|
import pytest
|
|
from unittest.mock import patch, MagicMock
|
|
from src.models import TextEditorConfig, ExternalEditorConfig
|
|
from src.external_editor import (
|
|
ExternalEditorLauncher,
|
|
get_default_launcher,
|
|
create_temp_modified_file,
|
|
)
|
|
|
|
|
|
@pytest.fixture
|
|
def vscode_editor():
|
|
return TextEditorConfig(name="vscode", path="C:\\path\\to\\code.exe", diff_args=["--diff"])
|
|
|
|
|
|
@pytest.fixture
|
|
def notepadpp_editor():
|
|
return TextEditorConfig(name="notepad++", path="C:\\path\\to\\notepad++.exe", diff_args=["-multiInst", "-nosession"])
|
|
|
|
|
|
@pytest.fixture
|
|
def ext_config(vscode_editor, notepadpp_editor):
|
|
return ExternalEditorConfig(
|
|
editors={"vscode": vscode_editor, "notepad++": notepadpp_editor},
|
|
default_editor="vscode",
|
|
)
|
|
|
|
|
|
@pytest.fixture
|
|
def launcher(ext_config):
|
|
return ExternalEditorLauncher(ext_config)
|
|
|
|
|
|
class TestTextEditorConfig:
|
|
def test_from_dict_with_diff_args(self):
|
|
data = {"name": "vscode", "path": "C:\\code.exe", "diff_args": ["--diff"]}
|
|
editor = TextEditorConfig.from_dict(data)
|
|
assert editor.name == "vscode"
|
|
assert editor.path == "C:\\code.exe"
|
|
assert editor.diff_args == ["--diff"]
|
|
|
|
def test_from_dict_without_diff_args(self):
|
|
data = {"name": "vscode", "path": "C:\\code.exe"}
|
|
editor = TextEditorConfig.from_dict(data)
|
|
assert editor.diff_args == []
|
|
|
|
def test_to_dict(self, vscode_editor):
|
|
result = vscode_editor.to_dict()
|
|
assert result["name"] == "vscode"
|
|
assert result["path"] == "C:\\path\\to\\code.exe"
|
|
assert result["diff_args"] == ["--diff"]
|
|
|
|
|
|
class TestExternalEditorConfig:
|
|
def test_from_dict_with_string_editors(self):
|
|
data = {"editors": {"vscode": "C:\\code.exe"}, "default_editor": "vscode"}
|
|
config = ExternalEditorConfig.from_dict(data)
|
|
assert "vscode" in config.editors
|
|
assert config.editors["vscode"].path == "C:\\code.exe"
|
|
|
|
def test_from_dict_with_dict_editors(self, vscode_editor):
|
|
data = {"editors": {"vscode": {"name": "vscode", "path": "C:\\code.exe", "diff_args": ["--diff"]}}}
|
|
config = ExternalEditorConfig.from_dict(data)
|
|
assert config.editors["vscode"].diff_args == ["--diff"]
|
|
|
|
def test_get_default_returns_configured(self, ext_config):
|
|
result = ext_config.get_default()
|
|
assert result.name == "vscode"
|
|
|
|
def test_get_default_fallback_to_first(self):
|
|
config = ExternalEditorConfig(editors={"notepad++": TextEditorConfig(name="notepad++", path="C:\\npp.exe")})
|
|
result = config.get_default()
|
|
assert result.name == "notepad++"
|
|
|
|
def test_get_default_returns_none_when_empty(self):
|
|
config = ExternalEditorConfig(editors={})
|
|
assert config.get_default() is None
|
|
|
|
def test_to_dict(self, ext_config):
|
|
result = ext_config.to_dict()
|
|
assert result["default_editor"] == "vscode"
|
|
assert "vscode" in result["editors"]
|
|
|
|
|
|
class TestExternalEditorLauncher:
|
|
def test_get_editor_by_name(self, launcher):
|
|
editor = launcher.get_editor("notepad++")
|
|
assert editor.name == "notepad++"
|
|
|
|
def test_get_editor_returns_default(self, launcher):
|
|
editor = launcher.get_editor()
|
|
assert editor.name == "vscode"
|
|
|
|
def test_get_editor_unknown_name(self, launcher):
|
|
editor = launcher.get_editor("unknown")
|
|
assert editor is None
|
|
|
|
def test_build_diff_command(self, launcher, vscode_editor):
|
|
cmd = launcher.build_diff_command(vscode_editor, "orig.txt", "mod.txt")
|
|
assert cmd == ["C:\\path\\to\\code.exe", "--diff", "orig.txt", "mod.txt"]
|
|
|
|
def test_launch_diff_missing_editor(self, launcher):
|
|
result = launcher.launch_diff_result("nonexistent", "orig.txt", "mod.txt")
|
|
assert not result.ok
|
|
assert result.data is None
|
|
|
|
@patch("subprocess.Popen")
|
|
def test_launch_diff_success(self, mock_popen, launcher):
|
|
mock_popen.return_value = MagicMock()
|
|
result = launcher.launch_diff_result("vscode", "orig.txt", "mod.txt")
|
|
assert result.ok
|
|
assert result.data is not None
|
|
mock_popen.assert_called_once()
|
|
|
|
@patch("subprocess.Popen")
|
|
def test_launch_diff_file_not_found(self, mock_popen, launcher):
|
|
mock_popen.side_effect = FileNotFoundError()
|
|
result = launcher.launch_diff_result("vscode", "orig.txt", "mod.txt")
|
|
assert not result.ok
|
|
assert result.data is None
|
|
|
|
|
|
class TestHelperFunctions:
|
|
def test_create_temp_modified_file(self):
|
|
content = "test content"
|
|
path = create_temp_modified_file(content)
|
|
assert path.endswith("_modified")
|
|
with open(path, encoding="utf-8") as f:
|
|
assert f.read() == content
|
|
import os
|
|
os.unlink(path)
|