|
1 | 1 | """Unit tests for self_review.py — self-review orchestration module.""" |
2 | 2 |
|
| 3 | +import os |
3 | 4 | from unittest.mock import MagicMock, patch |
4 | 5 |
|
5 | 6 | from models import AgentResult, RepoSetup, TaskConfig |
6 | | -from self_review import _MAX_DIFF_CHARS, _get_diff, _truncate_diff, run_self_review |
| 7 | +from self_review import ( |
| 8 | + _MAX_DIFF_CHARS, |
| 9 | + _SUMMARY_FILENAME, |
| 10 | + _get_diff, |
| 11 | + _truncate_diff, |
| 12 | + read_self_review_summary, |
| 13 | + run_self_review, |
| 14 | +) |
7 | 15 |
|
8 | 16 |
|
9 | 17 | def _make_config(**overrides) -> TaskConfig: |
@@ -314,3 +322,163 @@ def test_works_with_unlimited_budget(self, mock_asyncio_run, mock_diff): |
314 | 322 | result = run_self_review(config, setup, agent_result, trajectory, progress) |
315 | 323 | assert result is not None |
316 | 324 | assert result.status == "success" |
| 325 | + |
| 326 | + |
| 327 | +class TestReadSelfReviewSummary: |
| 328 | + """Tests for read_self_review_summary — reads and cleans up the summary file.""" |
| 329 | + |
| 330 | + def test_returns_content_when_file_exists(self, tmp_path): |
| 331 | + summary_content = ( |
| 332 | + "### Self-Review Summary\n\n" |
| 333 | + "**Findings:** 2\n" |
| 334 | + "**Fixes applied:** 1\n\n" |
| 335 | + "#### Issues found\n\n" |
| 336 | + "- Security: hardcoded token — fixed\n" |
| 337 | + "- Style: inconsistent naming — not fixed (cosmetic)\n" |
| 338 | + ) |
| 339 | + (tmp_path / _SUMMARY_FILENAME).write_text(summary_content) |
| 340 | + |
| 341 | + result = read_self_review_summary(str(tmp_path)) |
| 342 | + |
| 343 | + assert result == summary_content.strip() |
| 344 | + |
| 345 | + def test_returns_none_when_file_missing(self, tmp_path): |
| 346 | + result = read_self_review_summary(str(tmp_path)) |
| 347 | + assert result is None |
| 348 | + |
| 349 | + def test_deletes_file_after_reading(self, tmp_path): |
| 350 | + (tmp_path / _SUMMARY_FILENAME).write_text("No issues found — code looks good.") |
| 351 | + |
| 352 | + read_self_review_summary(str(tmp_path)) |
| 353 | + |
| 354 | + assert not (tmp_path / _SUMMARY_FILENAME).exists() |
| 355 | + |
| 356 | + def test_returns_none_for_empty_file(self, tmp_path): |
| 357 | + (tmp_path / _SUMMARY_FILENAME).write_text(" \n\n ") |
| 358 | + |
| 359 | + result = read_self_review_summary(str(tmp_path)) |
| 360 | + |
| 361 | + assert result is None |
| 362 | + |
| 363 | + def test_returns_none_for_whitespace_only(self, tmp_path): |
| 364 | + (tmp_path / _SUMMARY_FILENAME).write_text("\t\n ") |
| 365 | + |
| 366 | + result = read_self_review_summary(str(tmp_path)) |
| 367 | + |
| 368 | + assert result is None |
| 369 | + |
| 370 | + @patch("self_review.subprocess.run") |
| 371 | + def test_runs_git_rm_cached_for_cleanup(self, mock_run, tmp_path): |
| 372 | + (tmp_path / _SUMMARY_FILENAME).write_text("Some findings") |
| 373 | + mock_run.return_value = MagicMock(returncode=0) |
| 374 | + |
| 375 | + read_self_review_summary(str(tmp_path)) |
| 376 | + |
| 377 | + mock_run.assert_called_once_with( |
| 378 | + ["git", "rm", "--cached", "--ignore-unmatch", "-f", _SUMMARY_FILENAME], |
| 379 | + cwd=str(tmp_path), |
| 380 | + capture_output=True, |
| 381 | + timeout=30, |
| 382 | + ) |
| 383 | + |
| 384 | + @patch("self_review.subprocess.run", side_effect=OSError("git not found")) |
| 385 | + def test_git_rm_failure_does_not_block(self, mock_run, tmp_path): |
| 386 | + (tmp_path / _SUMMARY_FILENAME).write_text("Findings here") |
| 387 | + |
| 388 | + # Should not raise even if git rm fails |
| 389 | + result = read_self_review_summary(str(tmp_path)) |
| 390 | + |
| 391 | + assert result == "Findings here" |
| 392 | + |
| 393 | + |
| 394 | +class TestPostSelfReviewComment: |
| 395 | + """Tests for post_hooks.post_self_review_comment.""" |
| 396 | + |
| 397 | + def test_posts_comment_on_success(self, tmp_path): |
| 398 | + from post_hooks import post_self_review_comment |
| 399 | + |
| 400 | + (tmp_path / _SUMMARY_FILENAME).write_text("**Findings:** 1\n**Fixes applied:** 1") |
| 401 | + |
| 402 | + config = MagicMock() |
| 403 | + config.repo_url = "owner/repo" |
| 404 | + pr_url = "https://github.com/owner/repo/pull/42" |
| 405 | + |
| 406 | + with patch("post_hooks.subprocess.run") as mock_run: |
| 407 | + mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="") |
| 408 | + result = post_self_review_comment(str(tmp_path), pr_url, config) |
| 409 | + |
| 410 | + assert result is True |
| 411 | + call_args = mock_run.call_args[0][0] |
| 412 | + assert call_args[0:3] == ["gh", "pr", "comment"] |
| 413 | + assert "42" in call_args |
| 414 | + assert "owner/repo" in call_args |
| 415 | + |
| 416 | + def test_returns_false_when_no_summary(self, tmp_path): |
| 417 | + from post_hooks import post_self_review_comment |
| 418 | + |
| 419 | + config = MagicMock() |
| 420 | + config.repo_url = "owner/repo" |
| 421 | + pr_url = "https://github.com/owner/repo/pull/42" |
| 422 | + |
| 423 | + result = post_self_review_comment(str(tmp_path), pr_url, config) |
| 424 | + assert result is False |
| 425 | + |
| 426 | + def test_returns_false_on_invalid_pr_url(self, tmp_path): |
| 427 | + from post_hooks import post_self_review_comment |
| 428 | + |
| 429 | + (tmp_path / _SUMMARY_FILENAME).write_text("Some findings") |
| 430 | + |
| 431 | + config = MagicMock() |
| 432 | + config.repo_url = "owner/repo" |
| 433 | + pr_url = "https://github.com/owner/repo/issues/42" |
| 434 | + |
| 435 | + result = post_self_review_comment(str(tmp_path), pr_url, config) |
| 436 | + assert result is False |
| 437 | + |
| 438 | + def test_returns_false_on_gh_failure(self, tmp_path): |
| 439 | + from post_hooks import post_self_review_comment |
| 440 | + |
| 441 | + (tmp_path / _SUMMARY_FILENAME).write_text("**Findings:** 1") |
| 442 | + |
| 443 | + config = MagicMock() |
| 444 | + config.repo_url = "owner/repo" |
| 445 | + pr_url = "https://github.com/owner/repo/pull/99" |
| 446 | + |
| 447 | + with patch("post_hooks.subprocess.run") as mock_run: |
| 448 | + mock_run.return_value = MagicMock(returncode=1, stdout="", stderr="not found") |
| 449 | + result = post_self_review_comment(str(tmp_path), pr_url, config) |
| 450 | + |
| 451 | + assert result is False |
| 452 | + |
| 453 | + def test_fail_open_on_exception(self, tmp_path): |
| 454 | + from post_hooks import post_self_review_comment |
| 455 | + |
| 456 | + (tmp_path / _SUMMARY_FILENAME).write_text("**Findings:** 1") |
| 457 | + |
| 458 | + config = MagicMock() |
| 459 | + config.repo_url = "owner/repo" |
| 460 | + pr_url = "https://github.com/owner/repo/pull/5" |
| 461 | + |
| 462 | + with patch("post_hooks.subprocess.run", side_effect=OSError("network error")): |
| 463 | + result = post_self_review_comment(str(tmp_path), pr_url, config) |
| 464 | + |
| 465 | + assert result is False |
| 466 | + |
| 467 | + def test_comment_body_includes_header(self, tmp_path): |
| 468 | + from post_hooks import post_self_review_comment |
| 469 | + |
| 470 | + (tmp_path / _SUMMARY_FILENAME).write_text("**Findings:** 0\nNo issues.") |
| 471 | + |
| 472 | + config = MagicMock() |
| 473 | + config.repo_url = "owner/repo" |
| 474 | + pr_url = "https://github.com/owner/repo/pull/7" |
| 475 | + |
| 476 | + with patch("post_hooks.subprocess.run") as mock_run: |
| 477 | + mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="") |
| 478 | + post_self_review_comment(str(tmp_path), pr_url, config) |
| 479 | + |
| 480 | + call_args = mock_run.call_args[0][0] |
| 481 | + body_idx = call_args.index("--body") + 1 |
| 482 | + body = call_args[body_idx] |
| 483 | + assert "Self-Review Summary" in body |
| 484 | + assert "**Findings:** 0" in body |
0 commit comments