llvm_tools: use tiny_render in the nightly revert checker
Now that we have `tiny_render` imported into `cros_utils`, we can use
it. This lets us easily linkify and stylize our emails, and makes
comments like tcwang's in Ibc3ef1d2e3b5a3301366e971cacf53396a6ca2aa much
easier to address.
BUG=chromium:1046988
TEST=unittests; sent an email
Change-Id: I538efd2e349a48bd433c826359eb0dd94b422966
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/2169010
Reviewed-by: Tiancong Wang <tcwang@google.com>
Tested-by: George Burgess <gbiv@chromium.org>
diff --git a/llvm_tools/nightly_revert_checker.py b/llvm_tools/nightly_revert_checker.py
index b7b87fa..24130e0 100755
--- a/llvm_tools/nightly_revert_checker.py
+++ b/llvm_tools/nightly_revert_checker.py
@@ -24,6 +24,8 @@
import typing as t
import cros_utils.email_sender as email_sender
+import cros_utils.tiny_render as tiny_render
+import get_llvm_hash
import revert_checker
State = t.Any
@@ -77,35 +79,46 @@
_Email = t.NamedTuple('_Email', [
('subject', str),
- ('body', str),
+ ('body', tiny_render.Piece),
])
def _generate_revert_email(
friendly_name: str, sha: str,
+ prettify_sha: t.Callable[[str], tiny_render.Piece],
new_reverts: t.List[revert_checker.Revert]) -> _Email:
- email_lines = [
- 'It looks like there may be %s across %s (%s).' % (
+ email_pieces = [
+ 'It looks like there may be %s across %s (' % (
'a new revert' if len(new_reverts) == 1 else 'new reverts',
friendly_name,
- sha,
),
- '',
+ prettify_sha(sha),
+ ').',
+ tiny_render.line_break,
+ tiny_render.line_break,
'That is:' if len(new_reverts) == 1 else 'These are:',
]
+ revert_listing = []
for revert in sorted(new_reverts, key=lambda r: r.sha):
- email_lines.append(
- '\t- %s (appears to revert %s)' % (revert.sha, revert.reverted_sha))
+ revert_listing.append([
+ prettify_sha(revert.sha),
+ ' (appears to revert ',
+ prettify_sha(revert.reverted_sha),
+ ')',
+ ])
- email_lines.append('')
- email_lines.append('PTAL and consider reverting them locally.')
+ email_pieces.append(tiny_render.UnorderedList(items=revert_listing))
+ email_pieces += [
+ tiny_render.line_break,
+ 'PTAL and consider reverting them locally.',
+ ]
return _Email(
subject='[revert-checker] new %s discovered across %s' % (
'revert' if len(new_reverts) == 1 else 'reverts',
friendly_name,
),
- body='\n'.join(email_lines),
+ body=email_pieces,
)
@@ -113,9 +126,11 @@
email_sender.EmailSender().SendX20Email(
subject=email.subject,
identifier='revert-checker',
- well_known_recipients=['mage'],
+ well_known_recipients=[],
direct_recipients=['gbiv@google.com'],
- text_body=email.body)
+ text_body=tiny_render.render_text_pieces(email.body),
+ html_body=tiny_render.render_html_pieces(email.body),
+ )
def _write_state(state_file: str, new_state: State) -> None:
@@ -165,19 +180,31 @@
state_file = opts.state_file
dry_run = opts.dry_run
+ llvm_dir = opts.llvm_dir
state = _read_state(state_file)
interesting_shas = _find_interesting_shas(opts.chromeos_dir)
logging.info('Interesting SHAs were %r', interesting_shas)
+ def prettify_sha(sha: str) -> tiny_render.Piece:
+ rev = get_llvm_hash.GetVersionFrom(llvm_dir, sha)
+
+ # 12 is arbitrary, but should be unambiguous enough.
+ short_sha = sha[:12]
+ return tiny_render.Switch(
+ text='r%s (%s)' % (rev, short_sha),
+ html=tiny_render.Link(
+ href='https://reviews.llvm.org/rG' + sha, inner='r' + str(rev)),
+ )
+
new_state: State = {}
revert_emails_to_send: t.List[t.Tuple[str, t.List[revert_checker
.Revert]]] = []
for friendly_name, sha in interesting_shas:
logging.info('Finding reverts across %s (%s)', friendly_name, sha)
all_reverts = revert_checker.find_reverts(
- opts.llvm_dir, sha, root='origin/master')
+ llvm_dir, sha, root='origin/master')
logging.info('Detected the following revert(s) across %s:\n%s',
friendly_name, pprint.pformat(all_reverts))
@@ -195,14 +222,14 @@
continue
revert_emails_to_send.append(
- _generate_revert_email(friendly_name, sha, new_reverts))
+ _generate_revert_email(friendly_name, sha, prettify_sha, new_reverts))
# We want to be as free of obvious side-effects as possible in case something
# above breaks. Hence, send the email as late as possible.
for email in revert_emails_to_send:
if dry_run:
logging.info('Would send email:\nSubject: %s\nBody:\n%s\n', email.subject,
- email.body)
+ tiny_render.render_text_pieces(email.body))
else:
logging.info('Sending email with subject %r...', email.subject)
_send_revert_email(email)
diff --git a/llvm_tools/nightly_revert_checker_test.py b/llvm_tools/nightly_revert_checker_test.py
index 81cfbd2..dce2934 100755
--- a/llvm_tools/nightly_revert_checker_test.py
+++ b/llvm_tools/nightly_revert_checker_test.py
@@ -8,22 +8,28 @@
from __future__ import print_function
-# pylint: disable=protected-access
-
import io
import unittest
+import cros_utils.tiny_render as tiny_render
import nightly_revert_checker
import revert_checker
+# pylint: disable=protected-access
+
class Test(unittest.TestCase):
"""Tests for nightly_revert_checker."""
def test_email_rendering_works_for_singular_revert(self):
+
+ def prettify_sha(sha: str) -> tiny_render.Piece:
+ return 'pretty_' + sha
+
email = nightly_revert_checker._generate_revert_email(
friendly_name='${name}',
sha='${sha}',
+ prettify_sha=prettify_sha,
new_reverts=[
revert_checker.Revert(
sha='${revert_sha}', reverted_sha='${reverted_sha}')
@@ -31,21 +37,34 @@
expected_email = nightly_revert_checker._Email(
subject='[revert-checker] new revert discovered across ${name}',
- body='\n'.join((
- 'It looks like there may be a new revert across ${name} (${sha}).',
- '',
+ body=[
+ 'It looks like there may be a new revert across ${name} (',
+ 'pretty_${sha}',
+ ').',
+ tiny_render.line_break,
+ tiny_render.line_break,
'That is:',
- '\t- ${revert_sha} (appears to revert ${reverted_sha})',
- '',
+ tiny_render.UnorderedList([[
+ 'pretty_${revert_sha}',
+ ' (appears to revert ',
+ 'pretty_${reverted_sha}',
+ ')',
+ ]]),
+ tiny_render.line_break,
'PTAL and consider reverting them locally.',
- )))
+ ])
self.assertEqual(email, expected_email)
def test_email_rendering_works_for_multiple_reverts(self):
+
+ def prettify_sha(sha: str) -> tiny_render.Piece:
+ return 'pretty_' + sha
+
email = nightly_revert_checker._generate_revert_email(
friendly_name='${name}',
sha='${sha}',
+ prettify_sha=prettify_sha,
new_reverts=[
revert_checker.Revert(
sha='${revert_sha1}', reverted_sha='${reverted_sha1}'),
@@ -58,16 +77,36 @@
expected_email = nightly_revert_checker._Email(
subject='[revert-checker] new reverts discovered across ${name}',
- body='\n'.join((
- 'It looks like there may be new reverts across ${name} (${sha}).',
- '',
+ body=[
+ 'It looks like there may be new reverts across ${name} (',
+ 'pretty_${sha}',
+ ').',
+ tiny_render.line_break,
+ tiny_render.line_break,
'These are:',
- '\t- ${revert_sha0} (appears to revert ${reverted_sha0})',
- '\t- ${revert_sha1} (appears to revert ${reverted_sha1})',
- '\t- ${revert_sha2} (appears to revert ${reverted_sha2})',
- '',
+ tiny_render.UnorderedList([
+ [
+ 'pretty_${revert_sha0}',
+ ' (appears to revert ',
+ 'pretty_${reverted_sha0}',
+ ')',
+ ],
+ [
+ 'pretty_${revert_sha1}',
+ ' (appears to revert ',
+ 'pretty_${reverted_sha1}',
+ ')',
+ ],
+ [
+ 'pretty_${revert_sha2}',
+ ' (appears to revert ',
+ 'pretty_${reverted_sha2}',
+ ')',
+ ],
+ ]),
+ tiny_render.line_break,
'PTAL and consider reverting them locally.',
- )))
+ ])
self.assertEqual(email, expected_email)
