mirror of
https://github.com/All-Hands-AI/OpenHands.git
synced 2026-01-10 15:28:14 -05:00
(Chore): Rm legacy resolver code (#8001)
This commit is contained in:
Rohit Malhotra
@@ -16,7 +16,6 @@ from openhands.resolver.interfaces.gitlab import GitlabIssueHandler
|
||||
from openhands.resolver.interfaces.issue import Issue
|
||||
from openhands.resolver.interfaces.issue_definitions import ServiceContextIssue
|
||||
from openhands.resolver.io_utils import (
|
||||
load_all_resolver_outputs,
|
||||
load_single_resolver_output,
|
||||
)
|
||||
from openhands.resolver.patching import apply_diff, parse_patch
|
||||
@@ -549,40 +548,6 @@ def process_single_issue(
|
||||
)
|
||||
|
||||
|
||||
def process_all_successful_issues(
|
||||
output_dir: str,
|
||||
token: str,
|
||||
username: str,
|
||||
platform: ProviderType,
|
||||
pr_type: str,
|
||||
llm_config: LLMConfig,
|
||||
fork_owner: str | None,
|
||||
base_domain: str | None = None,
|
||||
) -> None:
|
||||
# Determine default base_domain based on platform
|
||||
if base_domain is None:
|
||||
base_domain = 'github.com' if platform == ProviderType.GITHUB else 'gitlab.com'
|
||||
output_path = os.path.join(output_dir, 'output.jsonl')
|
||||
for resolver_output in load_all_resolver_outputs(output_path):
|
||||
if resolver_output.success:
|
||||
logger.info(f'Processing issue {resolver_output.issue.number}')
|
||||
process_single_issue(
|
||||
output_dir,
|
||||
resolver_output,
|
||||
token,
|
||||
username,
|
||||
platform,
|
||||
pr_type,
|
||||
llm_config,
|
||||
fork_owner,
|
||||
False,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
base_domain,
|
||||
)
|
||||
|
||||
|
||||
def main() -> None:
|
||||
parser = argparse.ArgumentParser(
|
||||
description='Send a pull request to Github or Gitlab.'
|
||||
@@ -703,42 +668,28 @@ def main() -> None:
|
||||
if not os.path.exists(my_args.output_dir):
|
||||
raise ValueError(f'Output directory {my_args.output_dir} does not exist.')
|
||||
|
||||
if my_args.issue_number == 'all_successful':
|
||||
if not username:
|
||||
raise ValueError('username is required.')
|
||||
process_all_successful_issues(
|
||||
my_args.output_dir,
|
||||
token,
|
||||
username,
|
||||
platform,
|
||||
my_args.pr_type,
|
||||
llm_config,
|
||||
my_args.fork_owner,
|
||||
my_args.base_domain,
|
||||
)
|
||||
else:
|
||||
if not my_args.issue_number.isdigit():
|
||||
raise ValueError(f'Issue number {my_args.issue_number} is not a number.')
|
||||
issue_number = int(my_args.issue_number)
|
||||
output_path = os.path.join(my_args.output_dir, 'output.jsonl')
|
||||
resolver_output = load_single_resolver_output(output_path, issue_number)
|
||||
if not username:
|
||||
raise ValueError('username is required.')
|
||||
process_single_issue(
|
||||
my_args.output_dir,
|
||||
resolver_output,
|
||||
token,
|
||||
username,
|
||||
platform,
|
||||
my_args.pr_type,
|
||||
llm_config,
|
||||
my_args.fork_owner,
|
||||
my_args.send_on_failure,
|
||||
my_args.target_branch,
|
||||
my_args.reviewer,
|
||||
my_args.pr_title,
|
||||
my_args.base_domain,
|
||||
)
|
||||
if not my_args.issue_number.isdigit():
|
||||
raise ValueError(f'Issue number {my_args.issue_number} is not a number.')
|
||||
issue_number = int(my_args.issue_number)
|
||||
output_path = os.path.join(my_args.output_dir, 'output.jsonl')
|
||||
resolver_output = load_single_resolver_output(output_path, issue_number)
|
||||
if not username:
|
||||
raise ValueError('username is required.')
|
||||
process_single_issue(
|
||||
my_args.output_dir,
|
||||
resolver_output,
|
||||
token,
|
||||
username,
|
||||
platform,
|
||||
my_args.pr_type,
|
||||
llm_config,
|
||||
my_args.fork_owner,
|
||||
my_args.send_on_failure,
|
||||
my_args.target_branch,
|
||||
my_args.reviewer,
|
||||
my_args.pr_title,
|
||||
my_args.base_domain,
|
||||
)
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
|
||||
@@ -14,12 +14,11 @@ from openhands.resolver.send_pull_request import (
|
||||
initialize_repo,
|
||||
load_single_resolver_output,
|
||||
make_commit,
|
||||
process_all_successful_issues,
|
||||
process_single_issue,
|
||||
send_pull_request,
|
||||
update_existing_pull_request,
|
||||
)
|
||||
|
||||
from openhands.resolver.send_pull_request import main
|
||||
|
||||
@pytest.fixture
|
||||
def mock_output_dir():
|
||||
@@ -1028,131 +1027,6 @@ def test_process_single_issue_unsuccessful(
|
||||
mock_send_pull_request.assert_not_called()
|
||||
|
||||
|
||||
@patch('openhands.resolver.send_pull_request.load_all_resolver_outputs')
|
||||
@patch('openhands.resolver.send_pull_request.process_single_issue')
|
||||
def test_process_all_successful_issues(
|
||||
mock_process_single_issue, mock_load_all_resolver_outputs, mock_llm_config
|
||||
):
|
||||
# Create ResolverOutput objects with properly initialized GithubIssue instances
|
||||
resolver_output_1 = ResolverOutput(
|
||||
issue=Issue(
|
||||
owner='test-owner',
|
||||
repo='test-repo',
|
||||
number=1,
|
||||
title='Issue 1',
|
||||
body='Body 1',
|
||||
),
|
||||
issue_type='issue',
|
||||
instruction='Test instruction 1',
|
||||
base_commit='def456',
|
||||
git_patch='Test patch 1',
|
||||
history=[],
|
||||
metrics={},
|
||||
success=True,
|
||||
comment_success=None,
|
||||
result_explanation='Test success 1',
|
||||
error=None,
|
||||
)
|
||||
|
||||
resolver_output_2 = ResolverOutput(
|
||||
issue=Issue(
|
||||
owner='test-owner',
|
||||
repo='test-repo',
|
||||
number=2,
|
||||
title='Issue 2',
|
||||
body='Body 2',
|
||||
),
|
||||
issue_type='issue',
|
||||
instruction='Test instruction 2',
|
||||
base_commit='ghi789',
|
||||
git_patch='Test patch 2',
|
||||
history=[],
|
||||
metrics={},
|
||||
success=False,
|
||||
comment_success=None,
|
||||
result_explanation='',
|
||||
error='Test error 2',
|
||||
)
|
||||
|
||||
resolver_output_3 = ResolverOutput(
|
||||
issue=Issue(
|
||||
owner='test-owner',
|
||||
repo='test-repo',
|
||||
number=3,
|
||||
title='Issue 3',
|
||||
body='Body 3',
|
||||
),
|
||||
issue_type='issue',
|
||||
instruction='Test instruction 3',
|
||||
base_commit='jkl012',
|
||||
git_patch='Test patch 3',
|
||||
history=[],
|
||||
metrics={},
|
||||
success=True,
|
||||
comment_success=None,
|
||||
result_explanation='Test success 3',
|
||||
error=None,
|
||||
)
|
||||
|
||||
mock_load_all_resolver_outputs.return_value = [
|
||||
resolver_output_1,
|
||||
resolver_output_2,
|
||||
resolver_output_3,
|
||||
]
|
||||
|
||||
# Call the function
|
||||
process_all_successful_issues(
|
||||
'output_dir',
|
||||
'token',
|
||||
'username',
|
||||
ProviderType.GITHUB,
|
||||
'draft',
|
||||
mock_llm_config, # llm_config
|
||||
None, # fork_owner
|
||||
)
|
||||
|
||||
# Assert that process_single_issue was called for successful issues only
|
||||
assert mock_process_single_issue.call_count == 2
|
||||
|
||||
# Check that the function was called with the correct arguments for successful issues
|
||||
mock_process_single_issue.assert_has_calls(
|
||||
[
|
||||
call(
|
||||
'output_dir',
|
||||
resolver_output_1,
|
||||
'token',
|
||||
'username',
|
||||
ProviderType.GITHUB,
|
||||
'draft',
|
||||
mock_llm_config,
|
||||
None,
|
||||
False,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
'github.com',
|
||||
),
|
||||
call(
|
||||
'output_dir',
|
||||
resolver_output_3,
|
||||
'token',
|
||||
'username',
|
||||
ProviderType.GITHUB,
|
||||
'draft',
|
||||
mock_llm_config,
|
||||
None,
|
||||
False,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
'github.com',
|
||||
),
|
||||
]
|
||||
)
|
||||
|
||||
# Add more assertions as needed to verify the behavior of the function
|
||||
|
||||
|
||||
@patch('httpx.get')
|
||||
@patch('subprocess.run')
|
||||
def test_send_pull_request_branch_naming(
|
||||
@@ -1217,7 +1091,6 @@ def test_send_pull_request_branch_naming(
|
||||
|
||||
|
||||
@patch('openhands.resolver.send_pull_request.argparse.ArgumentParser')
|
||||
@patch('openhands.resolver.send_pull_request.process_all_successful_issues')
|
||||
@patch('openhands.resolver.send_pull_request.process_single_issue')
|
||||
@patch('openhands.resolver.send_pull_request.load_single_resolver_output')
|
||||
@patch('openhands.resolver.send_pull_request.identify_token')
|
||||
@@ -1229,10 +1102,8 @@ def test_main(
|
||||
mock_identify_token,
|
||||
mock_load_single_resolver_output,
|
||||
mock_process_single_issue,
|
||||
mock_process_all_successful_issues,
|
||||
mock_parser,
|
||||
):
|
||||
from openhands.resolver.send_pull_request import main
|
||||
|
||||
# Setup mock parser
|
||||
mock_args = MagicMock()
|
||||
@@ -1300,19 +1171,6 @@ def test_main(
|
||||
mock_path_exists.assert_called_with('/mock/output')
|
||||
mock_load_single_resolver_output.assert_called_with('/mock/output/output.jsonl', 42)
|
||||
|
||||
# Test for 'all_successful' issue number
|
||||
mock_args.issue_number = 'all_successful'
|
||||
main()
|
||||
mock_process_all_successful_issues.assert_called_with(
|
||||
'/mock/output',
|
||||
'mock_token',
|
||||
'mock_username',
|
||||
ProviderType.GITHUB,
|
||||
'draft',
|
||||
llm_config,
|
||||
None,
|
||||
ANY,
|
||||
)
|
||||
|
||||
# Test for invalid issue number
|
||||
mock_args.issue_number = 'invalid'
|
||||
|
||||
@@ -15,11 +15,11 @@ from openhands.resolver.send_pull_request import (
|
||||
initialize_repo,
|
||||
load_single_resolver_output,
|
||||
make_commit,
|
||||
process_all_successful_issues,
|
||||
process_single_issue,
|
||||
send_pull_request,
|
||||
update_existing_pull_request,
|
||||
)
|
||||
from openhands.resolver.send_pull_request import main
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
@@ -929,131 +929,6 @@ def test_process_single_issue_unsuccessful(
|
||||
mock_send_pull_request.assert_not_called()
|
||||
|
||||
|
||||
@patch('openhands.resolver.send_pull_request.load_all_resolver_outputs')
|
||||
@patch('openhands.resolver.send_pull_request.process_single_issue')
|
||||
def test_process_all_successful_issues(
|
||||
mock_process_single_issue, mock_load_all_resolver_outputs, mock_llm_config
|
||||
):
|
||||
# Create ResolverOutput objects with properly initialized GitlabIssue instances
|
||||
resolver_output_1 = ResolverOutput(
|
||||
issue=Issue(
|
||||
owner='test-owner',
|
||||
repo='test-repo',
|
||||
number=1,
|
||||
title='Issue 1',
|
||||
body='Body 1',
|
||||
),
|
||||
issue_type='issue',
|
||||
instruction='Test instruction 1',
|
||||
base_commit='def456',
|
||||
git_patch='Test patch 1',
|
||||
history=[],
|
||||
metrics={},
|
||||
success=True,
|
||||
comment_success=None,
|
||||
result_explanation='Test success 1',
|
||||
error=None,
|
||||
)
|
||||
|
||||
resolver_output_2 = ResolverOutput(
|
||||
issue=Issue(
|
||||
owner='test-owner',
|
||||
repo='test-repo',
|
||||
number=2,
|
||||
title='Issue 2',
|
||||
body='Body 2',
|
||||
),
|
||||
issue_type='issue',
|
||||
instruction='Test instruction 2',
|
||||
base_commit='ghi789',
|
||||
git_patch='Test patch 2',
|
||||
history=[],
|
||||
metrics={},
|
||||
success=False,
|
||||
comment_success=None,
|
||||
result_explanation='',
|
||||
error='Test error 2',
|
||||
)
|
||||
|
||||
resolver_output_3 = ResolverOutput(
|
||||
issue=Issue(
|
||||
owner='test-owner',
|
||||
repo='test-repo',
|
||||
number=3,
|
||||
title='Issue 3',
|
||||
body='Body 3',
|
||||
),
|
||||
issue_type='issue',
|
||||
instruction='Test instruction 3',
|
||||
base_commit='jkl012',
|
||||
git_patch='Test patch 3',
|
||||
history=[],
|
||||
metrics={},
|
||||
success=True,
|
||||
comment_success=None,
|
||||
result_explanation='Test success 3',
|
||||
error=None,
|
||||
)
|
||||
|
||||
mock_load_all_resolver_outputs.return_value = [
|
||||
resolver_output_1,
|
||||
resolver_output_2,
|
||||
resolver_output_3,
|
||||
]
|
||||
|
||||
# Call the function
|
||||
process_all_successful_issues(
|
||||
'output_dir',
|
||||
'token',
|
||||
'username',
|
||||
ProviderType.GITLAB,
|
||||
'draft',
|
||||
mock_llm_config, # llm_config
|
||||
None, # fork_owner
|
||||
)
|
||||
|
||||
# Assert that process_single_issue was called for successful issues only
|
||||
assert mock_process_single_issue.call_count == 2
|
||||
|
||||
# Check that the function was called with the correct arguments for successful issues
|
||||
mock_process_single_issue.assert_has_calls(
|
||||
[
|
||||
call(
|
||||
'output_dir',
|
||||
resolver_output_1,
|
||||
'token',
|
||||
'username',
|
||||
ProviderType.GITLAB,
|
||||
'draft',
|
||||
mock_llm_config,
|
||||
None,
|
||||
False,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
'gitlab.com',
|
||||
),
|
||||
call(
|
||||
'output_dir',
|
||||
resolver_output_3,
|
||||
'token',
|
||||
'username',
|
||||
ProviderType.GITLAB,
|
||||
'draft',
|
||||
mock_llm_config,
|
||||
None,
|
||||
False,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
'gitlab.com',
|
||||
),
|
||||
]
|
||||
)
|
||||
|
||||
# Add more assertions as needed to verify the behavior of the function
|
||||
|
||||
|
||||
@patch('httpx.get')
|
||||
@patch('subprocess.run')
|
||||
def test_send_pull_request_branch_naming(
|
||||
@@ -1119,7 +994,6 @@ def test_send_pull_request_branch_naming(
|
||||
|
||||
|
||||
@patch('openhands.resolver.send_pull_request.argparse.ArgumentParser')
|
||||
@patch('openhands.resolver.send_pull_request.process_all_successful_issues')
|
||||
@patch('openhands.resolver.send_pull_request.process_single_issue')
|
||||
@patch('openhands.resolver.send_pull_request.load_single_resolver_output')
|
||||
@patch('openhands.resolver.send_pull_request.identify_token')
|
||||
@@ -1131,10 +1005,8 @@ def test_main(
|
||||
mock_identify_token,
|
||||
mock_load_single_resolver_output,
|
||||
mock_process_single_issue,
|
||||
mock_process_all_successful_issues,
|
||||
mock_parser,
|
||||
):
|
||||
from openhands.resolver.send_pull_request import main
|
||||
|
||||
# Setup mock parser
|
||||
mock_args = MagicMock()
|
||||
@@ -1202,20 +1074,6 @@ def test_main(
|
||||
mock_path_exists.assert_called_with('/mock/output')
|
||||
mock_load_single_resolver_output.assert_called_with('/mock/output/output.jsonl', 42)
|
||||
|
||||
# Test for 'all_successful' issue number
|
||||
mock_args.issue_number = 'all_successful'
|
||||
main()
|
||||
mock_process_all_successful_issues.assert_called_with(
|
||||
'/mock/output',
|
||||
'mock_token',
|
||||
'mock_username',
|
||||
ProviderType.GITLAB,
|
||||
'draft',
|
||||
llm_config,
|
||||
None,
|
||||
ANY,
|
||||
)
|
||||
|
||||
# Test for invalid issue number
|
||||
mock_args.issue_number = 'invalid'
|
||||
with pytest.raises(ValueError):
|
||||
|
||||
Reference in New Issue
Block a user