Commit 00080bfa authored by Kyle Ju's avatar Kyle Ju Committed by Commit Bot

Revert "Prod Testing - open a backdoor to dump all export_notifier comments to a dummy CL."

This reverts commit 14dc6de7.

Reason for revert: Export Notifier didn't experience any issues in the prod testing and comments were posted correctly here, https://chromium-review.googlesource.com/c/chromium/src/+/2241634.

Original change's description:
> Prod Testing - open a backdoor to dump all export_notifier comments to a dummy CL.
>
> It will read the comment history and dump all comments to https://chromium-review.googlesource.com/c/chromium/src/+/2241634/. Also tweak the Gerrit CL SHA so that the SHA is unique identified to each CL.
>
> Testing command:
> third_party/blink/tools/wpt_export.py --credentials ~/.credentials --surface-failures-to-gerrit
>
> Output:
> https://chromium-review.googlesource.com/c/chromium/src/+/2241634
>
> Bug: 1027618
> Change-Id: I3a4932524488072cdbb820ccabdd4dd9a079cd3c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2242001
> Commit-Queue: Kyle Ju <kyleju@chromium.org>
> Reviewed-by: Robert Ma <robertma@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#777952}



Bug: 1027618
Change-Id: Iab53b6cbb2b6f57407c986983ce167cf02777379
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2261170Reviewed-by: default avatarRobert Ma <robertma@chromium.org>
Commit-Queue: Kyle Ju <kyleju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781554}
parent 7da83393
...@@ -89,21 +89,11 @@ class ExportNotifier(object): ...@@ -89,21 +89,11 @@ class ExportNotifier(object):
"""Processes and comments on CLs with failed TackCluster status.""" """Processes and comments on CLs with failed TackCluster status."""
_log.info('Processing %d CLs with failed Taskcluster status.', _log.info('Processing %d CLs with failed Taskcluster status.',
len(gerrit_dict)) len(gerrit_dict))
# Uses a dummy CL to test export_notifier on prod. The real comments will be dumped to
# https://chromium-review.googlesource.com/c/chromium/src/+/2241634.
try:
dummy_cl = self.gerrit.query_cl_comments_and_revisions(
'I4fd5039cd4ec991bb8f840eabe55574b37243ef2')
except GerritError as e:
_log.error('Could not process Dummy CL: %s', str(e))
return
for change_id, pr_status_info in gerrit_dict.items(): for change_id, pr_status_info in gerrit_dict.items():
try: try:
cl = self.gerrit.query_cl_comments_and_revisions(change_id) cl = self.gerrit.query_cl_comments_and_revisions(change_id)
has_commented = self.has_latest_taskcluster_status_commented( has_commented = self.has_latest_taskcluster_status_commented(
dummy_cl.messages, pr_status_info) cl.messages, pr_status_info)
if has_commented: if has_commented:
continue continue
...@@ -120,7 +110,7 @@ class ExportNotifier(object): ...@@ -120,7 +110,7 @@ class ExportNotifier(object):
_log.debug('Comments are:\n %s\n', cl_comment) _log.debug('Comments are:\n %s\n', cl_comment)
else: else:
_log.info('Commenting on CL %s\n', change_id) _log.info('Commenting on CL %s\n', change_id)
dummy_cl.post_comment(cl_comment) cl.post_comment(cl_comment)
except GerritError as e: except GerritError as e:
_log.error('Could not process Gerrit CL %s: %s', change_id, _log.error('Could not process Gerrit CL %s: %s', change_id,
str(e)) str(e))
...@@ -136,8 +126,7 @@ class ExportNotifier(object): ...@@ -136,8 +126,7 @@ class ExportNotifier(object):
""" """
for message in reversed(messages): for message in reversed(messages):
cl_gerrit_sha = PRStatusInfo.get_gerrit_sha_from_comment( cl_gerrit_sha = PRStatusInfo.get_gerrit_sha_from_comment(
message['message'], pr_status_info.pr_number) message['message'])
if cl_gerrit_sha: if cl_gerrit_sha:
return cl_gerrit_sha == pr_status_info.gerrit_sha return cl_gerrit_sha == pr_status_info.gerrit_sha
...@@ -207,11 +196,10 @@ class PRStatusInfo(object): ...@@ -207,11 +196,10 @@ class PRStatusInfo(object):
return self._gerrit_sha return self._gerrit_sha
@staticmethod @staticmethod
def get_gerrit_sha_from_comment(comment, pr_number): def get_gerrit_sha_from_comment(comment):
dummy_tag = str(pr_number) + PRStatusInfo.CL_SHA_TAG
for line in comment.splitlines(): for line in comment.splitlines():
if line.startswith(dummy_tag): if line.startswith(PRStatusInfo.CL_SHA_TAG):
return line[len(dummy_tag):] return line[len(PRStatusInfo.CL_SHA_TAG):]
return None return None
...@@ -224,8 +212,7 @@ class PRStatusInfo(object): ...@@ -224,8 +212,7 @@ class PRStatusInfo(object):
pr_url='%spull/%d' % (WPT_GH_URL, self.pr_number) pr_url='%spull/%d' % (WPT_GH_URL, self.pr_number)
) )
link_line = ('\n\n{}{}').format(PRStatusInfo.LINK_TAG, self.link) link_line = ('\n\n{}{}').format(PRStatusInfo.LINK_TAG, self.link)
dummy_tag = str(self.pr_number) + PRStatusInfo.CL_SHA_TAG sha_line = ('\n{}{}').format(PRStatusInfo.CL_SHA_TAG, self.gerrit_sha)
sha_line = ('\n{}{}').format(dummy_tag, self.gerrit_sha)
comment = status_line + link_line + sha_line comment = status_line + link_line + sha_line
if patchset is not None: if patchset is not None:
......
...@@ -22,14 +22,14 @@ class ExportNotifierTest(LoggingTestCase): ...@@ -22,14 +22,14 @@ class ExportNotifierTest(LoggingTestCase):
gerrit_comment = self.generate_notifier_comment( gerrit_comment = self.generate_notifier_comment(
123, 'bar', 'num', None) 123, 'bar', 'num', None)
actual = PRStatusInfo.get_gerrit_sha_from_comment(gerrit_comment, 123) actual = PRStatusInfo.get_gerrit_sha_from_comment(gerrit_comment)
self.assertEqual(actual, 'num') self.assertEqual(actual, 'num')
def test_get_gerrit_sha_from_comment_fail(self): def test_get_gerrit_sha_from_comment_fail(self):
gerrit_comment = 'ABC' gerrit_comment = 'ABC'
actual = PRStatusInfo.get_gerrit_sha_from_comment(gerrit_comment, 123) actual = PRStatusInfo.get_gerrit_sha_from_comment(gerrit_comment)
self.assertIsNone(actual) self.assertIsNone(actual)
...@@ -174,7 +174,7 @@ class ExportNotifierTest(LoggingTestCase): ...@@ -174,7 +174,7 @@ class ExportNotifierTest(LoggingTestCase):
self.notifier.process_failing_prs(gerrit_dict) self.notifier.process_failing_prs(gerrit_dict)
self.assertEqual(self.notifier.gerrit.cls_queried, ['I4fd5039cd4ec991bb8f840eabe55574b37243ef2', 'abc']) self.assertEqual(self.notifier.gerrit.cls_queried, ['abc'])
self.assertEqual(self.notifier.gerrit.request_posted, self.assertEqual(self.notifier.gerrit.request_posted,
[('/a/changes/abc/revisions/current/review', { [('/a/changes/abc/revisions/current/review', {
'message': expected 'message': expected
...@@ -186,7 +186,7 @@ class ExportNotifierTest(LoggingTestCase): ...@@ -186,7 +186,7 @@ class ExportNotifierTest(LoggingTestCase):
self.notifier.gerrit.cl = MockGerritCL( self.notifier.gerrit.cl = MockGerritCL(
data={ data={
'change_id': 'change_id':
'I4fd5039cd4ec991bb8f840eabe55574b37243ef2', 'abc',
'messages': [ 'messages': [
{ {
"date": "2019-08-20 17:42:05.000000000", "date": "2019-08-20 17:42:05.000000000",
...@@ -212,7 +212,7 @@ class ExportNotifierTest(LoggingTestCase): ...@@ -212,7 +212,7 @@ class ExportNotifierTest(LoggingTestCase):
self.notifier.process_failing_prs(gerrit_dict) self.notifier.process_failing_prs(gerrit_dict)
self.assertEqual(self.notifier.gerrit.cls_queried, ['I4fd5039cd4ec991bb8f840eabe55574b37243ef2', 'abc']) self.assertEqual(self.notifier.gerrit.cls_queried, ['abc'])
self.assertEqual(self.notifier.gerrit.request_posted, []) self.assertEqual(self.notifier.gerrit.request_posted, [])
def test_process_failing_prs_with_latest_sha(self): def test_process_failing_prs_with_latest_sha(self):
...@@ -248,7 +248,7 @@ class ExportNotifierTest(LoggingTestCase): ...@@ -248,7 +248,7 @@ class ExportNotifierTest(LoggingTestCase):
self.notifier.process_failing_prs(gerrit_dict) self.notifier.process_failing_prs(gerrit_dict)
self.assertEqual(self.notifier.gerrit.cls_queried, ['I4fd5039cd4ec991bb8f840eabe55574b37243ef2', 'abc']) self.assertEqual(self.notifier.gerrit.cls_queried, ['abc'])
self.assertEqual(self.notifier.gerrit.request_posted, self.assertEqual(self.notifier.gerrit.request_posted,
[('/a/changes/abc/revisions/current/review', { [('/a/changes/abc/revisions/current/review', {
'message': expected 'message': expected
...@@ -261,11 +261,11 @@ class ExportNotifierTest(LoggingTestCase): ...@@ -261,11 +261,11 @@ class ExportNotifierTest(LoggingTestCase):
self.notifier.process_failing_prs(gerrit_dict) self.notifier.process_failing_prs(gerrit_dict)
self.assertEqual(self.notifier.gerrit.cls_queried, ['I4fd5039cd4ec991bb8f840eabe55574b37243ef2']) self.assertEqual(self.notifier.gerrit.cls_queried, ['abc'])
self.assertEqual(self.notifier.gerrit.request_posted, []) self.assertEqual(self.notifier.gerrit.request_posted, [])
self.assertLog([ self.assertLog([
'INFO: Processing 1 CLs with failed Taskcluster status.\n', 'INFO: Processing 1 CLs with failed Taskcluster status.\n',
'ERROR: Could not process Dummy CL: Error from query_cl\n' 'ERROR: Could not process Gerrit CL abc: Error from query_cl\n'
]) ])
def test_export_notifier_success(self): def test_export_notifier_success(self):
...@@ -323,7 +323,7 @@ class ExportNotifierTest(LoggingTestCase): ...@@ -323,7 +323,7 @@ class ExportNotifierTest(LoggingTestCase):
'get_pr_branch', 'get_pr_branch',
'get_branch_statuses', 'get_branch_statuses',
]) ])
self.assertEqual(self.notifier.gerrit.cls_queried, ['I4fd5039cd4ec991bb8f840eabe55574b37243ef2', 'decafbad']) self.assertEqual(self.notifier.gerrit.cls_queried, ['decafbad'])
self.assertEqual(self.notifier.gerrit.request_posted, self.assertEqual(self.notifier.gerrit.request_posted,
[('/a/changes/decafbad/revisions/current/review', { [('/a/changes/decafbad/revisions/current/review', {
'message': expected 'message': expected
...@@ -337,11 +337,11 @@ class ExportNotifierTest(LoggingTestCase): ...@@ -337,11 +337,11 @@ class ExportNotifierTest(LoggingTestCase):
'cross-browser failures on the exported changes. Please contact ' 'cross-browser failures on the exported changes. Please contact '
'ecosystem-infra@chromium.org for more information.\n\n' 'ecosystem-infra@chromium.org for more information.\n\n'
'Taskcluster Link: {}\n' 'Taskcluster Link: {}\n'
'{}Gerrit CL SHA: {}\n' 'Gerrit CL SHA: {}\n'
'Patchset Number: {}' 'Patchset Number: {}'
'\n\nAny suggestions to improve this service are welcome; ' '\n\nAny suggestions to improve this service are welcome; '
'crbug.com/1027618.').format( 'crbug.com/1027618.').format(
pr_number, link, pr_number, sha, patchset pr_number, link, sha, patchset
) )
else: else:
comment = ( comment = (
...@@ -350,9 +350,9 @@ class ExportNotifierTest(LoggingTestCase): ...@@ -350,9 +350,9 @@ class ExportNotifierTest(LoggingTestCase):
'cross-browser failures on the exported changes. Please contact ' 'cross-browser failures on the exported changes. Please contact '
'ecosystem-infra@chromium.org for more information.\n\n' 'ecosystem-infra@chromium.org for more information.\n\n'
'Taskcluster Link: {}\n' 'Taskcluster Link: {}\n'
'{}Gerrit CL SHA: {}' 'Gerrit CL SHA: {}'
'\n\nAny suggestions to improve this service are welcome; ' '\n\nAny suggestions to improve this service are welcome; '
'crbug.com/1027618.').format( 'crbug.com/1027618.').format(
pr_number, link, pr_number, sha pr_number, link, sha
) )
return comment return comment
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