Skip to content

Commit ca4ebd1

Browse files
Add focused unit tests for core PR-Agent flows
Cover routing, PR processing, reviewer labels, code suggestions, GitHub Action behavior, webhook helpers, GitLab URL handling, LiteLLM request shaping, provider configuration errors, and PR description helpers. These tests target behavior that is important and likely to change, while keeping production code untouched. This work was developed through human-led AI collaboration using codex-cli v0.128.0 with GPT-5.5, GitHub Copilot CLI 1.0.40 with GPT-5.5, and Claude Opus 4.7.
1 parent 009ba5a commit ca4ebd1

10 files changed

Lines changed: 1268 additions & 3 deletions
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
from pr_agent.git_providers.utils import handle_configurations_errors
2+
3+
4+
class FakeMarkdownProvider:
5+
def __init__(self):
6+
self.persistent_comments = []
7+
8+
def is_supported(self, capability):
9+
return capability == "gfm_markdown"
10+
11+
def publish_persistent_comment(self, body, initial_header, update_header, final_update_message):
12+
self.persistent_comments.append({
13+
"body": body,
14+
"initial_header": initial_header,
15+
"update_header": update_header,
16+
"final_update_message": final_update_message,
17+
})
18+
19+
20+
class FakePlainProvider:
21+
def __init__(self):
22+
self.comments = []
23+
24+
def is_supported(self, capability):
25+
return False
26+
27+
def publish_comment(self, body):
28+
self.comments.append(body)
29+
30+
31+
class FakeMarkdownCommentProvider(FakePlainProvider):
32+
def is_supported(self, capability):
33+
return capability == "gfm_markdown"
34+
35+
36+
def test_handle_configurations_errors_uses_persistent_comment_when_supported():
37+
provider = FakeMarkdownProvider()
38+
39+
handle_configurations_errors([{
40+
"settings": b"[config]\nmodel =",
41+
"error": "Invalid value",
42+
"category": "local",
43+
}], provider)
44+
45+
assert len(provider.persistent_comments) == 1
46+
comment = provider.persistent_comments[0]
47+
assert comment["initial_header"] == "❌ **PR-Agent failed to apply 'local' repo settings**"
48+
assert comment["update_header"] is False
49+
assert comment["final_update_message"] is False
50+
assert "PR-Agent failed to apply 'local' repo settings" in comment["body"]
51+
assert "Invalid value" in comment["body"]
52+
assert "```toml\n[config]\nmodel =\n```" in comment["body"]
53+
assert "<details><summary>Configuration content:</summary>" in comment["body"]
54+
55+
56+
def test_handle_configurations_errors_keeps_markdown_details_when_persistent_comment_is_missing():
57+
provider = FakeMarkdownCommentProvider()
58+
59+
handle_configurations_errors([{
60+
"settings": b"[config]\nmodel =",
61+
"error": "Invalid value",
62+
"category": "local",
63+
}], provider)
64+
65+
assert len(provider.comments) == 1
66+
assert "PR-Agent failed to apply 'local' repo settings" in provider.comments[0]
67+
assert "Invalid value" in provider.comments[0]
68+
assert "```toml\n[config]\nmodel =\n```" in provider.comments[0]
69+
assert "<details><summary>Configuration content:</summary>" in provider.comments[0]
70+
71+
72+
def test_handle_configurations_errors_uses_plain_comment_without_markdown_support():
73+
provider = FakePlainProvider()
74+
75+
handle_configurations_errors([{
76+
"settings": b"[config]\nmodel =",
77+
"error": "Invalid value",
78+
"category": "local",
79+
}], provider)
80+
81+
assert len(provider.comments) == 1
82+
assert "❌ **PR-Agent failed to apply 'local' repo settings**" in provider.comments[0]
83+
assert "Invalid value" in provider.comments[0]
84+
assert "```toml\n[config]\nmodel =\n```" in provider.comments[0]
85+
assert "<details>" not in provider.comments[0]
86+
87+
88+
def test_handle_configurations_errors_returns_without_errors():
89+
provider = FakePlainProvider()
90+
91+
handle_configurations_errors([], provider)
92+
93+
assert provider.comments == []
94+
95+
96+
def test_handle_configurations_errors_publishes_each_error():
97+
provider = FakePlainProvider()
98+
99+
handle_configurations_errors([
100+
{
101+
"settings": b"[config]\nmodel =",
102+
"error": "First error",
103+
"category": "local",
104+
},
105+
{
106+
"settings": b"[pr_reviewer]\nnum_max_findings =",
107+
"error": "Second error",
108+
"category": "global",
109+
},
110+
], provider)
111+
112+
assert len(provider.comments) == 2
113+
assert "First error" in provider.comments[0]
114+
assert "[config]\nmodel =" in provider.comments[0]
115+
assert "Second error" in provider.comments[1]
116+
assert "[pr_reviewer]\nnum_max_findings =" in provider.comments[1]
117+
118+
119+
def test_handle_configurations_errors_ignores_empty_sentinel_entry():
120+
provider = FakePlainProvider()
121+
122+
handle_configurations_errors([None], provider)
123+
124+
assert provider.comments == []
125+
126+
127+
def test_handle_configurations_errors_skips_empty_sentinel_entries_in_mixed_list():
128+
provider = FakePlainProvider()
129+
130+
handle_configurations_errors([
131+
None,
132+
{
133+
"settings": b"[config]\nmodel =",
134+
"error": "Only error",
135+
"category": "local",
136+
},
137+
], provider)
138+
139+
assert len(provider.comments) == 1
140+
assert "Only error" in provider.comments[0]
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
import copy
2+
import json
3+
4+
import pytest
5+
6+
import pr_agent.servers.github_action_runner as github_action_runner
7+
from pr_agent.config_loader import get_settings
8+
9+
10+
def test_is_true_accepts_bool_and_case_insensitive_true_string():
11+
assert github_action_runner.is_true(True) is True
12+
assert github_action_runner.is_true(False) is False
13+
assert github_action_runner.is_true("TRUE") is True
14+
assert github_action_runner.is_true("false") is False
15+
assert github_action_runner.is_true(None) is False
16+
17+
18+
@pytest.mark.asyncio
19+
async def test_run_action_returns_when_required_env_is_missing(monkeypatch, capsys):
20+
monkeypatch.delenv("GITHUB_EVENT_NAME", raising=False)
21+
22+
await github_action_runner.run_action()
23+
24+
assert "GITHUB_EVENT_NAME not set" in capsys.readouterr().out
25+
26+
27+
@pytest.mark.asyncio
28+
async def test_run_action_invokes_enabled_auto_tools_for_pull_request_event(monkeypatch, tmp_path):
29+
settings = get_settings()
30+
original_is_auto_command = settings.config.get("is_auto_command", False)
31+
original_final_update_message = settings.pr_description.final_update_message
32+
original_response_language = settings.config.response_language
33+
had_github_settings = "GITHUB" in settings
34+
original_github_settings = copy.deepcopy(settings.get("GITHUB", None))
35+
had_github_action_config = "GITHUB_ACTION_CONFIG" in settings
36+
original_github_action_config = copy.deepcopy(settings.get("GITHUB_ACTION_CONFIG", None))
37+
event_path = tmp_path / "event.json"
38+
event_path.write_text(json.dumps({
39+
"action": "opened",
40+
"pull_request": {
41+
"url": "https://api.github.com/repos/org/repo/pulls/1",
42+
"html_url": "https://github.com/org/repo/pull/1",
43+
},
44+
}))
45+
monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request")
46+
monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_path))
47+
monkeypatch.setenv("GITHUB_TOKEN", "token")
48+
monkeypatch.setattr(github_action_runner, "apply_repo_settings", lambda pr_url: None)
49+
50+
def fake_get_setting_or_env(key, default=None):
51+
values = {
52+
"GITHUB_ACTION_CONFIG.PR_ACTIONS": ["opened"],
53+
"GITHUB_ACTION.AUTO_DESCRIBE": True,
54+
"GITHUB_ACTION.AUTO_REVIEW": False,
55+
"GITHUB_ACTION.AUTO_IMPROVE": True,
56+
"GITHUB_ACTION_CONFIG.ENABLE_OUTPUT": True,
57+
}
58+
return values.get(key, default)
59+
60+
monkeypatch.setattr(github_action_runner, "get_setting_or_env", fake_get_setting_or_env)
61+
runs = []
62+
63+
class FakeTool:
64+
name = "base"
65+
66+
def __init__(self, pr_url):
67+
self.pr_url = pr_url
68+
69+
async def run(self):
70+
runs.append((self.name, self.pr_url))
71+
72+
class FakeDescription(FakeTool):
73+
name = "describe"
74+
75+
class FakeReviewer(FakeTool):
76+
name = "review"
77+
78+
class FakeSuggestions(FakeTool):
79+
name = "improve"
80+
81+
monkeypatch.setattr(github_action_runner, "PRDescription", FakeDescription)
82+
monkeypatch.setattr(github_action_runner, "PRReviewer", FakeReviewer)
83+
monkeypatch.setattr(github_action_runner, "PRCodeSuggestions", FakeSuggestions)
84+
85+
try:
86+
settings.config.response_language = "en-us"
87+
88+
await github_action_runner.run_action()
89+
90+
assert runs == [
91+
("describe", "https://api.github.com/repos/org/repo/pulls/1"),
92+
("improve", "https://api.github.com/repos/org/repo/pulls/1"),
93+
]
94+
finally:
95+
settings.config.is_auto_command = original_is_auto_command
96+
settings.pr_description.final_update_message = original_final_update_message
97+
settings.config.response_language = original_response_language
98+
if had_github_settings:
99+
settings.set("GITHUB", original_github_settings)
100+
else:
101+
settings.unset("GITHUB", force=True)
102+
if had_github_action_config:
103+
settings.set("GITHUB_ACTION_CONFIG", original_github_action_config)
104+
else:
105+
settings.unset("GITHUB_ACTION_CONFIG", force=True)

tests/unittest/test_gitlab_provider.py

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ def test_create_or_update_pr_file_create_new(self, gitlab_provider, mock_project
9595

9696
def test_create_or_update_pr_file_update_existing(self, gitlab_provider, mock_project):
9797
mock_file = MagicMock(ProjectFile)
98-
mock_file.decode.return_value = "# Old changelog content"
98+
mock_file.content = "# Old changelog content"
9999
mock_project.files.get.return_value = mock_file
100100

101101
new_content = "# New changelog content"
@@ -106,8 +106,9 @@ def test_create_or_update_pr_file_update_existing(self, gitlab_provider, mock_pr
106106
)
107107

108108
mock_project.files.get.assert_called_once_with("CHANGELOG.md", "feature-branch")
109-
mock_file.content = new_content
109+
assert mock_file.content == new_content
110110
mock_file.save.assert_called_once_with(branch="feature-branch", commit_message=commit_message)
111+
mock_project.files.create.assert_not_called()
111112

112113
def test_create_or_update_pr_file_update_exception(self, gitlab_provider, mock_project):
113114
mock_project.files.get.side_effect = Exception("Network error")
@@ -174,13 +175,18 @@ def test_project_by_path_requires_exact_match(self, gitlab_provider):
174175
gitlab_provider.gl.projects.get.reset_mock()
175176
gitlab_provider.gl.projects.get.side_effect = Exception("not found")
176177
fake = MagicMock()
178+
fake.id = "mismatched-project-id"
177179
fake.path_with_namespace = "other/group/repo"
178180
gitlab_provider.gl.projects.list.return_value = [fake]
179181

180182
result = gitlab_provider._project_by_path("group/repo")
181183

182184
assert result is None
183-
assert gitlab_provider.gl.projects.get.call_count == 2
185+
gitlab_provider.gl.projects.list.assert_called_once()
186+
list_kwargs = gitlab_provider.gl.projects.list.call_args.kwargs
187+
assert list_kwargs["search"] == "repo"
188+
assert list_kwargs["membership"] is True
189+
assert all(call.args[0] != fake.id for call in gitlab_provider.gl.projects.get.call_args_list)
184190

185191
def test_compare_submodule_cached(self, gitlab_provider):
186192
proj = MagicMock()
@@ -192,3 +198,37 @@ def test_compare_submodule_cached(self, gitlab_provider):
192198
assert first == second == [{"diff": "d"}]
193199
m_pbp.assert_called_once_with("grp/repo")
194200
proj.repository_compare.assert_called_once_with("old", "new")
201+
202+
def test_compare_submodule_cache_hit_skips_project_resolution(self, gitlab_provider):
203+
cached_diffs = [{"diff": "d"}]
204+
gitlab_provider._submodule_cache[("grp/repo", "old", "new")] = cached_diffs
205+
206+
with patch.object(gitlab_provider, "_project_by_path") as m_pbp:
207+
result = gitlab_provider._compare_submodule("grp/repo", "old", "new")
208+
209+
assert result == cached_diffs
210+
m_pbp.assert_not_called()
211+
212+
def test_parse_merge_request_url_handles_nested_project_paths(self, gitlab_provider):
213+
project_path, mr_id = gitlab_provider._parse_merge_request_url(
214+
"https://gitlab.com/group/subgroup/repo/-/merge_requests/123"
215+
)
216+
217+
assert project_path == "group/subgroup/repo"
218+
assert mr_id == 123
219+
220+
def test_get_line_link_handles_file_and_line_ranges(self, gitlab_provider):
221+
gitlab_provider.gl.url = "https://gitlab.com"
222+
gitlab_provider.id_project = "group/repo"
223+
gitlab_provider.mr = MagicMock()
224+
gitlab_provider.mr.source_branch = "feature/cache"
225+
226+
assert gitlab_provider.get_line_link("src/app.py", -1) == (
227+
"https://gitlab.com/group/repo/-/blob/feature/cache/src/app.py?ref_type=heads"
228+
)
229+
assert gitlab_provider.get_line_link("src/app.py", 10) == (
230+
"https://gitlab.com/group/repo/-/blob/feature/cache/src/app.py?ref_type=heads#L10"
231+
)
232+
assert gitlab_provider.get_line_link("src/app.py", 10, 12) == (
233+
"https://gitlab.com/group/repo/-/blob/feature/cache/src/app.py?ref_type=heads#L10-12"
234+
)

0 commit comments

Comments
 (0)