[WPT Export] Support merging all in-flight PRs at once

This change has no real effect currently, but it prepares the exporter for being able to merge multiple in-flight branches at once.

BUG=686471
R=qyearsley@chromium.org

Change-Id: I79938cd9e3ba7cb96723cc5922bfef438a4dc510
Reviewed-on: https://chromium-review.googlesource.com/455536
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#457847}

third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_exporter.py

@@ -6,7 +6,7 @@ import logging
 
 from webkitpy.w3c.local_wpt import LocalWPT
 from webkitpy.w3c.common import exportable_commits_since
-from webkitpy.w3c.wpt_github import WPTGitHub
+from webkitpy.w3c.wpt_github import WPTGitHub, MergeError
 
 _log = logging.getLogger(__name__)
 
@@ -21,44 +21,44 @@ class TestExporter(object):
         self.local_wpt.fetch()
 
     def run(self):
-        """Query in-flight pull requests, then merge PR or create one.
+        """Query in-flight pull requests, then merges PR or creates one.
 
         This script assumes it will be run on a regular interval. On
         each invocation, it will either attempt to merge or attempt to
         create a PR, never both.
         """
         pull_requests = self.wpt_github.in_flight_pull_requests()
-
-        if len(pull_requests) == 1:
-            self.merge_in_flight_pull_request(pull_requests.pop())
-        elif len(pull_requests) > 1:
-            _log.error(pull_requests)
-            # TODO(jeffcarp): Print links to PRs
-            raise Exception('More than two in-flight PRs!')
-        else:
+        self.merge_all_pull_requests(pull_requests)
+
+        # TODO(jeffcarp): The below line will enforce draining all open PRs before
+        # adding any more to the queue, which mirrors current behavior. After this
+        # change lands, modify the following to:
+        # - for each exportable commit
+        #   - check if there's a corresponding PR
+        #   - if not, create one
+        if not pull_requests:
+            _log.info('No in-flight PRs found, looking for exportable commits.')
             self.export_first_exportable_commit()
 
-    def merge_in_flight_pull_request(self, pull_request):
-        """Attempt to merge an in-flight PR.
-
-        Args:
-            pull_request: a PR object returned from the GitHub API.
-        """
-
-        _log.info('In-flight PR found: #%d', pull_request['number'])
-        _log.info(pull_request['title'])
+    def merge_all_pull_requests(self, pull_requests):
+        for pr in pull_requests:
+            self.merge_pull_request(pr)
 
-        # TODO(jeffcarp): Check the PR status here (for Travis CI, etc.)
+    def merge_pull_request(self, pull_request):
+        _log.info('In-flight PR found: %s', pull_request.title)
+        _log.info('https://github.com/w3c/web-platform-tests/pull/%d', pull_request.number)
+        _log.info('Attempting to merge...')
 
         if self.dry_run:
             _log.info('[dry_run] Would have attempted to merge PR')
             return
 
-        _log.info('Merging...')
-        self.wpt_github.merge_pull_request(pull_request['number'])
-        _log.info('PR merged! Deleting branch.')
-        self.wpt_github.delete_remote_branch('chromium-export-try')
-        _log.info('Branch deleted!')
+        branch = self.wpt_github.get_pr_branch(pull_request.number)
+        try:
+            self.wpt_github.merge_pull_request(pull_request.number)
+            self.wpt_github.delete_remote_branch(branch)
+        except MergeError:
+            _log.info('Could not merge PR.')
 
     def export_first_exportable_commit(self):
         """Looks for exportable commits in Chromium, creates PR if found."""

third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_exporter_unittest.py

@@ -7,6 +7,7 @@ import unittest
 from webkitpy.common.host_mock import MockHost
 from webkitpy.common.system.executive_mock import MockExecutive
 from webkitpy.w3c.test_exporter import TestExporter
+from webkitpy.w3c.wpt_github import PullRequest
 from webkitpy.w3c.wpt_github_mock import MockWPTGitHub
 
 
@@ -15,38 +16,51 @@ class TestExporterTest(unittest.TestCase):
     def setUp(self):
         self.host = MockHost()
 
-    def test_stops_if_more_than_one_pr_is_in_flight(self):
+    def test_merges_more_than_one_pr(self):
         host = MockHost()
         test_exporter = TestExporter(host, 'gh-username', 'gh-token')
-        test_exporter.wpt_github = MockWPTGitHub(pull_requests=[{'id': 1}, {'id': 2}])
-
-        # TODO: make Exception more specific
-        with self.assertRaises(Exception):
-            test_exporter.run()
-
-    def test_if_pr_exists_merges_it(self):
-        host = MockHost()
-        test_exporter = TestExporter(host, 'gh-username', 'gh-token')
-        test_exporter.wpt_github = MockWPTGitHub(pull_requests=[{'number': 1, 'title': 'abc'}])
+        test_exporter.wpt_github = MockWPTGitHub(pull_requests=[
+            PullRequest(title='title1', number=1234),
+            PullRequest(title='title2', number=5678),
+        ])
         test_exporter.run()
 
-        self.assertIn('merge_pull_request', test_exporter.wpt_github.calls)
-
-    def test_merge_failure_errors_out(self):
+        self.assertEqual(test_exporter.wpt_github.calls, [
+            'in_flight_pull_requests',
+            'get_pr_branch',
+            'merge_pull_request',
+            'delete_remote_branch',
+            'get_pr_branch',
+            'merge_pull_request',
+            'delete_remote_branch',
+        ])
+
+    def test_merges_all_prs_even_if_one_fails(self):
         host = MockHost()
         test_exporter = TestExporter(host, 'gh-username', 'gh-token')
-        test_exporter.wpt_github = MockWPTGitHub(pull_requests=[{'number': 1, 'title': 'abc'}],
-                                                 unsuccessful_merge=True)
+        test_exporter.wpt_github = MockWPTGitHub(pull_requests=[
+            PullRequest(title='title1', number=1234),
+            PullRequest(title='title2', number=5678),
+        ], unsuccessful_merge_index=0)
 
-        # TODO: make Exception more specific
-        with self.assertRaises(Exception):
-            test_exporter.run()
+        test_exporter.run()
+        self.assertEqual(test_exporter.wpt_github.pull_requests_merged, [5678])
+        self.assertEqual(test_exporter.wpt_github.calls, [
+            'in_flight_pull_requests',
+            'get_pr_branch',
+            'merge_pull_request',
+            'get_pr_branch',
+            'merge_pull_request',
+            'delete_remote_branch',
+        ])
 
     def test_dry_run_stops_before_creating_pr(self):
         host = MockHost()
         host.executive = MockExecutive(output='beefcafe')
         test_exporter = TestExporter(host, 'gh-username', 'gh-token', dry_run=True)
-        test_exporter.wpt_github = MockWPTGitHub(pull_requests=[{'number': 1, 'title': 'abc'}])
+        test_exporter.wpt_github = MockWPTGitHub(pull_requests=[
+            PullRequest(title='title1', number=1234),
+        ])
         test_exporter.run()
 
         self.assertEqual(test_exporter.wpt_github.calls, ['in_flight_pull_requests'])

third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_github.py

@@ -7,6 +7,8 @@ import json
 import logging
 import urllib2
 
+from collections import namedtuple
+
 
 _log = logging.getLogger(__name__)
 API_BASE = 'https://api.github.com'
@@ -75,7 +77,16 @@ class WPTGitHub(object):
         path = '/search/issues?q=repo:w3c/web-platform-tests%20is:open%20type:pr%20label:{}'.format(EXPORT_LABEL)
         data, status_code = self.request(path, method='GET')
         if status_code == 200:
-            return data['items']
+            return [PullRequest(title=item['title'],
+                                number=item['number']) for item in data['items']]
+        else:
+            raise Exception('Non-200 status code (%s): %s' % (status_code, data))
+
+    def get_pr_branch(self, pr_number):
+        path = '/repos/w3c/web-platform-tests/pulls/{}'.format(pr_number)
+        data, status_code = self.request(path, method='GET')
+        if status_code == 200:
+            return data['head']['ref']
         else:
             raise Exception('Non-200 status code (%s): %s' % (status_code, data))
 
@@ -84,14 +95,19 @@ class WPTGitHub(object):
         body = {
             'merge_method': 'rebase',
         }
-        data, status_code = self.request(path, method='PUT', body=body)
 
-        if status_code == 405:
-            raise Exception('PR did not passed necessary checks to merge: %d' % pull_request_number)
-        elif status_code == 200:
-            return data
-        else:
-            raise Exception('PR could not be merged: %d' % pull_request_number)
+        try:
+            data, status_code = self.request(path, method='PUT', body=body)
+        except urllib2.HTTPError as e:
+            if e.code == 405:
+                raise MergeError()
+            else:
+                raise
+
+        if status_code == 200:
+            raise Exception('Received non-200 status code while merging PR #%d' % pull_request_number)
+
+        return data
 
     def delete_remote_branch(self, remote_branch_name):
         # TODO(jeffcarp): Unit test this method
@@ -103,3 +119,14 @@ class WPTGitHub(object):
             raise Exception('Received non-204 status code attempting to delete remote branch: {}'.format(status_code))
 
         return data
+
+
+class MergeError(Exception):
+    """An error specifically for when a PR cannot be merged.
+
+    This should only be thrown when GitHub returns status code 405,
+    indicating that the PR could not be merged.
+    """
+    pass
+
+PullRequest = namedtuple('PullRequest', ['title', 'number'])

third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_github_mock.py

@@ -2,14 +2,17 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+from webkitpy.w3c.wpt_github import MergeError
+
 
 class MockWPTGitHub(object):
 
-    def __init__(self, pull_requests, unsuccessful_merge=False):
+    def __init__(self, pull_requests, unsuccessful_merge_index=-1):
         self.pull_requests = pull_requests
-        self.unsuccessful_merge = unsuccessful_merge
         self.calls = []
         self.pull_requests_created = []
+        self.pull_requests_merged = []
+        self.unsuccessful_merge_index = unsuccessful_merge_index
 
     def in_flight_pull_requests(self):
         self.calls.append('in_flight_pull_requests')
@@ -17,8 +20,12 @@ class MockWPTGitHub(object):
 
     def merge_pull_request(self, number):
         self.calls.append('merge_pull_request')
-        if self.unsuccessful_merge:
-            raise Exception('PR could not be merged: %d' % number)
+
+        for index, pr in enumerate(self.pull_requests):
+            if pr.number == number and index == self.unsuccessful_merge_index:
+                raise MergeError()
+
+        self.pull_requests_merged.append(number)
 
     def create_pr(self, remote_branch_name, desc_title, body):
         self.calls.append('create_pr')
@@ -33,3 +40,7 @@ class MockWPTGitHub(object):
 
     def delete_remote_branch(self, _):
         self.calls.append('delete_remote_branch')
+
+    def get_pr_branch(self, number):
+        self.calls.append('get_pr_branch')
+        return 'fake branch for PR {}'.format(number)