Commit f3a0ac6c authored by Robert Ma's avatar Robert Ma Committed by Commit Bot

[WPT export] Add Change-Id back if missing

Change-Id can be accidentally deleted from an in-flight CL *, in which
case we need to add it back so that the exporter can find the export PR
later.

* https://crbug.com/gerrit/12244

Bug: 1032044
Change-Id: I9796e4a1e13dc8e2f617d9e45e7d2cb1a4ee61f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2065132
Auto-Submit: Robert Ma <robertma@chromium.org>
Commit-Queue: Philip Jägenstedt <foolip@chromium.org>
Reviewed-by: default avatarPhilip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743905}
parent e24ddcde
...@@ -44,6 +44,8 @@ class MockChromiumCommit(object): ...@@ -44,6 +44,8 @@ class MockChromiumCommit(object):
def body(self): def body(self):
# The final newline is intentionally added to match the real behavior. # The final newline is intentionally added to match the real behavior.
if not self.change_id():
return self._body + '\n'
return self._body + '\n\nChange-Id: ' + self.change_id() + '\n' return self._body + '\n\nChange-Id: ' + self.change_id() + '\n'
def message(self): def message(self):
......
...@@ -15,7 +15,8 @@ WPT_GH_REPO_NAME = 'wpt' ...@@ -15,7 +15,8 @@ WPT_GH_REPO_NAME = 'wpt'
WPT_GH_URL = 'https://github.com/%s/%s/' % (WPT_GH_ORG, WPT_GH_REPO_NAME) WPT_GH_URL = 'https://github.com/%s/%s/' % (WPT_GH_ORG, WPT_GH_REPO_NAME)
WPT_MIRROR_URL = 'https://chromium.googlesource.com/external/github.com/web-platform-tests/wpt.git' WPT_MIRROR_URL = 'https://chromium.googlesource.com/external/github.com/web-platform-tests/wpt.git'
WPT_GH_SSH_URL_TEMPLATE = 'https://{}@github.com/%s/%s.git' % (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:' WPT_REVISION_FOOTER = 'WPT-Export-Revision: '
CHANGE_ID_FOOTER = 'Change-Id: '
EXPORT_PR_LABEL = 'chromium-export' EXPORT_PR_LABEL = 'chromium-export'
PROVISIONAL_PR_LABEL = 'do not merge yet' PROVISIONAL_PR_LABEL = 'do not merge yet'
......
...@@ -65,8 +65,7 @@ class ExportNotifier(object): ...@@ -65,8 +65,7 @@ class ExportNotifier(object):
'Can not retrieve Change-Id for %s.', pr.number) 'Can not retrieve Change-Id for %s.', pr.number)
continue continue
gerrit_sha = self.wpt_github.extract_metadata( gerrit_sha = self.wpt_github.extract_metadata(WPT_REVISION_FOOTER, pr.body)
WPT_REVISION_FOOTER + ' ', pr.body)
gerrit_dict[gerrit_id] = PRStatusInfo( gerrit_dict[gerrit_id] = PRStatusInfo(
taskcluster_status['node_id'], taskcluster_status['node_id'],
taskcluster_status['target_url'], taskcluster_status['target_url'],
......
...@@ -11,11 +11,12 @@ from blinkpy.common.system.log_utils import configure_logging ...@@ -11,11 +11,12 @@ from blinkpy.common.system.log_utils import configure_logging
from blinkpy.w3c.local_wpt import LocalWPT from blinkpy.w3c.local_wpt import LocalWPT
from blinkpy.w3c.chromium_exportable_commits import exportable_commits_over_last_n_commits from blinkpy.w3c.chromium_exportable_commits import exportable_commits_over_last_n_commits
from blinkpy.w3c.common import ( from blinkpy.w3c.common import (
CHANGE_ID_FOOTER,
WPT_GH_URL, WPT_GH_URL,
WPT_REVISION_FOOTER, WPT_REVISION_FOOTER,
EXPORT_PR_LABEL, EXPORT_PR_LABEL,
PROVISIONAL_PR_LABEL, PROVISIONAL_PR_LABEL,
read_credentials read_credentials,
) )
from blinkpy.w3c.gerrit import GerritAPI, GerritCL, GerritError from blinkpy.w3c.gerrit import GerritAPI, GerritCL, GerritError
from blinkpy.w3c.wpt_github import WPTGitHub, MergeError from blinkpy.w3c.wpt_github import WPTGitHub, MergeError
...@@ -135,7 +136,7 @@ class TestExporter(object): ...@@ -135,7 +136,7 @@ class TestExporter(object):
pr_url = '{}pull/{}'.format(WPT_GH_URL, pull_request.number) pr_url = '{}pull/{}'.format(WPT_GH_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) pr_cl_revision = self.wpt_github.extract_metadata(WPT_REVISION_FOOTER, pull_request.body)
if cl.current_revision_sha == pr_cl_revision: if cl.current_revision_sha == pr_cl_revision:
_log.info('PR revision matches CL revision. Nothing to do here.') _log.info('PR revision matches CL revision. Nothing to do here.')
return return
...@@ -214,7 +215,7 @@ class TestExporter(object): ...@@ -214,7 +215,7 @@ class TestExporter(object):
try: try:
self.wpt_github.merge_pr(pull_request.number) self.wpt_github.merge_pr(pull_request.number)
change_id = self.wpt_github.extract_metadata('Change-Id: ', pull_request.body) change_id = self.wpt_github.extract_metadata(CHANGE_ID_FOOTER, pull_request.body)
if change_id: if change_id:
cl = GerritCL(data={'change_id': change_id}, api=self.gerrit) cl = GerritCL(data={'change_id': change_id}, api=self.gerrit)
pr_url = '{}pull/{}'.format(WPT_GH_URL, pull_request.number) pr_url = '{}pull/{}'.format(WPT_GH_URL, pull_request.number)
...@@ -261,11 +262,18 @@ class TestExporter(object): ...@@ -261,11 +262,18 @@ class TestExporter(object):
_log.debug('END_OF_PATCH_EXCERPT') _log.debug('END_OF_PATCH_EXCERPT')
return return
footer = ''
# Change-Id can be deleted from the body of an in-flight CL in Chromium
# (https://crbug.com/gerrit/12244). We need to add it back. And we've
# asserted that cl.change_id is present in GerritCL.
if not self.wpt_github.extract_metadata(CHANGE_ID_FOOTER, commit.message()):
_log.warn('Adding missing Change-Id back to %s', cl.url)
footer += '{}{}\n'.format(CHANGE_ID_FOOTER, cl.change_id)
# Reviewed-on footer is not in the git commit message of in-flight CLs, # Reviewed-on footer is not in the git commit message of in-flight CLs,
# but a link to code review is useful so we add it manually. # but a link to code review is useful so we add it manually.
footer = 'Reviewed-on: {}\n'.format(cl.url) footer += 'Reviewed-on: {}\n'.format(cl.url)
# WPT_REVISION_FOOTER is used by the exporter to check the CL revision. # WPT_REVISION_FOOTER is used by the exporter to check the CL revision.
footer += '{} {}'.format(WPT_REVISION_FOOTER, cl.current_revision_sha) footer += '{}{}'.format(WPT_REVISION_FOOTER, cl.current_revision_sha)
if pull_request: if pull_request:
pr_number = self.create_or_update_pr_from_commit( pr_number = self.create_or_update_pr_from_commit(
......
...@@ -197,22 +197,44 @@ class TestExporterTest(LoggingTestCase): ...@@ -197,22 +197,44 @@ class TestExporterTest(LoggingTestCase):
test_exporter.wpt_github = MockWPTGitHub(pull_requests=[]) test_exporter.wpt_github = MockWPTGitHub(pull_requests=[])
test_exporter.get_exportable_commits = lambda: ([], []) test_exporter.get_exportable_commits = lambda: ([], [])
test_exporter.gerrit = MockGerritAPI() test_exporter.gerrit = MockGerritAPI()
test_exporter.gerrit.exportable_open_cls = [MockGerritCL( test_exporter.gerrit.exportable_open_cls = [
data={ MockGerritCL(
'change_id': 'I001', data={
'subject': 'subject', 'change_id': 'I001',
'_number': 1234, 'subject': 'subject',
'current_revision': '1', '_number': 1234,
'has_review_started': True, 'current_revision': '1',
'revisions': { 'has_review_started': True,
'1': {'commit_with_footers': 'a commit with footers'} 'revisions': {
'1': {
'commit_with_footers': 'a commit with footers'
}
},
'owner': {
'email': 'test@chromium.org'
},
}, },
'owner': {'email': 'test@chromium.org'}, api=test_exporter.gerrit,
}, chromium_commit=MockChromiumCommit(self.host, subject='subject', body='fake body <html>', change_id='I001')),
api=test_exporter.gerrit, MockGerritCL(
chromium_commit=MockChromiumCommit(self.host, subject='subject', data={
body='fake body <faketag1>, <faketag2>', change_id='I001') 'change_id': 'I002',
)] 'subject': 'subject',
'_number': 1235,
'current_revision': '1',
'has_review_started': True,
'revisions': {
'1': {
'commit_with_footers': 'a commit with footers'
}
},
'owner': {
'email': 'test@chromium.org'
},
},
api=test_exporter.gerrit,
chromium_commit=MockChromiumCommit(self.host, subject='subject', body='body', change_id=None)),
]
test_exporter.main(['--credentials-json', '/tmp/credentials.json']) test_exporter.main(['--credentials-json', '/tmp/credentials.json'])
self.assertEqual(test_exporter.wpt_github.calls, [ self.assertEqual(test_exporter.wpt_github.calls, [
...@@ -220,12 +242,18 @@ class TestExporterTest(LoggingTestCase): ...@@ -220,12 +242,18 @@ class TestExporterTest(LoggingTestCase):
'create_pr', 'create_pr',
'add_label "chromium-export"', 'add_label "chromium-export"',
'add_label "do not merge yet"', 'add_label "do not merge yet"',
'pr_with_change_id',
'create_pr',
'add_label "chromium-export"',
'add_label "do not merge yet"',
]) ])
self.assertEqual(test_exporter.wpt_github.pull_requests_created, [ self.assertEqual(test_exporter.wpt_github.pull_requests_created, [
('chromium-export-cl-1234', ('chromium-export-cl-1234', 'subject', 'fake body \\<html>\n\nChange-Id: I001\nReviewed-on: '
'subject', 'https://chromium-review.googlesource.com/1234\n'
'fake body \\<faketag1>, \\<faketag2>\n\nChange-Id: I001\nReviewed-on: ' 'WPT-Export-Revision: 1'),
'https://chromium-review.googlesource.com/1234\nWPT-Export-Revision: 1'), ('chromium-export-cl-1235', 'subject', 'body\nChange-Id: I002\nReviewed-on: '
'https://chromium-review.googlesource.com/1235\n'
'WPT-Export-Revision: 1'),
]) ])
self.assertEqual(test_exporter.wpt_github.pull_requests_merged, []) self.assertEqual(test_exporter.wpt_github.pull_requests_merged, [])
......
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