-
Notifications
You must be signed in to change notification settings - Fork 5
feat(adapters): detect modified files via git diff after subprocess execution #436
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,12 @@ | |
| class TestOpenCodeAdapter: | ||
| """Unit tests for OpenCodeAdapter.""" | ||
|
|
||
| @pytest.fixture(autouse=True) | ||
| def _no_git(self): | ||
| """Prevent _detect_modified_files from calling real git.""" | ||
| with patch.object(OpenCodeAdapter, "_detect_modified_files", return_value=[]): | ||
| yield | ||
|
|
||
|
Comment on lines
+15
to
+20
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a v2 marker for this test module. The new v2-related adapter test behavior added around Line 15 is in a test module that is not marked with Suggested patch import pytest
from codeframe.core.adapters.agent_adapter import AgentAdapter
from codeframe.core.adapters.opencode import OpenCodeAdapter
+pytestmark = pytest.mark.v2
+
class TestOpenCodeAdapter:As per coding guidelines: 🤖 Prompt for AI Agents |
||
| def test_name(self) -> None: | ||
| with patch("shutil.which", return_value="/usr/bin/opencode"): | ||
| adapter = OpenCodeAdapter() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,6 +42,12 @@ def test_init_stores_resolved_path(self): | |
| class TestSubprocessAdapterRun: | ||
| """Tests for subprocess execution.""" | ||
|
|
||
| @pytest.fixture(autouse=True) | ||
| def _no_git(self): | ||
| """Prevent _detect_modified_files from calling real git.""" | ||
| with patch.object(SubprocessAdapter, "_detect_modified_files", return_value=[]): | ||
| yield | ||
|
|
||
| @pytest.fixture | ||
| def adapter(self): | ||
| with patch("shutil.which", return_value="/usr/bin/test-agent"): | ||
|
|
@@ -222,6 +228,119 @@ def test_default_returns_empty_prompt(self): | |
| assert adapter.get_stdin("") == "" | ||
|
|
||
|
|
||
| class TestSubprocessAdapterModifiedFiles: | ||
| """Tests for git diff file detection after execution.""" | ||
|
|
||
|
Comment on lines
+231
to
+233
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mark the newly added v2 tests with The new test class introduced at Line 231 is not marked as v2. Suggested patch+@pytest.mark.v2
class TestSubprocessAdapterModifiedFiles:
"""Tests for git diff file detection after execution."""As per coding guidelines: 🤖 Prompt for AI Agents |
||
| @pytest.fixture | ||
| def adapter(self): | ||
| with patch("shutil.which", return_value="/usr/bin/test-agent"): | ||
| return SubprocessAdapter("test-agent") | ||
|
|
||
| def _make_mock_process( | ||
| self, stdout_lines=None, stderr_text="", returncode=0 | ||
| ): | ||
| mock = MagicMock() | ||
| mock.stdout = iter(stdout_lines or []) | ||
| mock.stderr = MagicMock() | ||
| mock.stderr.read.return_value = stderr_text | ||
| mock.stdin = MagicMock() | ||
| mock.returncode = returncode | ||
| mock.wait.return_value = None | ||
| return mock | ||
|
|
||
| def test_populates_modified_files_on_success(self, adapter, tmp_path): | ||
| """After successful execution, modified_files should list changed files.""" | ||
| mock_process = self._make_mock_process( | ||
| stdout_lines=["done\n"], returncode=0 | ||
| ) | ||
| with ( | ||
| patch("subprocess.Popen", return_value=mock_process), | ||
| patch( | ||
| "subprocess.run", | ||
| side_effect=[ | ||
| MagicMock(returncode=0, stdout="src/main.py\ntests/test_main.py\n"), | ||
| MagicMock(returncode=0, stdout=""), # no untracked files | ||
| ], | ||
| ), | ||
| ): | ||
| result = adapter.run("task-1", "fix", tmp_path) | ||
|
|
||
| assert result.status == "completed" | ||
| assert result.modified_files == ["src/main.py", "tests/test_main.py"] | ||
|
|
||
| def test_empty_modified_files_when_no_changes(self, adapter, tmp_path): | ||
| mock_process = self._make_mock_process( | ||
| stdout_lines=["done\n"], returncode=0 | ||
| ) | ||
| with ( | ||
| patch("subprocess.Popen", return_value=mock_process), | ||
| patch( | ||
| "subprocess.run", | ||
| side_effect=[ | ||
| MagicMock(returncode=0, stdout=""), | ||
| MagicMock(returncode=0, stdout=""), | ||
| ], | ||
| ), | ||
| ): | ||
| result = adapter.run("task-1", "fix", tmp_path) | ||
|
|
||
| assert result.modified_files == [] | ||
|
|
||
| def test_detects_files_even_on_failure(self, adapter, tmp_path): | ||
| """Failed execution should still detect modified files.""" | ||
| mock_process = self._make_mock_process( | ||
| stderr_text="error", returncode=1 | ||
| ) | ||
| with ( | ||
| patch("subprocess.Popen", return_value=mock_process), | ||
| patch( | ||
| "subprocess.run", | ||
| side_effect=[ | ||
| MagicMock(returncode=0, stdout="src/broken.py\n"), | ||
| MagicMock(returncode=0, stdout=""), | ||
| ], | ||
| ), | ||
| ): | ||
| result = adapter.run("task-1", "fix", tmp_path) | ||
|
|
||
| assert result.status == "failed" | ||
| assert "src/broken.py" in result.modified_files | ||
|
|
||
| def test_graceful_when_not_git_repo(self, adapter, tmp_path): | ||
| """Should return empty modified_files if git is unavailable.""" | ||
| mock_process = self._make_mock_process( | ||
| stdout_lines=["done\n"], returncode=0 | ||
| ) | ||
| with ( | ||
| patch("subprocess.Popen", return_value=mock_process), | ||
| patch( | ||
| "subprocess.run", | ||
| side_effect=FileNotFoundError("git not found"), | ||
| ), | ||
| ): | ||
| result = adapter.run("task-1", "fix", tmp_path) | ||
|
|
||
| assert result.status == "completed" | ||
| assert result.modified_files == [] | ||
|
|
||
| def test_graceful_when_git_fails(self, adapter, tmp_path): | ||
| """Should return empty modified_files if git diff fails.""" | ||
| mock_process = self._make_mock_process( | ||
| stdout_lines=["done\n"], returncode=0 | ||
| ) | ||
| with ( | ||
| patch("subprocess.Popen", return_value=mock_process), | ||
| patch( | ||
| "subprocess.run", | ||
| return_value=MagicMock(returncode=128, stdout=""), | ||
| ), | ||
| ): | ||
| result = adapter.run("task-1", "fix", tmp_path) | ||
|
|
||
| assert result.status == "completed" | ||
| assert result.modified_files == [] | ||
|
|
||
|
|
||
| class TestSubprocessAdapterBlockerExtraction: | ||
| """Tests for blocker question extraction.""" | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a v2 marker for this test module.
The v2 adapter test updates introduced around Line 15 are in a module without
@pytest.mark.v2(or module-levelpytestmark).Suggested patch
As per coding guidelines:
tests/**/*.py: Test files must use the@pytest.mark.v2decorator or module-levelpytestmark = pytest.mark.v2for v2 functionality tests.📝 Committable suggestion
🤖 Prompt for AI Agents