Commit 9b5b69a0 authored by Jeff Carpenter's avatar Jeff Carpenter Committed by Commit Bot

Revert "[WPT Export] Create GitHub PRs from Gerrit CLs"

This reverts commit ff67881b.

Reason for revert: Updated credentials but that uncovered a bug with how we handle responses from the Gerrit API.

Original change's description:
> [WPT Export] Create GitHub PRs from Gerrit CLs
> 
> Bug: 700092
> Change-Id: I7fd398d1e25b77493eef609cbf818747216cd140
> Reviewed-on: https://chromium-review.googlesource.com/479931
> Commit-Queue: Jeff Carpenter <jeffcarp@chromium.org>
> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#471061}

TBR=qyearsley@chromium.org,jeffcarp@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Bug: 700092

Change-Id: Ieb6f34d9762184033c10d6ba7c3b4e8208a7fe5b
Reviewed-on: https://chromium-review.googlesource.com/503553Reviewed-by: default avatarJeff Carpenter <jeffcarp@chromium.org>
Commit-Queue: Jeff Carpenter <jeffcarp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#471109}
parent 031529c7
...@@ -11,11 +11,7 @@ from webkitpy.w3c.chromium_finder import absolute_chromium_dir ...@@ -11,11 +11,7 @@ from webkitpy.w3c.chromium_finder import absolute_chromium_dir
WPT_DEST_NAME = 'wpt' WPT_DEST_NAME = 'wpt'
WPT_GH_ORG = 'w3c' WPT_GH_REPO_URL_TEMPLATE = 'https://{}@github.com/w3c/web-platform-tests.git'
WPT_GH_REPO_NAME = 'web-platform-tests'
WPT_GH_URL = 'https://github.com/%s/%s/' % (WPT_GH_ORG, WPT_GH_REPO_NAME)
WPT_GH_SSH_URL_TEMPLATE = 'https://{}@github.com/%s/%s.git' % (WPT_GH_ORG, WPT_GH_REPO_NAME)
WPT_REVISION_FOOTER = 'WPT-Export-Revision:'
# TODO(qyearsley): This directory should be able to be constructed with # TODO(qyearsley): This directory should be able to be constructed with
# WebKitFinder and WPT_DEST_NAME, plus the string "external". # WebKitFinder and WPT_DEST_NAME, plus the string "external".
......
...@@ -12,7 +12,7 @@ _log = logging.getLogger(__name__) ...@@ -12,7 +12,7 @@ _log = logging.getLogger(__name__)
URL_BASE = 'https://chromium-review.googlesource.com' URL_BASE = 'https://chromium-review.googlesource.com'
class GerritAPI(object): class Gerrit(object):
"""A utility class for the Chromium code review API. """A utility class for the Chromium code review API.
Wraps the API for Chromium's Gerrit instance at chromium-review.googlesource.com. Wraps the API for Chromium's Gerrit instance at chromium-review.googlesource.com.
...@@ -23,18 +23,13 @@ class GerritAPI(object): ...@@ -23,18 +23,13 @@ class GerritAPI(object):
self.user = user self.user = user
self.token = token self.token = token
def get(self, path, raw=False): def api_get(self, url):
url = URL_BASE + path
raw_data = self.host.web.get_binary(url) raw_data = self.host.web.get_binary(url)
if raw: return json.loads(raw_data[5:]) # strip JSONP preamble
return raw_data
# Gerrit API responses are prefixed by a 5-character JSONP preamble def api_post(self, url, data):
return json.loads(raw_data[5:]) if not (self.user and self.token):
raise 'Gerrit user and token requried for authenticated routes.'
def post(self, path, data):
url = URL_BASE + path
assert self.user and self.token, 'Gerrit user and token required for authenticated routes.'
b64auth = base64.b64encode('{}:{}'.format(self.user, self.token)) b64auth = base64.b64encode('{}:{}'.format(self.user, self.token))
headers = { headers = {
...@@ -43,72 +38,54 @@ class GerritAPI(object): ...@@ -43,72 +38,54 @@ class GerritAPI(object):
} }
return self.host.web.request('POST', url, data=json.dumps(data), headers=headers) return self.host.web.request('POST', url, data=json.dumps(data), headers=headers)
def query_exportable_open_cls(self, limit=200): def cl_url(self, number):
path = ('/changes/?q=project:\"chromium/src\"+status:open' return 'https://chromium-review.googlesource.com/c/{}/'.format(number)
'&o=CURRENT_FILES&o=CURRENT_REVISION&o=COMMIT_FOOTERS'
'&o=DETAILED_ACCOUNTS&n={}').format(limit) def get_diff(self, cl):
open_cls_data = self.get(path) """Get full diff for latest revision of CL."""
open_cls = [GerritCL(data, self) for data in open_cls_data] revision = cl['revisions'].items()[0][1]
revision_id = cl['revisions'].items()[0][0]
return [cl for cl in open_cls if cl.is_exportable()] files = revision['files'].keys()
diff = ''
class GerritCL(object): for filename in files:
"""A data wrapper for a Chromium Gerrit CL.""" url = '{url_base}/changes/{change_id}/revisions/{revision_id}/files/{file_id}/diff'.format(
url_base=URL_BASE,
def __init__(self, data, api): change_id=cl['id'],
assert data['change_id'] revision_id=revision_id,
self._data = data file_id=urllib.quote_plus(filename),
self.api = api )
data = self.api_get(url)
@property diff += data
def url(self):
return 'https://chromium-review.googlesource.com/c/%s' % self._data['_number'] return diff
@property def post_comment(self, cl_id, revision_id, message):
def subject(self): url = '{url_base}/a/changes/{change_id}/revisions/{revision_id}/review'.format(
return self._data['subject'] url_base=URL_BASE,
change_id=cl_id,
@property revision_id=revision_id,
def change_id(self):
return self._data['change_id']
@property
def owner_email(self):
return self._data['owner']['email']
@property
def current_revision(self):
return self._data['current_revision']
def latest_commit_message_with_footers(self):
return self._data['revisions'][self.current_revision]['commit_with_footers']
@property
def current_revision_description(self):
return self._data['revisions'][self.current_revision]['description']
def post_comment(self, message):
path = '/a/changes/{change_id}/revisions/current/review'.format(
change_id=self.change_id,
) )
return self.api.post(path, {'message': message}) return self.api_post(url, {'message': message})
def is_exportable(self): def query_open_cls(self, limit=200):
files = self.current_revision['files'].keys() url = ('{}/changes/?q=project:\"chromium/src\"+status:open'
'&o=CURRENT_FILES&o=CURRENT_REVISION&o=COMMIT_FOOTERS&n={}').format(URL_BASE, limit)
open_cls = self.api_get(url)
# Guard against accidental CLs that touch thousands of files. return [cl for cl in open_cls if self.is_exportable(cl)]
if len(files) > 1000:
_log.info('Rejecting CL with over 1000 files: %s (ID: %s) ', self.subject, self.change_id)
return False
if self.subject.startswith('Import wpt@'): def is_exportable(self, cl):
revision = cl['revisions'].items()[0][1]
files = revision['files'].keys()
if cl['subject'].startswith('Import wpt@'):
return False return False
if 'Import' in self.subject: if 'Import' in cl['subject']:
return False return False
if 'NOEXPORT=true' in self.current_revision['commit_with_footers']: if 'NOEXPORT=true' in revision['commit_with_footers']:
return False return False
files_in_wpt = [f for f in files if f.startswith('third_party/WebKit/LayoutTests/external/wpt')] files_in_wpt = [f for f in files if f.startswith('third_party/WebKit/LayoutTests/external/wpt')]
...@@ -129,8 +106,3 @@ class GerritCL(object): ...@@ -129,8 +106,3 @@ class GerritCL(object):
and not filename.startswith('.') and not filename.startswith('.')
and not filename.endswith('.json') and not filename.endswith('.json')
) )
def get_patch(self):
"""Gets patch for latest revision of CL."""
path = '/changes/{change_id}/revisions/current/patch'.format(change_id=self.change_id)
return base64.b64decode(self.api.get(path, raw=True))
...@@ -3,18 +3,12 @@ ...@@ -3,18 +3,12 @@
# found in the LICENSE file. # found in the LICENSE file.
class MockGerritAPI(object): class MockGerrit(object):
def __init__(self, host, user, token): def __init__(self, host, user, token):
self.host = host self.host = host
self.user = user self.user = user
self.token = token self.token = token
def query_exportable_open_cls(self): def query_open_cls(self):
return [] return []
def get(self, path, raw=False): # pylint: disable=unused-argument
return '' if raw else {}
def post(self, path, data): # pylint: disable=unused-argument
return {}
...@@ -8,7 +8,7 @@ import logging ...@@ -8,7 +8,7 @@ import logging
from webkitpy.common.system.executive import ScriptError from webkitpy.common.system.executive import ScriptError
from webkitpy.w3c.chromium_commit import ChromiumCommit from webkitpy.w3c.chromium_commit import ChromiumCommit
from webkitpy.w3c.common import WPT_GH_SSH_URL_TEMPLATE, CHROMIUM_WPT_DIR from webkitpy.w3c.common import WPT_GH_REPO_URL_TEMPLATE, CHROMIUM_WPT_DIR
_log = logging.getLogger(__name__) _log = logging.getLogger(__name__)
...@@ -36,7 +36,7 @@ class LocalWPT(object): ...@@ -36,7 +36,7 @@ class LocalWPT(object):
self.run(['git', 'checkout', 'origin/master']) self.run(['git', 'checkout', 'origin/master'])
else: else:
_log.info('Cloning GitHub w3c/web-platform-tests into %s', self.path) _log.info('Cloning GitHub w3c/web-platform-tests into %s', self.path)
remote_url = WPT_GH_SSH_URL_TEMPLATE.format(self.gh_token) remote_url = WPT_GH_REPO_URL_TEMPLATE.format(self.gh_token)
self.host.executive.run_command(['git', 'clone', remote_url, self.path]) self.host.executive.run_command(['git', 'clone', remote_url, self.path])
def run(self, command, **kwargs): def run(self, command, **kwargs):
...@@ -62,7 +62,7 @@ class LocalWPT(object): ...@@ -62,7 +62,7 @@ class LocalWPT(object):
self.run(['git', 'clean', '-fdx']) self.run(['git', 'clean', '-fdx'])
self.run(['git', 'checkout', 'origin/master']) self.run(['git', 'checkout', 'origin/master'])
def create_branch_with_patch(self, branch_name, message, patch, author, force_push=False): def create_branch_with_patch(self, branch_name, message, patch, author):
"""Commits the given patch and pushes to the upstream repo. """Commits the given patch and pushes to the upstream repo.
Args: Args:
...@@ -70,20 +70,9 @@ class LocalWPT(object): ...@@ -70,20 +70,9 @@ class LocalWPT(object):
message: Commit message string. message: Commit message string.
patch: A patch that can be applied by git apply. patch: A patch that can be applied by git apply.
author: The git commit author. author: The git commit author.
force_push: Applies the -f flag in `git push`.
""" """
self.clean() self.clean()
try:
# This won't be exercised in production because wpt-exporter
# always runs on a clean machine. But it's useful when running
# locally since branches stick around.
_log.info('Deleting old branch %s', branch_name)
self.run(['git', 'branch', '-D', branch_name])
except ScriptError:
# Ignore errors if branch not found.
pass
_log.info('Creating local branch %s', branch_name) _log.info('Creating local branch %s', branch_name)
self.run(['git', 'checkout', '-b', branch_name]) self.run(['git', 'checkout', '-b', branch_name])
...@@ -95,13 +84,7 @@ class LocalWPT(object): ...@@ -95,13 +84,7 @@ class LocalWPT(object):
self.run(['git', 'apply', '-'], input=patch) self.run(['git', 'apply', '-'], input=patch)
self.run(['git', 'add', '.']) self.run(['git', 'add', '.'])
self.run(['git', 'commit', '--author', author, '-am', message]) self.run(['git', 'commit', '--author', author, '-am', message])
self.run(['git', 'push', 'origin', branch_name])
# Force push is necessary when updating a PR with a new patch
# from Gerrit.
if force_push:
self.run(['git', 'push', '-f', 'origin', branch_name])
else:
self.run(['git', 'push', 'origin', branch_name])
def test_patch(self, patch, chromium_commit=None): def test_patch(self, patch, chromium_commit=None):
"""Returns the expected output of a patch against origin/master. """Returns the expected output of a patch against origin/master.
......
...@@ -93,7 +93,6 @@ class LocalWPTTest(unittest.TestCase): ...@@ -93,7 +93,6 @@ class LocalWPTTest(unittest.TestCase):
['git', 'reset', '--hard', 'HEAD'], ['git', 'reset', '--hard', 'HEAD'],
['git', 'clean', '-fdx'], ['git', 'clean', '-fdx'],
['git', 'checkout', 'origin/master'], ['git', 'checkout', 'origin/master'],
['git', 'branch', '-D', 'chromium-export-decafbad'],
['git', 'checkout', '-b', 'chromium-export-decafbad'], ['git', 'checkout', '-b', 'chromium-export-decafbad'],
['git', 'apply', '-'], ['git', 'apply', '-'],
['git', 'add', '.'], ['git', 'add', '.'],
......
...@@ -5,18 +5,15 @@ ...@@ -5,18 +5,15 @@
import logging import logging
from webkitpy.w3c.local_wpt import LocalWPT from webkitpy.w3c.local_wpt import LocalWPT
from webkitpy.w3c.common import ( from webkitpy.w3c.common import exportable_commits_over_last_n_commits
exportable_commits_over_last_n_commits, from webkitpy.w3c.gerrit import Gerrit
WPT_GH_URL,
WPT_REVISION_FOOTER
)
from webkitpy.w3c.gerrit import GerritAPI, GerritCL
from webkitpy.w3c.wpt_github import WPTGitHub, MergeError from webkitpy.w3c.wpt_github import WPTGitHub, MergeError
_log = logging.getLogger(__name__) _log = logging.getLogger(__name__)
PR_HISTORY_WINDOW = 100 PR_HISTORY_WINDOW = 100
COMMIT_HISTORY_WINDOW = 5000 COMMIT_HISTORY_WINDOW = 5000
WPT_URL = 'https://github.com/w3c/web-platform-tests/'
class TestExporter(object): class TestExporter(object):
...@@ -25,7 +22,7 @@ class TestExporter(object): ...@@ -25,7 +22,7 @@ class TestExporter(object):
self.host = host self.host = host
self.wpt_github = WPTGitHub(host, gh_user, gh_token, pr_history_window=PR_HISTORY_WINDOW) self.wpt_github = WPTGitHub(host, gh_user, gh_token, pr_history_window=PR_HISTORY_WINDOW)
self.gerrit = GerritAPI(self.host, gerrit_user, gerrit_token) self.gerrit = Gerrit(self.host, gerrit_user, gerrit_token)
self.dry_run = dry_run self.dry_run = dry_run
self.local_wpt = LocalWPT(self.host, gh_token) self.local_wpt = LocalWPT(self.host, gh_token)
...@@ -36,43 +33,39 @@ class TestExporter(object): ...@@ -36,43 +33,39 @@ class TestExporter(object):
The exporter will look in chronological order at every commit in Chromium. The exporter will look in chronological order at every commit in Chromium.
""" """
open_gerrit_cls = self.gerrit.query_exportable_open_cls() open_gerrit_cls = self.gerrit.query_open_cls()
self.process_gerrit_cls(open_gerrit_cls) self.process_gerrit_cls(open_gerrit_cls)
exportable_commits = self.get_exportable_commits(limit=COMMIT_HISTORY_WINDOW) exportable_commits = self.get_exportable_commits(limit=COMMIT_HISTORY_WINDOW)
for exportable_commit in exportable_commits: for exportable_commit in exportable_commits:
pull_request = self.wpt_github.pr_with_position(exportable_commit.position) pull_request = self.wpt_github.pr_with_position(exportable_commit.position)
if pull_request: if pull_request:
if pull_request.state == 'open': if pull_request.state == 'open':
self.merge_pull_request(pull_request) self.merge_pull_request(pull_request)
# TODO(jeffcarp): if this was from Gerrit, comment back on the Gerrit CL that the PR was merged
else: else:
_log.info('Pull request is not open: #%d %s', pull_request.number, pull_request.title) _log.info('Pull request is not open: #%d %s', pull_request.number, pull_request.title)
else: else:
self.create_pull_request(exportable_commit) self.create_pull_request(exportable_commit)
def process_gerrit_cls(self, gerrit_cls): def process_gerrit_cls(self, gerrit_cls):
"""Creates or updates PRs for Gerrit CLs.""" """Iterates through Gerrit CLs and prints their statuses.
Right now this method does nothing. In the future it will create PRs for CLs and help
transition them when they're landed.
"""
for cl in gerrit_cls: for cl in gerrit_cls:
_log.info('Found Gerrit in-flight CL: "%s" %s', cl.subject, cl.url) cl_url = 'https://chromium-review.googlesource.com/c/%s' % cl['_number']
_log.info('Found Gerrit in-flight CL: "%s" %s', cl['subject'], cl_url)
# Check if CL already has a corresponding PR # Check if CL already has a corresponding PR
pull_request = self.wpt_github.pr_with_change_id(cl.change_id) pull_request = self.wpt_github.pr_with_change_id(cl['change_id'])
if pull_request: if pull_request:
pr_url = '{}pull/{}'.format(WPT_GH_URL, pull_request.number) pr_url = '{}pull/{}'.format(WPT_URL, pull_request.number)
_log.info('In-flight PR found: %s', pr_url) _log.info('In-flight PR found: %s', pr_url)
pr_cl_revision = self.wpt_github.extract_metadata(WPT_REVISION_FOOTER + ' ', pull_request.body)
if cl.current_revision == pr_cl_revision:
_log.info('PR revision matches CL revision. Nothing to do here.')
continue
_log.info('New revision found, updating PR...')
self.create_or_update_pull_request_from_cl(cl, pull_request)
else: else:
_log.info('No in-flight PR found for CL. Creating...') _log.info('No in-flight PR found for CL.')
self.create_or_update_pull_request_from_cl(cl)
def get_exportable_commits(self, limit): def get_exportable_commits(self, limit):
...@@ -80,7 +73,7 @@ class TestExporter(object): ...@@ -80,7 +73,7 @@ class TestExporter(object):
def merge_pull_request(self, pull_request): def merge_pull_request(self, pull_request):
_log.info('In-flight PR found: %s', pull_request.title) _log.info('In-flight PR found: %s', pull_request.title)
_log.info('%spull/%d', WPT_GH_URL, pull_request.number) _log.info('https://github.com/w3c/web-platform-tests/pull/%d', pull_request.number)
if self.dry_run: if self.dry_run:
_log.info('[dry_run] Would have attempted to merge PR') _log.info('[dry_run] Would have attempted to merge PR')
...@@ -98,17 +91,6 @@ class TestExporter(object): ...@@ -98,17 +91,6 @@ class TestExporter(object):
# This is in the try block because if a PR can't be merged, we shouldn't # This is in the try block because if a PR can't be merged, we shouldn't
# delete its branch. # delete its branch.
self.wpt_github.delete_remote_branch(branch) self.wpt_github.delete_remote_branch(branch)
change_id = self.wpt_github.extract_metadata('Change-Id: ', pull_request.body)
if change_id:
cl = GerritCL(data={'change_id': change_id}, api=self.gerrit)
pr_url = '{}pull/{}'.format(WPT_GH_URL, pull_request.number)
cl.post_comment((
'The WPT PR for this CL has been merged upstream! {pr_url}'
).format(
pr_url=pr_url
))
except MergeError: except MergeError:
_log.info('Could not merge PR.') _log.info('Could not merge PR.')
...@@ -172,57 +154,3 @@ class TestExporter(object): ...@@ -172,57 +154,3 @@ class TestExporter(object):
_log.info('Add label response (status %s): %s', status_code, data) _log.info('Add label response (status %s): %s', status_code, data)
return response_data return response_data
def create_or_update_pull_request_from_cl(self, cl, pull_request=None):
patch = cl.get_patch()
updating = bool(pull_request)
if self.dry_run:
if updating:
_log.info('[dry_run] Stopping before updating PR from CL')
else:
_log.info('[dry_run] Stopping before creating PR from CL')
_log.info('\n\n[dry_run] subject:')
_log.info(cl.subject)
_log.debug('\n\n[dry_run] patch:')
_log.debug(patch)
return
message = cl.latest_commit_message_with_footers()
if not updating:
# Annotate revision footer for Exporter's later use.
message += '\n{} {}'.format(WPT_REVISION_FOOTER, cl.current_revision)
branch_name = 'chromium-export-cl-{id}'.format(id=cl.change_id)
self.local_wpt.create_branch_with_patch(branch_name, message, patch, cl.owner_email, force_push=True)
if updating:
response_data = self.wpt_github.update_pr(pull_request.number, cl.subject, message)
_log.debug('Update PR response: %s', response_data)
# TODO(jeffcarp): Turn PullRequest into a class with a .url method
cl.post_comment((
'Successfully updated WPT GitHub pull request with '
'new revision "{subject}": {pr_url}'
).format(
subject=cl.current_revision_description,
pr_url='%spull/%d' % (WPT_GH_URL, pull_request.number),
))
else:
response_data = self.wpt_github.create_pr(branch_name, cl.subject, message)
_log.debug('Create PR response: %s', response_data)
data, status_code = self.wpt_github.add_label(response_data['number'])
_log.info('Add label response (status %s): %s', status_code, data)
cl.post_comment((
'Exportable changes to web-platform-tests were detected in this CL '
'and a pull request in the upstream repo has been made: {pr_url}.\n\n'
'Travis CI has been kicked off and if it fails, we will let you know here. '
'If this CL lands and Travis CI is green, we will auto-merge the PR.'
).format(
pr_url='%spull/%d' % (WPT_GH_URL, response_data['number'])
))
return response_data
...@@ -10,8 +10,7 @@ from webkitpy.w3c.chromium_commit import ChromiumCommit ...@@ -10,8 +10,7 @@ from webkitpy.w3c.chromium_commit import ChromiumCommit
from webkitpy.w3c.test_exporter import TestExporter from webkitpy.w3c.test_exporter import TestExporter
from webkitpy.w3c.wpt_github import PullRequest from webkitpy.w3c.wpt_github import PullRequest
from webkitpy.w3c.wpt_github_mock import MockWPTGitHub from webkitpy.w3c.wpt_github_mock import MockWPTGitHub
from webkitpy.w3c.gerrit import GerritCL from webkitpy.w3c.gerrit_mock import MockGerrit
from webkitpy.w3c.gerrit_mock import MockGerritAPI
class TestExporterTest(unittest.TestCase): class TestExporterTest(unittest.TestCase):
...@@ -29,7 +28,7 @@ class TestExporterTest(unittest.TestCase): ...@@ -29,7 +28,7 @@ class TestExporterTest(unittest.TestCase):
test_exporter.wpt_github = MockWPTGitHub(pull_requests=[ test_exporter.wpt_github = MockWPTGitHub(pull_requests=[
PullRequest(title='title1', number=1234, body='', state='open'), PullRequest(title='title1', number=1234, body='', state='open'),
]) ])
test_exporter.gerrit = MockGerritAPI(host, 'gerrit-username', 'gerrit-token') test_exporter.gerrit = MockGerrit(host, 'gerrit-username', 'gerrit-token')
test_exporter.get_exportable_commits = lambda limit: [ test_exporter.get_exportable_commits = lambda limit: [
ChromiumCommit(host, position='refs/heads/master@{#458475}'), ChromiumCommit(host, position='refs/heads/master@{#458475}'),
ChromiumCommit(host, position='refs/heads/master@{#458476}'), ChromiumCommit(host, position='refs/heads/master@{#458476}'),
...@@ -72,19 +71,16 @@ class TestExporterTest(unittest.TestCase): ...@@ -72,19 +71,16 @@ class TestExporterTest(unittest.TestCase):
host.executive = MockExecutive(run_command_fn=mock_command) host.executive = MockExecutive(run_command_fn=mock_command)
test_exporter = TestExporter(host, 'gh-username', 'gh-token', gerrit_user=None, gerrit_token=None) test_exporter = TestExporter(host, 'gh-username', 'gh-token', gerrit_user=None, gerrit_token=None)
test_exporter.wpt_github = MockWPTGitHub(pull_requests=[], create_pr_fail_index=1) test_exporter.wpt_github = MockWPTGitHub(pull_requests=[], create_pr_fail_index=1)
test_exporter.gerrit = MockGerritAPI(host, 'gerrit-username', 'gerrit-token') test_exporter.gerrit = MockGerrit(host, 'gerrit-username', 'gerrit-token')
test_exporter.run() test_exporter.run()
self.assertEqual(test_exporter.wpt_github.calls, [ self.assertEqual(test_exporter.wpt_github.calls, [
'pr_with_position', 'pr_with_position',
'create_pr', 'create_pr',
'add_label',
'pr_with_position', 'pr_with_position',
'create_pr', 'create_pr',
'add_label',
'pr_with_position', 'pr_with_position',
'create_pr', 'create_pr',
'add_label',
]) ])
self.assertEqual(test_exporter.wpt_github.pull_requests_created, [ self.assertEqual(test_exporter.wpt_github.pull_requests_created, [
('chromium-export-c881563d73', 'older fake text', 'older fake text'), ('chromium-export-c881563d73', 'older fake text', 'older fake text'),
...@@ -123,7 +119,7 @@ class TestExporterTest(unittest.TestCase): ...@@ -123,7 +119,7 @@ class TestExporterTest(unittest.TestCase):
state='open' state='open'
), ),
], unsuccessful_merge_index=0) ], unsuccessful_merge_index=0)
test_exporter.gerrit = MockGerritAPI(host, 'gerrit-username', 'gerrit-token') test_exporter.gerrit = MockGerrit(host, 'gerrit-username', 'gerrit-token')
test_exporter.get_exportable_commits = lambda limit: [ test_exporter.get_exportable_commits = lambda limit: [
ChromiumCommit(host, position='refs/heads/master@{#458475}'), ChromiumCommit(host, position='refs/heads/master@{#458475}'),
ChromiumCommit(host, position='refs/heads/master@{#458476}'), ChromiumCommit(host, position='refs/heads/master@{#458476}'),
...@@ -137,7 +133,6 @@ class TestExporterTest(unittest.TestCase): ...@@ -137,7 +133,6 @@ class TestExporterTest(unittest.TestCase):
'merge_pull_request', 'merge_pull_request',
'pr_with_position', 'pr_with_position',
'create_pr', 'create_pr',
'add_label',
'pr_with_position', 'pr_with_position',
'pr_with_position', 'pr_with_position',
'get_pr_branch', 'get_pr_branch',
...@@ -149,98 +144,3 @@ class TestExporterTest(unittest.TestCase): ...@@ -149,98 +144,3 @@ class TestExporterTest(unittest.TestCase):
'git show text\nCr-Commit-Position: refs/heads/master@{#458476}', 'git show text\nCr-Commit-Position: refs/heads/master@{#458476}',
'git show text\nCr-Commit-Position: refs/heads/master@{#458476}'), 'git show text\nCr-Commit-Position: refs/heads/master@{#458476}'),
]) ])
def test_new_gerrit_cl(self):
host = MockHost()
test_exporter = TestExporter(host, 'gh-username', 'gh-token', gerrit_user=None,
gerrit_token=None, dry_run=False)
test_exporter.wpt_github = MockWPTGitHub(pull_requests=[])
test_exporter.get_exportable_commits = lambda limit: []
test_exporter.gerrit = MockGerritAPI(host, 'gerrit-username', 'gerrit-token')
test_exporter.gerrit.query_exportable_open_cls = lambda: [
GerritCL(data={
'change_id': '1',
'subject': 'subject',
'_number': '1',
'current_revision': '1',
'revisions': {
'1': {'commit_with_footers': 'a commit with footers'}
},
'owner': {'email': 'test@chromium.org'},
}, api=test_exporter.gerrit),
]
test_exporter.run()
self.assertEqual(test_exporter.wpt_github.calls, [
'pr_with_change_id',
'create_pr',
'add_label',
])
self.assertEqual(test_exporter.wpt_github.pull_requests_created, [
('chromium-export-cl-1',
'subject',
'a commit with footers\nWPT-Export-Revision: 1'),
])
def test_gerrit_cl_no_update_if_pr_with_same_revision(self):
host = MockHost()
test_exporter = TestExporter(host, 'gh-username', 'gh-token', gerrit_user=None,
gerrit_token=None, dry_run=False)
test_exporter.wpt_github = MockWPTGitHub(pull_requests=[
PullRequest(title='title1', number=1234,
body='description\nWPT-Export-Revision: 1', state='open'),
])
test_exporter.get_exportable_commits = lambda limit: []
test_exporter.gerrit = MockGerritAPI(host, 'gerrit-username', 'gerrit-token')
test_exporter.gerrit.query_exportable_open_cls = lambda: [
GerritCL(data={
'change_id': '1',
'subject': 'subject',
'_number': '1',
'current_revision': '1',
'revisions': {
'1': {'commit_with_footers': 'a commit with footers'}
},
'owner': {'email': 'test@chromium.org'},
}, api=test_exporter.gerrit),
]
test_exporter.run()
self.assertEqual(test_exporter.wpt_github.calls, [
'pr_with_change_id',
])
self.assertEqual(test_exporter.wpt_github.pull_requests_created, [])
def test_gerrit_cl_updates_if_cl_has_new_revision(self):
host = MockHost()
test_exporter = TestExporter(host, 'gh-username', 'gh-token', gerrit_user=None,
gerrit_token=None, dry_run=False)
test_exporter.wpt_github = MockWPTGitHub(pull_requests=[
PullRequest(title='title1', number=1234,
body='description\nWPT-Export-Revision: 1', state='open'),
])
test_exporter.get_exportable_commits = lambda limit: []
test_exporter.gerrit = MockGerritAPI(host, 'gerrit-username', 'gerrit-token')
test_exporter.gerrit.query_exportable_open_cls = lambda: [
GerritCL(data={
'change_id': '1',
'subject': 'subject',
'_number': '1',
'current_revision': '2',
'revisions': {
'1': {
'commit_with_footers': 'a commit with footers 1',
'description': 'subject 1',
},
'2': {
'commit_with_footers': 'a commit with footers 2',
'description': 'subject 2',
},
},
'owner': {'email': 'test@chromium.org'},
}, api=test_exporter.gerrit),
]
test_exporter.run()
self.assertEqual(test_exporter.wpt_github.calls, [
'pr_with_change_id',
'update_pr',
])
self.assertEqual(test_exporter.wpt_github.pull_requests_created, [])
...@@ -8,7 +8,6 @@ import logging ...@@ -8,7 +8,6 @@ import logging
import urllib2 import urllib2
from collections import namedtuple from collections import namedtuple
from webkitpy.w3c.common import WPT_GH_ORG, WPT_GH_REPO_NAME
from webkitpy.common.memoized import memoized from webkitpy.common.memoized import memoized
...@@ -64,7 +63,7 @@ class WPTGitHub(object): ...@@ -64,7 +63,7 @@ class WPTGitHub(object):
assert desc_title assert desc_title
assert body assert body
path = '/repos/%s/%s/pulls' % (WPT_GH_ORG, WPT_GH_REPO_NAME) path = '/repos/w3c/web-platform-tests/pulls'
body = { body = {
'title': desc_title, 'title': desc_title,
'body': body, 'body': body,
...@@ -78,45 +77,13 @@ class WPTGitHub(object): ...@@ -78,45 +77,13 @@ class WPTGitHub(object):
return data return data
def update_pr(self, pr_number, desc_title, body):
"""Updates a PR on GitHub.
API doc: https://developer.github.com/v3/pulls/#update-a-pull-request
Returns:
A raw response object if successful, None if not.
"""
path = '/repos/{}/{}/pulls/{}'.format(
WPT_GH_ORG,
WPT_GH_REPO_NAME,
pr_number
)
body = {
'title': desc_title,
'body': body,
}
data, status_code = self.request(path, method='PATCH', body=body)
if status_code != 201:
return None
return data
def add_label(self, number): def add_label(self, number):
path = '/repos/%s/%s/issues/%d/labels' % ( path = '/repos/w3c/web-platform-tests/issues/%d/labels' % number
WPT_GH_ORG,
WPT_GH_REPO_NAME,
number
)
body = [EXPORT_LABEL] body = [EXPORT_LABEL]
return self.request(path, method='POST', body=body) return self.request(path, method='POST', body=body)
def in_flight_pull_requests(self): def in_flight_pull_requests(self):
path = '/search/issues?q=repo:{}/{}%20is:open%20type:pr%20label:{}'.format( path = '/search/issues?q=repo:w3c/web-platform-tests%20is:open%20type:pr%20label:{}'.format(EXPORT_LABEL)
WPT_GH_ORG,
WPT_GH_REPO_NAME,
EXPORT_LABEL
)
data, status_code = self.request(path, method='GET') data, status_code = self.request(path, method='GET')
if status_code == 200: if status_code == 200:
return [self.make_pr_from_item(item) for item in data['items']] return [self.make_pr_from_item(item) for item in data['items']]
...@@ -134,17 +101,10 @@ class WPTGitHub(object): ...@@ -134,17 +101,10 @@ class WPTGitHub(object):
def all_pull_requests(self): def all_pull_requests(self):
# TODO(jeffcarp): Add pagination to fetch >99 PRs # TODO(jeffcarp): Add pagination to fetch >99 PRs
assert self._pr_history_window <= 100, 'Maximum GitHub page size exceeded.' assert self._pr_history_window <= 100, 'Maximum GitHub page size exceeded.'
path = ( path = ('/search/issues'
'/search/issues' '?q=repo:w3c/web-platform-tests%20type:pr%20label:{}'
'?q=repo:{}/{}%20type:pr%20label:{}' '&page=1'
'&page=1' '&per_page={}').format(EXPORT_LABEL, self._pr_history_window)
'&per_page={}'
).format(
WPT_GH_ORG,
WPT_GH_REPO_NAME,
EXPORT_LABEL,
self._pr_history_window
)
data, status_code = self.request(path, method='GET') data, status_code = self.request(path, method='GET')
if status_code == 200: if status_code == 200:
return [self.make_pr_from_item(item) for item in data['items']] return [self.make_pr_from_item(item) for item in data['items']]
...@@ -152,11 +112,7 @@ class WPTGitHub(object): ...@@ -152,11 +112,7 @@ class WPTGitHub(object):
raise Exception('Non-200 status code (%s): %s' % (status_code, data)) raise Exception('Non-200 status code (%s): %s' % (status_code, data))
def get_pr_branch(self, pr_number): def get_pr_branch(self, pr_number):
path = '/repos/{}/{}/pulls/{}'.format( path = '/repos/w3c/web-platform-tests/pulls/{}'.format(pr_number)
WPT_GH_ORG,
WPT_GH_REPO_NAME,
pr_number
)
data, status_code = self.request(path, method='GET') data, status_code = self.request(path, method='GET')
if status_code == 200: if status_code == 200:
return data['head']['ref'] return data['head']['ref']
...@@ -164,11 +120,7 @@ class WPTGitHub(object): ...@@ -164,11 +120,7 @@ class WPTGitHub(object):
raise Exception('Non-200 status code (%s): %s' % (status_code, data)) raise Exception('Non-200 status code (%s): %s' % (status_code, data))
def merge_pull_request(self, pull_request_number): def merge_pull_request(self, pull_request_number):
path = '/repos/%s/%s/pulls/%d/merge' % ( path = '/repos/w3c/web-platform-tests/pulls/%d/merge' % pull_request_number
WPT_GH_ORG,
WPT_GH_REPO_NAME,
pull_request_number
)
body = { body = {
# This currently will noop because the feature is in an opt-in beta. # This currently will noop because the feature is in an opt-in beta.
# Once it leaves beta this will start working. # Once it leaves beta this will start working.
...@@ -190,11 +142,7 @@ class WPTGitHub(object): ...@@ -190,11 +142,7 @@ class WPTGitHub(object):
def delete_remote_branch(self, remote_branch_name): def delete_remote_branch(self, remote_branch_name):
# TODO(jeffcarp): Unit test this method # TODO(jeffcarp): Unit test this method
path = '/repos/%s/%s/git/refs/heads/%s' % ( path = '/repos/w3c/web-platform-tests/git/refs/heads/%s' % remote_branch_name
WPT_GH_ORG,
WPT_GH_REPO_NAME,
remote_branch_name
)
data, status_code = self.request(path, method='DELETE') data, status_code = self.request(path, method='DELETE')
if status_code != 204: if status_code != 204:
...@@ -205,19 +153,19 @@ class WPTGitHub(object): ...@@ -205,19 +153,19 @@ class WPTGitHub(object):
def pr_with_change_id(self, target_change_id): def pr_with_change_id(self, target_change_id):
for pull_request in self.all_pull_requests(): for pull_request in self.all_pull_requests():
change_id = self.extract_metadata('Change-Id: ', pull_request.body) change_id = self._extract_metadata('Change-Id: ', pull_request.body)
if change_id == target_change_id: if change_id == target_change_id:
return pull_request return pull_request
return None return None
def pr_with_position(self, position): def pr_with_position(self, position):
for pull_request in self.all_pull_requests(): for pull_request in self.all_pull_requests():
pr_commit_position = self.extract_metadata('Cr-Commit-Position: ', pull_request.body) pr_commit_position = self._extract_metadata('Cr-Commit-Position: ', pull_request.body)
if position == pr_commit_position: if position == pr_commit_position:
return pull_request return pull_request
return None return None
def extract_metadata(self, tag, commit_body): def _extract_metadata(self, tag, commit_body):
for line in commit_body.splitlines(): for line in commit_body.splitlines():
if line.startswith(tag): if line.startswith(tag):
return line[len(tag):] return line[len(tag):]
......
...@@ -32,25 +32,20 @@ class MockWPTGitHub(object): ...@@ -32,25 +32,20 @@ class MockWPTGitHub(object):
def create_pr(self, remote_branch_name, desc_title, body): def create_pr(self, remote_branch_name, desc_title, body):
self.calls.append('create_pr') self.calls.append('create_pr')
assert remote_branch_name
assert desc_title
assert body
if self.create_pr_fail_index != self.create_pr_index: if self.create_pr_fail_index != self.create_pr_index:
self.pull_requests_created.append((remote_branch_name, desc_title, body)) self.pull_requests_created.append((remote_branch_name, desc_title, body))
self.create_pr_index += 1 self.create_pr_index += 1
return {'number': 5678} return {}
def update_pr(self, pr_number, desc_title, body): # pylint: disable=unused-argument
self.calls.append('update_pr')
return {'number': 5678}
def delete_remote_branch(self, _): def delete_remote_branch(self, _):
self.calls.append('delete_remote_branch') self.calls.append('delete_remote_branch')
def add_label(self, _):
self.calls.append('add_label')
return {}, 200
def get_pr_branch(self, number): def get_pr_branch(self, number):
self.calls.append('get_pr_branch') self.calls.append('get_pr_branch')
return 'fake branch for PR {}'.format(number) return 'fake branch for PR {}'.format(number)
...@@ -66,9 +61,3 @@ class MockWPTGitHub(object): ...@@ -66,9 +61,3 @@ class MockWPTGitHub(object):
for pr in self.pull_requests: for pr in self.pull_requests:
if change_id in pr.body: if change_id in pr.body:
return pr return pr
def extract_metadata(self, tag, commit_body):
for line in commit_body.splitlines():
if line.startswith(tag):
return line[len(tag):]
return None
...@@ -44,46 +44,34 @@ def main(): ...@@ -44,46 +44,34 @@ def main():
help='Gerrit username (found on settings page or in .gitcookies).') help='Gerrit username (found on settings page or in .gitcookies).')
parser.add_argument('--gerrit-token', default=None, parser.add_argument('--gerrit-token', default=None,
help='Gerrit API key (found on settings page or in .gitcookies).') help='Gerrit API key (found on settings page or in .gitcookies).')
# TODO(jeffcarp): Remove this after SSHing into wpt-exporter and updating file
parser.add_argument( parser.add_argument(
'--github-credentials-json', '--github-credentials-json',
help='Deprecated. Use --credentials-json.') help='A JSON file with schema {"GH_USER": "", "GH_TOKEN": ""}. '
'This will override credentials that were passed via command line or env var.')
parser.add_argument(
'--credentials-json',
help='A JSON file with an object containing zero or more of the following '
'keys that can override command line flags: '
'GH_USER, GH_TOKEN, GERRIT_USER, GERRIT_TOKEN')
args = parser.parse_args() args = parser.parse_args()
host = Host() host = Host()
credentials = { gh_user = args.gh_user
'GH_USER': args.gh_user, gh_token = args.gh_token
'GH_TOKEN': args.gh_token,
}
credentials_json = None
if not gh_user:
gh_user = host.environ.get('GH_USER')
if not gh_token:
gh_token = host.environ.get('GH_TOKEN')
if args.github_credentials_json: if args.github_credentials_json:
credentials_json = args.github_credentials_json with open(args.github_credentials_json, 'r') as f:
if args.credentials_json:
credentials_json = args.credentials_json
if credentials_json:
with open(credentials_json, 'r') as f:
contents = json.load(f) contents = json.load(f)
for key in ('GH_USER', 'GH_TOKEN', 'GERRIT_USER', 'GERRIT_TOKEN'): gh_user = contents['GH_USER']
if key in contents: gh_token = contents['GH_TOKEN']
credentials[key] = contents[key]
if not (credentials['GH_USER'] and credentials['GH_TOKEN']): if not (gh_user and gh_token):
parser.error('Must provide both gh_user and gh_token for GitHub.') parser.error('Must provide both gh_user and gh_token for GitHub.')
TestExporter( TestExporter(
host=host, host,
gh_user=credentials['GH_USER'], gh_user,
gh_token=credentials['GH_TOKEN'], gh_token,
gerrit_user=credentials['GERRIT_USER'], args.gerrit_user,
gerrit_token=credentials['GERRIT_TOKEN'], args.gerrit_token,
dry_run=args.dry_run dry_run=args.dry_run
).run() ).run()
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment