Browse Source

Fix issue #5450: In openhands-resolver.yml, request code review from the person who initiated the workflow (#5451)

Co-authored-by: Graham Neubig <neubig@gmail.com>
OpenHands 1 year ago
parent
commit
6de177521f

+ 2 - 1
.github/workflows/openhands-resolver.yml

@@ -239,7 +239,8 @@ jobs:
           if [ "${{ steps.check_result.outputs.RESOLUTION_SUCCESS }}" == "true" ]; then
           if [ "${{ steps.check_result.outputs.RESOLUTION_SUCCESS }}" == "true" ]; then
             cd /tmp && python -m openhands.resolver.send_pull_request \
             cd /tmp && python -m openhands.resolver.send_pull_request \
               --issue-number ${{ env.ISSUE_NUMBER }} \
               --issue-number ${{ env.ISSUE_NUMBER }} \
-              --pr-type draft | tee pr_result.txt && \
+              --pr-type draft \
+              --reviewer ${{ github.actor }} | tee pr_result.txt && \
               grep "draft created" pr_result.txt | sed 's/.*\///g' > pr_number.txt
               grep "draft created" pr_result.txt | sed 's/.*\///g' > pr_number.txt
           else
           else
             cd /tmp && python -m openhands.resolver.send_pull_request \
             cd /tmp && python -m openhands.resolver.send_pull_request \

+ 23 - 0
openhands/resolver/send_pull_request.py

@@ -238,6 +238,7 @@ def send_pull_request(
     fork_owner: str | None = None,
     fork_owner: str | None = None,
     additional_message: str | None = None,
     additional_message: str | None = None,
     target_branch: str | None = None,
     target_branch: str | None = None,
+    reviewer: str | None = None,
 ) -> str:
 ) -> str:
     """Send a pull request to a GitHub repository.
     """Send a pull request to a GitHub repository.
 
 
@@ -350,6 +351,19 @@ def send_pull_request(
         response.raise_for_status()
         response.raise_for_status()
         pr_data = response.json()
         pr_data = response.json()
 
 
+        # Request review if a reviewer was specified
+        if reviewer and pr_type != 'branch':
+            review_data = {'reviewers': [reviewer]}
+            review_response = requests.post(
+                f'{base_url}/pulls/{pr_data["number"]}/requested_reviewers',
+                headers=headers,
+                json=review_data,
+            )
+            if review_response.status_code != 201:
+                print(
+                    f'Warning: Failed to request review from {reviewer}: {review_response.text}'
+                )
+
         url = pr_data['html_url']
         url = pr_data['html_url']
 
 
     print(f'{pr_type} created: {url}\n\n--- Title: {pr_title}\n\n--- Body:\n{pr_body}')
     print(f'{pr_type} created: {url}\n\n--- Title: {pr_title}\n\n--- Body:\n{pr_body}')
@@ -520,6 +534,7 @@ def process_single_issue(
     fork_owner: str | None,
     fork_owner: str | None,
     send_on_failure: bool,
     send_on_failure: bool,
     target_branch: str | None = None,
     target_branch: str | None = None,
+    reviewer: str | None = None,
 ) -> None:
 ) -> None:
     if not resolver_output.success and not send_on_failure:
     if not resolver_output.success and not send_on_failure:
         print(
         print(
@@ -569,6 +584,7 @@ def process_single_issue(
             fork_owner=fork_owner,
             fork_owner=fork_owner,
             additional_message=resolver_output.success_explanation,
             additional_message=resolver_output.success_explanation,
             target_branch=target_branch,
             target_branch=target_branch,
+            reviewer=reviewer,
         )
         )
 
 
 
 
@@ -665,6 +681,12 @@ def main():
         default=None,
         default=None,
         help='Target branch to create the pull request against (defaults to repository default branch)',
         help='Target branch to create the pull request against (defaults to repository default branch)',
     )
     )
+    parser.add_argument(
+        '--reviewer',
+        type=str,
+        help='GitHub username of the person to request review from',
+        default=None,
+    )
     my_args = parser.parse_args()
     my_args = parser.parse_args()
 
 
     github_token = (
     github_token = (
@@ -718,6 +740,7 @@ def main():
             my_args.fork_owner,
             my_args.fork_owner,
             my_args.send_on_failure,
             my_args.send_on_failure,
             my_args.target_branch,
             my_args.target_branch,
+            my_args.reviewer,
         )
         )
 
 
 
 

+ 66 - 0
tests/unit/resolver/test_send_pull_request.py

@@ -432,6 +432,69 @@ def test_send_pull_request(
         assert post_data['draft'] == (pr_type == 'draft')
         assert post_data['draft'] == (pr_type == 'draft')
 
 
 
 
+@patch('subprocess.run')
+@patch('requests.post')
+@patch('requests.get')
+def test_send_pull_request_with_reviewer(
+    mock_get, mock_post, mock_run, mock_github_issue, mock_output_dir
+):
+    repo_path = os.path.join(mock_output_dir, 'repo')
+    reviewer = 'test-reviewer'
+
+    # Mock API responses
+    mock_get.side_effect = [
+        MagicMock(status_code=404),  # Branch doesn't exist
+        MagicMock(json=lambda: {'default_branch': 'main'}),  # Get default branch
+    ]
+
+    # Mock PR creation response
+    mock_post.side_effect = [
+        MagicMock(
+            status_code=201,
+            json=lambda: {
+                'html_url': 'https://github.com/test-owner/test-repo/pull/1',
+                'number': 1,
+            },
+        ),  # PR creation
+        MagicMock(status_code=201),  # Reviewer request
+    ]
+
+    # Mock subprocess.run calls
+    mock_run.side_effect = [
+        MagicMock(returncode=0),  # git checkout -b
+        MagicMock(returncode=0),  # git push
+    ]
+
+    # Call the function with reviewer
+    result = send_pull_request(
+        github_issue=mock_github_issue,
+        github_token='test-token',
+        github_username='test-user',
+        patch_dir=repo_path,
+        pr_type='ready',
+        reviewer=reviewer,
+    )
+
+    # Assert API calls
+    assert mock_get.call_count == 2
+    assert mock_post.call_count == 2
+
+    # Check PR creation
+    pr_create_call = mock_post.call_args_list[0]
+    assert pr_create_call[1]['json']['title'] == 'Fix issue #42: Test Issue'
+
+    # Check reviewer request
+    reviewer_request_call = mock_post.call_args_list[1]
+    assert (
+        reviewer_request_call[0][0]
+        == 'https://api.github.com/repos/test-owner/test-repo/pulls/1/requested_reviewers'
+    )
+    assert reviewer_request_call[1]['json'] == {'reviewers': ['test-reviewer']}
+
+    # Check the result URL
+    assert result == 'https://github.com/test-owner/test-repo/pull/1'
+
+
 @patch('requests.get')
 @patch('requests.get')
 def test_send_pull_request_invalid_target_branch(
 def test_send_pull_request_invalid_target_branch(
     mock_get, mock_github_issue, mock_output_dir
     mock_get, mock_github_issue, mock_output_dir
@@ -764,6 +827,7 @@ def test_process_single_issue(
         fork_owner=None,
         fork_owner=None,
         additional_message=resolver_output.success_explanation,
         additional_message=resolver_output.success_explanation,
         target_branch=None,
         target_branch=None,
+        reviewer=None,
     )
     )
 
 
 
 
@@ -1031,6 +1095,7 @@ def test_main(
     mock_args.llm_base_url = 'mock_url'
     mock_args.llm_base_url = 'mock_url'
     mock_args.llm_api_key = 'mock_key'
     mock_args.llm_api_key = 'mock_key'
     mock_args.target_branch = None
     mock_args.target_branch = None
+    mock_args.reviewer = None
     mock_parser.return_value.parse_args.return_value = mock_args
     mock_parser.return_value.parse_args.return_value = mock_args
 
 
     # Setup environment variables
     # Setup environment variables
@@ -1065,6 +1130,7 @@ def test_main(
         None,
         None,
         False,
         False,
         mock_args.target_branch,
         mock_args.target_branch,
+        mock_args.reviewer,
     )
     )
 
 
     # Other assertions
     # Other assertions