Refactor WPT Export to ensure only one PR in flight at a time

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

+# Copyright 2016 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import logging
+from webkitpy.w3c.local_wpt import LocalWPT
+from webkitpy.w3c.chromium_wpt import ChromiumWPT
+from webkitpy.w3c.chromium_commit import ChromiumCommit
+
+_log = logging.getLogger(__name__)
+
+
+class TestExporter(object):
+
+    def __init__(self, host, wpt_github, dry_run=False):
+        self.host = host
+        self.wpt_github = wpt_github
+        self.dry_run = dry_run
+
+    def run(self):
+        # First, poll for an in-flight pull request and merge if exists
+        pull_requests = self.wpt_github.in_flight_pull_requests()
+
+        if len(pull_requests) == 1:
+            pull_request = pull_requests.pop()
+
+            _log.info('In-flight PR found: #%d', pull_request['number'])
+            _log.info(pull_request['title'])
+
+            # In the future we would check the PR status here.
+            # Because the current bot doesn't use the GitHub status system

[foolip, 2016/11/29 11:12:20]

You mean the stability checker I assume? There are still other things that run
using the status checks, at least it takes a while for the merge button to go
green. Make this a TODO :)

+            # (it just comments the results), this will be a bit more difficult.
+
+            if self.dry_run:
+                _log.info('[dry_run] Would have attempted to merge PR')
+            else:
+                _log.info('Merging...')
+                self.wpt_github.merge_pull_request(pull_request['number'])
+                _log.info('PR merged!')
+        elif len(pull_requests) > 1:
+            _log.error(pull_requests)
+            raise Exception('More than two in-flight PRs!')

[foolip, 2016/11/29 11:12:21]

Maybe link to them so that a human can close them?

[jeffcarp, 2016/11/29 19:00:48]

👍

+
+        # Second, look for exportable commits in Chromium
+        # At this point, no in-flight PRs should exist
+        # If there was an issue merging, it should have errored out
+        local_wpt = LocalWPT(self.host, use_github=False)
+        chromium_wpt = ChromiumWPT(self.host)
+
+        # TODO(jeffcarp): fetch Chromium origin/master as well

[foolip, 2016/11/29 11:12:21]

I'm thinking this could be done by the system that runs this script? If the
script updates the repo it's in, it would also have to restart itself, since it
could have changed in origin/master.

[jeffcarp, 2016/11/29 19:00:48]

Good point I'll change the TODO.

+        # TODO(jeffcarp): move WPT fetch out of its constructor to match planned ChromiumWPT pattern
+
+        wpt_commit, chromium_commit = local_wpt.most_recent_chromium_commit()
+        assert chromium_commit, 'No Chromium commit found, this is impossible'
+
+        wpt_behind_master = local_wpt.commits_behind_master(wpt_commit)
+
+        _log.info('\nLast Chromium export commit in web-platform-tests:')
+        _log.info('web-platform-tests@%s', wpt_commit)
+        _log.info('(%d behind web-platform-tests@origin/master)', wpt_behind_master)
+
+        _log.info('\nThe above WPT commit points to the following Chromium commit:')
+        _log.info('chromium@%s', chromium_commit.sha)
+        _log.info('(%d behind chromium@origin/master)', chromium_commit.num_behind_master())
+
+        # TODO(jeffcarp): Have this function return ChromiumCommits
+        exportable_commits = chromium_wpt.exportable_commits_since(chromium_commit.sha)
+
+        if not exportable_commits:
+            _log.info('No exportable commits found in Chromium, stopping.')
+            return
+
+        _log.info('Found %d exportable commits in chromium:', len(exportable_commits))

[foolip, 2016/11/29 11:12:21]

s/chromium/Chromium/, or the other way around above.

+        for commit in exportable_commits:
+            _log.info('- %s %s', commit, chromium_wpt.subject(commit))
+
+        outbound_commit = ChromiumCommit(self.host, sha=exportable_commits[-1])

[foolip, 2016/11/29 11:12:21]

--reverse inside exportable_commits_since would make this a bit nicer I think,
otherwise some comment is needed to explain that the last exportable commit is
actually the first, chronologically.

+        _log.info('Picking the earliest commit and creating a PR')
+        _log.info('- %s %s', outbound_commit.sha, outbound_commit.subject())
+
+        if self.dry_run:
+            _log.info('[dry_run] Stopping before creating PR')
+            return
+
+        patch = outbound_commit.format_patch()
+        message = outbound_commit.message()
+
+        local_branch_name = local_wpt.create_branch_with_patch(message, patch)
+
+        self.wpt_github.create_pr(
+            local_branch_name=local_branch_name,
+            desc_title=outbound_commit.subject(),
+            body=outbound_commit.body())