Skip commits that don't generate a patch + fixes to get export working

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

Index: third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_exporter.py
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_exporter.py b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_exporter.py
index d7f5b4de58e3a7336669c8ec0e8c52f8ca9d6a65..07c08165d11e12853bf8a74081ed9cbb1ed504eb 100644
--- a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_exporter.py
+++ b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_exporter.py
@@ -18,32 +18,40 @@ class TestExporter(object):
         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'])
-
-            # TODO(jeffcarp): Check the PR status here
-
-            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!')
+            self.merge_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.create_export_pull_request()
+
+    def merge_pull_request(self, pull_request):
+        """Attempts to merge an in-flight PR.
+
+        Args:
+            pull_request: a dict representing the pull request.
+        """
+        _log.info('In-flight PR found: #%d', pull_request['number'])
+        _log.info(pull_request['title'])
+
+        # TODO(jeffcarp): Check the PR status here
 
-        # 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)
+        if self.dry_run:
+            _log.info('[dry_run] Would have attempted to merge PR')

[qyearsley, 2016/12/07 19:05:36]

Optional minor rephrasing: "[dry run] Would attempt to merge PR."

+        else:
+            _log.info('Merging...')
+            self.wpt_github.merge_pull_request(pull_request['number'])
+            _log.info('PR merged!')
+            # TODO(jeffcarp): Delete remote branch after merging
+
+    def create_export_pull_request(self):
+        """Looks for exportable commits in Chromium, creates one PR if exists.
+        """

[qyearsley, 2016/12/07 19:05:36]

Possible rephrasing:

Creates a PR for the earliest exportable commit in Chromium if there is one.

Note that this also makes the request to github to create the PR, right? And it
always returns None?

+        local_wpt = LocalWPT(self.host)
         chromium_wpt = ChromiumWPT(self.host)
 
         # TODO(jeffcarp): have the script running this fetch Chromium origin/master
@@ -65,6 +73,16 @@ class TestExporter(object):
         # TODO(jeffcarp): Have this function return ChromiumCommits
         exportable_commits = chromium_wpt.exportable_commits_since(chromium_commit.sha)
 
+        def has_patch(commit):
+            # TODO(jeffcarp): now do a test comparison of patch against local WPT
+            # TODO(jeffcarp): dedupe from format_patch below
+            outbound_commit = ChromiumCommit(self.host, sha=commit)
+            patch = outbound_commit.format_patch()
+            output = local_wpt.test_patch(patch)
+            return bool(output)
+
+        exportable_commits = filter(has_patch, exportable_commits)
+
         if not exportable_commits:
             _log.info('No exportable commits found in Chromium, stopping.')
             return

[qyearsley, 2016/12/07 19:05:36]

Note: we'll want to extract a separate function to get the list of exportable
commits since the last export; we could do that in this CL or another one if it
would be cleaner.

[jeffcarp, 2016/12/07 20:24:38]

I'd prefer landing this on master and doing it in its own CL if you're ok with
that.

@@ -80,8 +98,6 @@ class TestExporter(object):
         patch = outbound_commit.format_patch()
         message = outbound_commit.message()
 
-        # TODO: now do a test comparison of patch against local WPT
-
         if self.dry_run:
             _log.info('[dry_run] Stopping before creating PR')
             _log.info('\n\n[dry_run] message:')
@@ -90,9 +106,11 @@ class TestExporter(object):
             _log.info(patch)
             return
 
-        local_branch_name = local_wpt.create_branch_with_patch(message, patch)
+        remote_branch_name = local_wpt.create_branch_with_patch(message, patch)
 
         self.wpt_github.create_pr(
-            local_branch_name=local_branch_name,
+            remote_branch_name=remote_branch_name,
             desc_title=outbound_commit.subject(),
             body=outbound_commit.body())
+
+        # TODO(jeffcarp): Create a comment on the PR to CC foolip and qyearsley