Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(165)

Unified Diff: third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_exporter.py

Issue 2544173002: Skip commits that don't generate a patch + fixes to get export working (Closed)
Patch Set: Merge ChromiumWPT functionality into TestExporter, expose exportable_commits Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 d23f73b21972aa8b4ff4f8355fe30e1b5a497901..f4bf8b718562732393e4f53267b5cf5b24f3ff1c 100644
--- a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_exporter.py
+++ b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_exporter.py
@@ -4,21 +4,23 @@
import logging
from webkitpy.w3c.local_wpt import LocalWPT
-from webkitpy.w3c.chromium_wpt import ChromiumWPT
from webkitpy.w3c.chromium_commit import ChromiumCommit
+CHROMIUM_WPT_DIR = 'third_party/WebKit/LayoutTests/imported/wpt/'
_log = logging.getLogger(__name__)
+# TODO(jeffcarp): integrate chromium_wpt_unittests into this module's tests
+# TODO(jeffcarp): have the script running this fetch Chromium origin/master
class TestExporter(object):
def __init__(self, host, wpt_github, dry_run=False):
self.host = host
self.wpt_github = wpt_github
+ self.local_wpt = LocalWPT(self.host)
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:
@@ -35,35 +37,50 @@ class TestExporter(object):
_log.info('Merging...')
self.wpt_github.merge_pull_request(pull_request['number'])
_log.info('PR merged!')
+ # TODO(jeffcarp): Delete remote branch after merging
+
+ # The script needs to stop here since the Chromium
+ # WPT mirror lags behind a little bit. Also, it's probably safer
+ # to not be merging and creating a PR in the same run.
+ return
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()
- # 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)
+ def merge_pull_request(self, pull_request):
+ """Attempts to merge an in-flight PR.
- # TODO(jeffcarp): have the script running this fetch Chromium origin/master
- # TODO(jeffcarp): move WPT fetch out of its constructor to match planned ChromiumWPT pattern
+ Args:
+ pull_request: a dict representing the pull request.
+ """
+ _log.info('In-flight PR found: #%d', pull_request['number'])
+ _log.info(pull_request['title'])
- wpt_commit, chromium_commit = local_wpt.most_recent_chromium_commit()
- assert chromium_commit, 'No Chromium commit found, this is impossible'
+ # TODO(jeffcarp): Check the PR status here
- wpt_behind_master = local_wpt.commits_behind_master(wpt_commit)
+ if self.dry_run:
+ _log.info('[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
- _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)
+ def create_export_pull_request(self):
+ """Creates a PR for the earliest exportable commit in Chromium if there is one.
- _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())
+ This will create a branch on w3c/web-platform-tests and make a request
+ to the GitHub API to create a pull request.
+
+ At this point, no in-flight PRs should exist.
- # TODO(jeffcarp): Have this function return ChromiumCommits
- exportable_commits = chromium_wpt.exportable_commits_since(chromium_commit.sha)
+ Returns:
+ None
+ """
+ exportable_commits = self.exportable_commits()
if not exportable_commits:
_log.info('No exportable commits found in Chromium, stopping.')
@@ -71,17 +88,15 @@ class TestExporter(object):
_log.info('Found %d exportable commits in Chromium:', len(exportable_commits))
for commit in exportable_commits:
- _log.info('- %s %s', commit, chromium_wpt.subject(commit))
+ _log.info('- %s %s', commit, commit.subject())
- outbound_commit = ChromiumCommit(self.host, sha=exportable_commits[0])
_log.info('Picking the earliest commit and creating a PR')
+ outbound_commit = exportable_commits[0]
_log.info('- %s %s', outbound_commit.sha, outbound_commit.subject())
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,11 +105,60 @@ class TestExporter(object):
_log.info(patch)
return
- local_branch_name = local_wpt.create_branch_with_patch(message, patch)
+ remote_branch_name = self.local_wpt.create_branch_with_patch(message, patch)
response_data = 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())
_log.info('Create PR response: %s', response_data)
+
+ if response_data:
+ data, status_code = self.wpt_github.add_label(response_data['number'])
+ _log.info('Add label response (status %s): %s', status_code, data)
+
+ # TODO(jeffcarp): Create a comment on the PR to CC foolip and qyearsley
+ # also explain that this is an automated export and will be auto-merged if green
+
+ def exportable_commits(self):
+ """Returns all exportable Chromium commits, earliest first.
+
qyearsley 2016/12/15 20:59:14 Possible extra comment: this looks at commits in C
+ Returns:
+ A list of ChromiumCommits
qyearsley 2016/12/15 20:59:14 Nit: period
+ """
+ wpt_commit, chromium_commit = self.local_wpt.most_recent_chromium_commit()
+ assert chromium_commit, 'No Chromium commit found, this is impossible.'
+
+ _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)',
+ self.local_wpt.commits_behind_master(wpt_commit))
+
+ _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())
qyearsley 2016/12/15 20:59:14 This could potentially be changed to debug-level r
+
+ chromium_commits = [ChromiumCommit(self.host, sha=sha)
+ for sha in self.commits_with_changes_in_wpt_since(chromium_commit.sha)]
+
+ def is_exportable(commit):
+ patch = commit.format_patch()
+ return (
+ patch
+ and self.local_wpt.test_patch(patch)
+ and 'NOEXPORT=true' not in commit.message()
+ and not commit.message().startswith('Import ')
+ )
+
+ return [c for c in chromium_commits if is_exportable(c)]
+
+ def commits_with_changes_in_wpt_since(self, sha):
+ toplevel = self.host.executive.run_command([
+ 'git', 'rev-parse', '--show-toplevel'
+ ]).strip()
+
+ return self.host.executive.run_command([
+ 'git', 'rev-list', '{}..HEAD'.format(sha), '--reverse',
+ '--', toplevel + '/' + CHROMIUM_WPT_DIR
+ ]).splitlines()

Powered by Google App Engine
This is Rietveld 408576698