mirror of
https://github.com/OpenHands/OpenHands.git
synced 2025-12-26 05:48:36 +08:00
[Fix]: Fix bugs for target_branch param on resolver (#5745)
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
parent
ad2237d7dd
commit
a1f1c802d9
@ -307,7 +307,6 @@ async def resolve_issue(
|
||||
repo_instruction: str | None,
|
||||
issue_number: int,
|
||||
comment_id: int | None,
|
||||
target_branch: str | None = None,
|
||||
reset_logger: bool = False,
|
||||
) -> None:
|
||||
"""Resolve a single github issue.
|
||||
@ -326,7 +325,7 @@ async def resolve_issue(
|
||||
repo_instruction: Repository instruction to use.
|
||||
issue_number: Issue number to resolve.
|
||||
comment_id: Optional ID of a specific comment to focus on.
|
||||
target_branch: Optional target branch to create PR against (for PRs).
|
||||
|
||||
reset_logger: Whether to reset the logger for multiprocessing.
|
||||
"""
|
||||
issue_handler = issue_handler_factory(issue_type, owner, repo, token, llm_config)
|
||||
@ -424,9 +423,9 @@ async def resolve_issue(
|
||||
try:
|
||||
# checkout to pr branch if needed
|
||||
if issue_type == 'pr':
|
||||
branch_to_use = target_branch if target_branch else issue.head_branch
|
||||
branch_to_use = issue.head_branch
|
||||
logger.info(
|
||||
f'Checking out to PR branch {target_branch} for issue {issue.number}'
|
||||
f'Checking out to PR branch {branch_to_use} for issue {issue.number}'
|
||||
)
|
||||
|
||||
if not branch_to_use:
|
||||
@ -446,10 +445,6 @@ async def resolve_issue(
|
||||
cwd=repo_dir,
|
||||
)
|
||||
|
||||
# Update issue's base_branch if using custom target branch
|
||||
if target_branch:
|
||||
issue.base_branch = target_branch
|
||||
|
||||
base_commit = (
|
||||
subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=repo_dir)
|
||||
.decode('utf-8')
|
||||
@ -644,7 +639,6 @@ def main():
|
||||
repo_instruction=repo_instruction,
|
||||
issue_number=my_args.issue_number,
|
||||
comment_id=my_args.comment_id,
|
||||
target_branch=my_args.target_branch,
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
@ -500,6 +500,111 @@ def test_send_pull_request_with_reviewer(
|
||||
assert result == 'https://github.com/test-owner/test-repo/pull/1'
|
||||
|
||||
|
||||
@patch('subprocess.run')
|
||||
@patch('requests.post')
|
||||
@patch('requests.get')
|
||||
def test_send_pull_request_target_branch_with_fork(
|
||||
mock_get, mock_post, mock_run, mock_github_issue, mock_output_dir
|
||||
):
|
||||
"""Test that target_branch works correctly when using a fork."""
|
||||
repo_path = os.path.join(mock_output_dir, 'repo')
|
||||
fork_owner = 'fork-owner'
|
||||
target_branch = 'custom-target'
|
||||
|
||||
# Mock API responses
|
||||
mock_get.side_effect = [
|
||||
MagicMock(status_code=404), # Branch doesn't exist
|
||||
MagicMock(status_code=200), # Target branch exists
|
||||
]
|
||||
|
||||
mock_post.return_value.json.return_value = {
|
||||
'html_url': 'https://github.com/test-owner/test-repo/pull/1'
|
||||
}
|
||||
|
||||
# Mock subprocess.run calls
|
||||
mock_run.side_effect = [
|
||||
MagicMock(returncode=0), # git checkout -b
|
||||
MagicMock(returncode=0), # git push
|
||||
]
|
||||
|
||||
# Call the function with fork_owner and target_branch
|
||||
result = send_pull_request(
|
||||
github_issue=mock_github_issue,
|
||||
github_token='test-token',
|
||||
github_username='test-user',
|
||||
patch_dir=repo_path,
|
||||
pr_type='ready',
|
||||
fork_owner=fork_owner,
|
||||
target_branch=target_branch,
|
||||
)
|
||||
|
||||
# Assert API calls
|
||||
assert mock_get.call_count == 2
|
||||
|
||||
# Verify target branch was checked in original repo, not fork
|
||||
target_branch_check = mock_get.call_args_list[1]
|
||||
assert target_branch_check[0][0] == f'https://api.github.com/repos/test-owner/test-repo/branches/{target_branch}'
|
||||
|
||||
# Check PR creation
|
||||
mock_post.assert_called_once()
|
||||
post_data = mock_post.call_args[1]['json']
|
||||
assert post_data['base'] == target_branch # PR should target the specified branch
|
||||
assert post_data['head'] == 'openhands-fix-issue-42' # Branch name should be standard
|
||||
|
||||
# Check that push was to fork
|
||||
push_call = mock_run.call_args_list[1]
|
||||
assert f'https://test-user:test-token@github.com/{fork_owner}/test-repo.git' in str(push_call)
|
||||
|
||||
|
||||
@patch('subprocess.run')
|
||||
@patch('requests.post')
|
||||
@patch('requests.get')
|
||||
def test_send_pull_request_target_branch_with_additional_message(
|
||||
mock_get, mock_post, mock_run, mock_github_issue, mock_output_dir
|
||||
):
|
||||
"""Test that target_branch works correctly with additional PR message."""
|
||||
repo_path = os.path.join(mock_output_dir, 'repo')
|
||||
target_branch = 'feature-branch'
|
||||
additional_message = 'Additional PR context'
|
||||
|
||||
# Mock API responses
|
||||
mock_get.side_effect = [
|
||||
MagicMock(status_code=404), # Branch doesn't exist
|
||||
MagicMock(status_code=200), # Target branch exists
|
||||
]
|
||||
|
||||
mock_post.return_value.json.return_value = {
|
||||
'html_url': 'https://github.com/test-owner/test-repo/pull/1'
|
||||
}
|
||||
|
||||
# Mock subprocess.run calls
|
||||
mock_run.side_effect = [
|
||||
MagicMock(returncode=0), # git checkout -b
|
||||
MagicMock(returncode=0), # git push
|
||||
]
|
||||
|
||||
# Call the function with target_branch and additional_message
|
||||
result = send_pull_request(
|
||||
github_issue=mock_github_issue,
|
||||
github_token='test-token',
|
||||
github_username='test-user',
|
||||
patch_dir=repo_path,
|
||||
pr_type='ready',
|
||||
target_branch=target_branch,
|
||||
additional_message=additional_message,
|
||||
)
|
||||
|
||||
# Assert API calls
|
||||
assert mock_get.call_count == 2
|
||||
|
||||
# Check PR creation
|
||||
mock_post.assert_called_once()
|
||||
post_data = mock_post.call_args[1]['json']
|
||||
assert post_data['base'] == target_branch
|
||||
assert additional_message in post_data['body']
|
||||
assert 'This pull request fixes #42' in post_data['body']
|
||||
|
||||
|
||||
@patch('requests.get')
|
||||
def test_send_pull_request_invalid_target_branch(
|
||||
mock_get, mock_github_issue, mock_output_dir
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user