Reland "Implement auto command in the update_wpr script"

This is a reland of 68ef4428

Original change's description:
> Implement auto command in the update_wpr script
>
> R=perezju@chromium.org
>
> Bug: 895891
> Change-Id: Ib4b6b0ea123b64d0333cb0911930358dcb3ff2ad
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1496871
> Commit-Queue: Sergiy Belozorov <sergiyb@chromium.org>
> Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#637677}

Bug: 895891, 938487
Change-Id: I4dbf7e1adee233598a78772024d71dfebc23e562
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1504014Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Commit-Queue: Sergiy Belozorov <sergiyb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638049}

tools/perf/cli_tools/update_wpr/update_wpr.py

@@ -9,10 +9,13 @@ import argparse
 import datetime
 import json
 import os
+import random
 import re
 import shutil
 import subprocess
 import tempfile
+import time
+import webbrowser
 
 from core import cli_helpers
 from core import path_util
@@ -24,7 +27,7 @@ from services import request  # pylint: disable=import-error
 
 
 SRC_ROOT = os.path.abspath(
-    os.path.join(os.path.dirname(__file__), '..', '..', '..'))
+    os.path.join(os.path.dirname(__file__), '..', '..', '..', '..'))
 RESULTS2JSON = os.path.join(
     SRC_ROOT, 'third_party', 'catapult', 'tracing', 'bin', 'results2json')
 HISTOGRAM2CSV = os.path.join(
@@ -35,6 +38,41 @@ RECORD_WPR = os.path.join(SRC_ROOT, 'tools', 'perf', 'record_wpr')
 DEFAULT_REVIEWERS = ['perezju@chromium.org']
 
 
+def _GetBranchName():
+  return subprocess.check_output(['git', 'rev-parse', '--abbrev-ref', 'HEAD'])
+
+
+def _OpenBrowser(url):
+  # Redirect I/O before invoking browser to avoid it spamming our output.
+  # Based on https://stackoverflow.com/a/2323563.
+  savout = os.dup(1)
+  saverr = os.dup(2)
+  os.close(1)
+  os.close(2)
+  os.open(os.devnull, os.O_RDWR)
+  try:
+    webbrowser.open(url)
+  finally:
+    os.dup2(savout, 1)
+    os.dup2(saverr, 2)
+
+
+def _SendCLForReview(comment):
+  subprocess.check_call(
+      ['git', 'cl', 'comments', '--publish', '--add-comment', comment])
+
+
+def _EnsureEditor():
+  if 'EDITOR' not in os.environ:
+    os.environ['EDITOR'] = cli_helpers.Prompt(
+        'Looks like EDITOR environment varible is not defined. Please enter '
+        'the command to view logs: ')
+
+
+def _OpenEditor(filepath):
+  subprocess.check_call([os.environ['EDITOR'], filepath])
+
+
 class WprUpdater(object):
   def __init__(self, args):
     self.story = args.story
@@ -226,6 +264,48 @@ class WprUpdater(object):
     with open(output_file, 'r') as output_fd:
       return json.load(output_fd)['issue_url']
 
+  def _CreateBranch(self):
+    sanitized_story = re.sub(r'[^A-Za-z0-9-_.]', r'-', self.story)
+    subprocess.check_call([
+      'git', 'new-branch',
+      'update-wpr-%s-%d' % (sanitized_story, random.randint(0, 10000)),
+    ])
+
+  def _FilterLogForDiff(self, log_filename):
+    """Removes unimportant details from console logs for cleaner diffs.
+
+    For example, log line from file `log_filename`
+
+      2018-02-01 22:23:22,123 operation abcdef01-abcd-abcd-0123-abcdef012345
+      from /tmp/tmpX34v/results.html took 22145ms when accessed via
+      https://127.0.0.1:1233/endpoint
+
+    would become
+
+      <timestamp> operation <guid> from /tmp/tmp<random>/results.html took
+      <duration> when accessed via https://127.0.0.1:<port>/endpoint
+
+    Returns:
+      Path to the filtered log.
+    """
+    with open(log_filename) as src, tempfile.NamedTemporaryFile(
+        suffix='diff', dir=self.output_dir, delete=False) as dest:
+      for line in src:
+        # Remove timestamps.
+        line = re.sub(
+            r'\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2},\d{3}', r'<timestamp>', line)
+        # Remove GUIDs.
+        line = re.sub(
+            r'[0-9a-f]{8}(?:-[0-9a-f]{4}){3}-[0-9a-f]{12}', r'<guid>', line)
+        # Remove random letters in paths to temp dirs and files.
+        line = re.sub(r'(/tmp/tmp)[^/\s]+', r'\1<random>', line)
+        # Remove random port in localhost URLs.
+        line = re.sub(r'(://127.0.0.1:)\d+', r'\1<port>', line)
+        # Remove random durations in ms.
+        line = re.sub(r'\d+ ms', r'<duration>', line)
+        dest.write(line)
+        return dest.name
+
   def _StartPinpointJob(self, configuration):
     """Creates, starts a Pinpoint job and returns its URL."""
     try:
@@ -328,6 +408,153 @@ class WprUpdater(object):
         job_urls.append(job_url)
     return job_urls, failed_configs
 
+  def AutoRun(self):
+    # Let the quest begin...
+    cli_helpers.Comment(
+        'This script will help you update the recording of a story. It will go '
+        'through the following stages, which you can also invoke manually via '
+        'subcommand specified in parentheses:')
+    cli_helpers.Comment('  - help create a new branch if needed')
+    cli_helpers.Comment('  - run story with live network connection (live)')
+    cli_helpers.Comment('  - record story (record)')
+    cli_helpers.Comment('  - replay the recording (replay)')
+    cli_helpers.Comment('  - upload the recording to Google Storage (upload)')
+    cli_helpers.Comment(
+        '  - upload CL with updated recording reference (review)')
+    cli_helpers.Comment('  - trigger pinpoint tryjobs (pinpoint)')
+    cli_helpers.Comment('  - post links to these jobs on the CL')
+    cli_helpers.Comment(
+        'Note that you can always enter prefix of the answer to any of the '
+        'questions asked below, e.g. "y" for "yes" or "j" for "just-replay".')
+
+    # TODO(sergiyb): Detect if benchmark is not implemented and try to add it
+    # automatically by copying the same benchmark without :<current-year> suffix
+    # and changing name of the test, name of the benchmark and the year tag.
+
+    # Create branch if needed.
+    reuse_cl = False
+    branch = _GetBranchName()
+    if branch == 'HEAD':
+      cli_helpers.Comment('You are not on a branch.')
+      if not cli_helpers.Ask(
+          'Should script create a new branch automatically?'):
+        cli_helpers.Comment(
+            'Please create a new branch and start this script again')
+        return
+      self._CreateBranch()
+    else:
+      issue = self._GetBranchIssueUrl()
+      if issue is not None:
+        issue_message = 'with an associated issue: %s' % issue
+      else:
+        issue_message = 'without an associated issue'
+      cli_helpers.Comment(
+          'You are on a branch {branch} {issue_message}. Please commit or '
+          'stash any changes unrelated to the updated story before '
+          'proceeding.', branch=branch, issue_message=issue_message)
+      ans = cli_helpers.Ask(
+          'Should the script create a new branch automatically, reuse '
+          'existing one or exit?', answers=['create', 'reuse', 'exit'],
+          default='create')
+      if ans == 'create':
+        self._CreateBranch()
+      elif ans == 'reuse':
+        reuse_cl = issue is not None
+      elif ans == 'exit':
+        return
+
+    # Live run.
+    live_out_file = self.LiveRun()
+    cli_helpers.Comment(
+        'Please inspect the live run results above for any errors.')
+    ans = None
+    while ans != 'continue':
+      ans = cli_helpers.Ask(
+          'Should I continue with recording, view metric results in a browser, '
+          'view stdout/stderr output or stop?',
+          ['continue', 'metrics', 'output', 'stop'], default='continue')
+      if ans == 'stop':
+        cli_helpers.Comment(
+            'Please update the story class to resolve the observed issues and '
+            'then run this script again.')
+        return
+      elif ans == 'metrics':
+        _OpenBrowser('file://%s.results.html' % live_out_file)
+      elif ans == 'output':
+        _OpenEditor(live_out_file)
+
+    # Record & replay.
+    action = 'record'
+    while action != 'continue':
+      if action == 'record':
+        self.RecordWpr()
+      if action in ['record', 'just-replay']:
+        replay_out_file = self.ReplayWpr()
+        cli_helpers.Comment(
+            'Check that the console:error:all metrics above have low values '
+            'and are similar to the live run above.')
+      if action == 'diff':
+        diff_path = os.path.join(self.output_dir, 'live_replay.diff')
+        with open(diff_path, 'w') as diff_file:
+          subprocess.call([
+            'diff', '--color', self._FilterLogForDiff(live_out_file),
+            self._FilterLogForDiff(replay_out_file)], stdout=diff_file)
+        _OpenEditor(diff_path)
+      if action == 'stop':
+        return
+      action = cli_helpers.Ask(
+          'Should I record and replay again, just replay, continue with '
+          'uploading CL, stop and exit, or would you prefer to see diff '
+          'between live/replay console logs?',
+          ['record', 'just-replay', 'continue', 'stop', 'diff'],
+          default='continue')
+
+    # Upload WPR and create a WIP CL for the new story.
+    if not self.UploadWpr():
+      return
+    while self.UploadCL(short_description=reuse_cl) != 0:
+      if not cli_helpers.Ask('Upload failed. Should I try again?'):
+        return
+
+    # Gerrit needs some time to sync its backends, hence we sleep here for 5
+    # seconds. Otherwise, pinpoint app may get an answer that the CL that we've
+    # just uploaded does not exist yet.
+    cli_helpers.Comment(
+        'Waiting 20 seconds for the Gerrit backends to sync, so that Pinpoint '
+        'app can detect the newly-created CL.')
+    time.sleep(20)
+
+    # Trigger pinpoint jobs.
+    configs_to_trigger = None
+    job_urls = []
+    while True:
+      new_job_urls, configs_to_trigger = self.StartPinpointJobs(
+          configs_to_trigger)
+      job_urls.extend(new_job_urls)
+      if not configs_to_trigger or not cli_helpers.Ask(
+          'Do you want to try triggering the failed configs again?'):
+        break
+
+    if configs_to_trigger:
+      if not cli_helpers.Ask(
+          'Some jobs failed to trigger. Do you still want to send created '
+          'CL for review?', default='no'):
+        return
+
+    # Post a link to the triggered jobs, publish CL for review and open it.
+    _SendCLForReview(
+        'Started the following Pinpoint jobs:\n%s' %
+        '\n'.join('  - %s' % url for url in job_urls))
+    cli_helpers.Comment(
+        'Posted a message with Pinpoint job URLs on the CL and sent it for '
+        'review. Opening the CL in a browser...')
+    _OpenBrowser(self._GetBranchIssueUrl())
+
+    # Hooray, you won! :-)
+    cli_helpers.Comment(
+        'Thank you, you have successfully updated the recording for %s. Please '
+        'wait for LGTM and land the created CL.' % self.story)
+
 
 def Main(argv):
   parser = argparse.ArgumentParser()
@@ -351,15 +578,27 @@ def Main(argv):
       '--binary', default=None,
       help='Path to the Chromium/Chrome binary relative to output directory. '
            'Defaults to default Chrome browser installed if not specified.')
-  parser.add_argument(
-      'command', choices=[
-        'live', 'record', 'replay', 'upload', 'review', 'pinpoint'],
-      help='Mode in which to run this script.')
+
+  subparsers = parser.add_subparsers(
+      title='Mode in which to run this script', dest='command')
+  subparsers.add_parser(
+      'auto', help='interactive mode automating updating a recording')
+  subparsers.add_parser('live', help='run story on a live website')
+  subparsers.add_parser('record', help='record story from a live website')
+  subparsers.add_parser('replay', help='replay story from the recording')
+  subparsers.add_parser('upload', help='upload recording to the Google Storage')
+  subparsers.add_parser('review', help='create a CL with updated recording')
+  subparsers.add_parser(
+      'pinpoint', help='trigger Pinpoint jobs to test the recording')
 
   args = parser.parse_args(argv)
 
   updater = WprUpdater(args)
-  if args.command =='live':
+  if args.command == 'auto':
+    _EnsureEditor()
+    luci_auth.CheckLoggedIn()
+    updater.AutoRun()
+  elif args.command =='live':
     updater.LiveRun()
   elif args.command == 'record':
     updater.RecordWpr()

tools/perf/cli_tools/update_wpr/update_wpr_unittest.py

@@ -35,6 +35,7 @@ class UpdateWprTest(unittest.TestCase):
     datetime.now.return_value.strftime.return_value = '<tstamp>'
 
     mock.patch('tempfile.mkdtemp', return_value='/tmp/dir').start()
+    mock.patch('random.randint', return_value=1234).start()
     mock.patch('core.cli_helpers.Fatal').start()
     mock.patch('core.cli_helpers.Error').start()
     mock.patch('core.cli_helpers.Step').start()
@@ -47,6 +48,7 @@ class UpdateWprTest(unittest.TestCase):
     mock.patch(WPR_UPDATER + 'RECORD_WPR', '.../record_wpr').start()
     mock.patch('os.path.join', lambda *parts: '/'.join(parts)).start()
     mock.patch('os.path.exists', return_value=True).start()
+    mock.patch('time.sleep').start()
 
     self.wpr_updater = update_wpr.WprUpdater(argparse.Namespace(
       story='<story>', device_id=None, repeat=1, binary=None, bug_id=None,
@@ -58,13 +60,13 @@ class UpdateWprTest(unittest.TestCase):
   def testMain(self):
     wpr_updater_cls = mock.patch(WPR_UPDATER + 'WprUpdater').start()
     update_wpr.Main([
-      'live',
       '-s', 'foo:bar:story:2019',
       '-d', 'H2345234FC33',
       '--binary', '<binary>',
       '-b', '1234',
       '-r', 'test_user1@chromium.org',
       '-r', 'test_user2@chromium.org',
+      'live',
     ])
     self.assertListEqual(wpr_updater_cls.mock_calls, [
       mock.call(argparse.Namespace(
@@ -90,6 +92,83 @@ class UpdateWprTest(unittest.TestCase):
     self.wpr_updater.Cleanup()
     rmtree.assert_called_once_with('/tmp/dir', ignore_errors=True)
 
+  def testGetBranchName(self):
+    self._check_output.return_value = 'master'
+    self.assertEqual(update_wpr._GetBranchName(), 'master')
+    self._check_output.assert_called_once_with(
+        ['git', 'rev-parse', '--abbrev-ref', 'HEAD'])
+
+  def testCreateBranch(self):
+    self.wpr_updater._CreateBranch()
+    self._check_call.assert_called_once_with(
+        ['git', 'new-branch', 'update-wpr--story--1234'])
+
+  def testSendCLForReview(self):
+    update_wpr._SendCLForReview('comment')
+    self._check_call.assert_called_once_with(
+        ['git', 'cl', 'comments', '--publish', '--add-comment', 'comment'])
+
+  @mock.patch('os.dup')
+  @mock.patch('os.close')
+  @mock.patch('os.dup2')
+  @mock.patch('webbrowser.open')
+  def testOpenBrowser(self, webbrowser_open, os_dup2, os_close, os_dup):
+    del os_dup2, os_close, os_dup  # unused
+    update_wpr._OpenBrowser('<url>')
+    webbrowser_open.assert_called_once_with('<url>')
+
+  def testAutoRun(self):
+    # Mock low-level methods tested above.
+    mock.patch(WPR_UPDATER + '_GetBranchName', return_value='HEAD').start()
+    mock.patch(
+        WPR_UPDATER + 'WprUpdater._GetBranchIssueUrl',
+        return_value='<issue-url>').start()
+    mock.patch(WPR_UPDATER + 'WprUpdater._CreateBranch').start()
+    send_cl_for_review = mock.patch(WPR_UPDATER + '_SendCLForReview').start()
+    open_browser = mock.patch(WPR_UPDATER + '_OpenBrowser').start()
+
+    # Mock high-level methods tested below.
+    live_run = mock.patch(WPR_UPDATER + 'WprUpdater.LiveRun').start()
+    record_wpr = mock.patch(WPR_UPDATER + 'WprUpdater.RecordWpr').start()
+    replay_wpr = mock.patch(WPR_UPDATER + 'WprUpdater.ReplayWpr').start()
+    upload_wpr = mock.patch(
+        WPR_UPDATER + 'WprUpdater.UploadWpr', return_value=True).start()
+    upload_cl = mock.patch(
+        WPR_UPDATER + 'WprUpdater.UploadCL', return_value=0).start()
+    start_pinpoint_jobs = mock.patch(
+        WPR_UPDATER + 'WprUpdater.StartPinpointJobs',
+        return_value=(['<url1>', '<url2>', '<url3>'], [])).start()
+
+    # Mock user interaction.
+    mock.patch('core.cli_helpers.Ask', side_effect=[
+        True,        # Should script create a new branch automatically?
+        'continue',  # Should I continue with recording, ...?
+        'continue',  # Should I record and replay again, ...?
+    ]).start()
+
+    self.wpr_updater.AutoRun()
+
+    # Run once to make sure story works.
+    live_run.assert_called_once_with()
+    # Run again to create a recording.
+    record_wpr.assert_called_once_with()
+    # Replay to verify the recording.
+    replay_wpr.assert_called_once_with()
+    # Upload the recording.
+    upload_wpr.assert_called_once_with()
+    # Upload the CL.
+    upload_cl.assert_called_once_with(short_description=False)
+    # Start pinpoint jobs to verify recording works on the bots.
+    start_pinpoint_jobs.assert_called_once_with(None)
+    # Send CL for review with a comment listing triggered Pinpoint jobs.
+    send_cl_for_review.assert_called_once_with(
+        'Started the following Pinpoint jobs:\n'
+        '  - <url1>\n'
+        '  - <url2>\n'
+        '  - <url3>')
+    # Open the CL in browser,
+    open_browser.assert_called_once_with('<issue-url>')
+
   def testLiveRun(self):
     run_benchmark = mock.patch(
         WPR_UPDATER + 'WprUpdater._RunSystemHealthMemoryBenchmark',

tools/perf/core/cli_helpers.py

@@ -137,6 +137,15 @@ def Ask(question, answers=None, default=None):
         ', '.join(choices[:-1]), choices[-1]))
 
 
+def Prompt(question, accept_empty=False):
+  while True:
+    print(Colored(question, color='cyan'))
+    answer = raw_input().strip()
+    if answer or accept_empty:
+      return answer
+    Error('Please enter non-empty answer')
+
+
 def CheckLog(command, log_path, env=None):
   """Executes a command and writes its stdout to a specified log file.
 

tools/perf/core/cli_helpers_unittest.py

@@ -11,6 +11,7 @@ from core import cli_helpers
 from telemetry import decorators
 
 
+@decorators.Disabled('android', 'chromeos')
 class CLIHelpersTest(unittest.TestCase):
   def testUnsupportedColor(self):
     with self.assertRaises(AssertionError):
@@ -85,8 +86,6 @@ class CLIHelpersTest(unittest.TestCase):
       mock.call('\033[96mReady? [foo/bar] \033[0m', end=' ')
     ])
 
-  # https://crbug.com/937654.
-  @decorators.Disabled('android', 'chromeos')
   def testAskWithInvalidDefaultAnswer(self):
     with self.assertRaises(ValueError):
       cli_helpers.Ask('Ready?', ['foo', 'bar'], 'baz')
@@ -158,6 +157,16 @@ class CLIHelpersTest(unittest.TestCase):
     with self.assertRaises(ValueError):
       cli_helpers.Run('cmd with args')
 
+  @mock.patch('__builtin__.print')
+  @mock.patch('__builtin__.raw_input')
+  def testPrompt(self, raw_input_mock, print_mock):
+    raw_input_mock.side_effect = ['', '42']
+    self.assertEqual(cli_helpers.Prompt(
+        'What is the ultimate meaning of life, universe and everything?'), '42')
+    self.assertEqual(raw_input_mock.call_count, 2)
+    self.assertEqual(print_mock.call_count, 3)
+
+
 
 if __name__ == "__main__":
   unittest.main()