Updating Layout test analyzer.

It includes:
* intoducing -z commandline option to show/not-show issue details
* fix the issues discovered by gpylint
* adding bug link and flakiness dashboard link to test expection file display

BUG=109008,107773
TEST= unit test passes and run script locally.

Review URL: https://chromiumcodereview.appspot.com/9476021

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@124746 0039d316-1c4b-4281-b951-d872f2087c98

14 files changed, 242 insertions, 182 deletions

diff --git a/media/tools/layout_tests/bug.py b/media/tools/layout_tests/bug.py
index df9b5f70..42dd17d 100644
--- a/media/tools/layout_tests/bug.py
+++ b/media/tools/layout_tests/bug.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2011 The Chromium Authors. All rights reserved.
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
@@ -31,26 +31,29 @@ class Bug(object):
         currently, BUGWK12345, BUGCR12345, BUGV8_12345, BUGDPRANKE are
         possible.
     """
-    self.bug_txt = bug_modifier
-    pattern_for_webkit_bug = r'BUGWK(\d+)'
+    pattern_for_webkit_bug = r'(BUGWK(\d+))'
     match = re.search(pattern_for_webkit_bug, bug_modifier)
     if match:
       self.type = self.WEBKIT
-      self.url = self.WEBKIT_BUG_URL + match.group(1)
+      self.url = self.WEBKIT_BUG_URL + match.group(2)
+      self.bug_txt = match.group(1)
       return
-    pattern_for_chrome_bug = r'BUGCR(\d+)'
+    pattern_for_chrome_bug = r'(BUGCR(\d+))'
     match = re.search(pattern_for_chrome_bug, bug_modifier)
     if match:
       self.type = self.CHROMIUM
-      self.url = self.CHROME_BUG_URL + match.group(1)
+      self.url = self.CHROME_BUG_URL + match.group(2)
+      self.bug_txt = match.group(1)
       return
-    pattern_for_other_bug = r'BUG(\S+)'
+    pattern_for_other_bug = r'(BUG(\S+))'
     match = re.search(pattern_for_other_bug, bug_modifier)
     if match:
       self.type = self.OTHERS
-      self.url = 'mailto:%s@chromium.org' % match.group(1).lower()
+      self.url = 'mailto:%s@chromium.org' % match.group(2).lower()
+      self.bug_txt = match.group(1)
       return
     self.url = ''
+    self.bug_txt = ''
 
   def __str__(self):
     """Get a string representation of a bug object.
diff --git a/media/tools/layout_tests/layouttest_analyzer.py b/media/tools/layout_tests/layouttest_analyzer.py
index 2543045..8b5f371 100755..100644
--- a/media/tools/layout_tests/layouttest_analyzer.py
+++ b/media/tools/layout_tests/layouttest_analyzer.py
@@ -1,5 +1,5 @@
 #!/usr/bin/env python
-# Copyright (c) 2011 The Chromium Authors. All rights reserved.
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
@@ -12,8 +12,9 @@ import os
 import sys
 import time
 
-import layouttests
 import layouttest_analyzer_helpers
+import layouttests
+
 from test_expectations import TestExpectations
 from trend_graph import TrendGraph
 
@@ -73,9 +74,9 @@ def ParseOption():
                            default=None)
   option_parser.add_option('-x', '--test-group-name',
                            dest='test_group_name',
-                           help='A name of test group. Either '
-                                '--test_group_file_location or this option '
-                                'needs to be specified.')
+                           help=('A name of test group. Either '
+                                 '--test_group_file_location or this option '
+                                 'needs to be specified.'))
   option_parser.add_option('-d', '--result-directory-location',
                            dest='result_directory_location',
                            help=('Name of result directory location '
@@ -100,6 +101,12 @@ def ParseOption():
                            help=('Location of dashboard file. The results are '
                                  'not reported to the dashboard if this '
                                  'option is not specified.'))
+  option_parser.add_option('-z', '--issue-detail-mode',
+                           dest='issue_detail_mode',
+                           help=('With this mode, email includes issue details '
+                                 '(links to the flakiness dashboard)'
+                                 ' (off by default)'),
+                           action='store_true', default=False)
   return option_parser.parse_args()[0]
 
 
@@ -160,7 +167,7 @@ def GetCurrentAndPreviousResults(debug, test_group_file_location,
     analyzer_result_map = layouttest_analyzer_helpers.AnalyzerResultMap(
         layouttests_object.JoinWithTestExpectation(TestExpectations()))
     result = layouttest_analyzer_helpers.FindLatestResult(
-                 result_directory_location)
+        result_directory_location)
     if result:
       (prev_time, prev_analyzer_result_map) = result
     else:
@@ -217,7 +224,7 @@ def ReadEmailInformation(bug_annotation_file_location,
 
 def SendEmail(prev_time, prev_analyzer_result_map, analyzer_result_map,
               anno_map, appended_text_to_email, email_only_change_mode, debug,
-              receiver_email_address, test_group_name):
+              receiver_email_address, test_group_name, issue_detail_mode):
   """Send result status email.
 
   Args:
@@ -232,6 +239,7 @@ def SendEmail(prev_time, prev_analyzer_result_map, analyzer_result_map,
     debug: please refer to |options|.
     receiver_email_address: please refer to |options|.
     test_group_name: please refer to |options|.
+    issue_detail_mode: please refer to |options|.
 
   Returns:
     a tuple of the following:
@@ -253,7 +261,7 @@ def SendEmail(prev_time, prev_analyzer_result_map, analyzer_result_map,
     diff_map = analyzer_result_map.CompareToOtherResultMap(
         prev_analyzer_result_map)
     result_change = (any(diff_map['whole']) or any(diff_map['skip']) or
-                        any(diff_map['nonskip']))
+                     any(diff_map['nonskip']))
     # Email only when |email_only_change_mode| is False or there
     # is a change in the result compared to the last result.
     simple_rev_str = ''
@@ -270,8 +278,10 @@ def SendEmail(prev_time, prev_analyzer_result_map, analyzer_result_map,
           layouttest_analyzer_helpers.GetRevisionString(prev_time_in_float,
                                                         cur_time_in_float,
                                                         diff_map))
-      email_content = analyzer_result_map.ConvertToString(prev_time, diff_map,
-                                                          anno_map)
+      email_content = analyzer_result_map.ConvertToString(prev_time,
+                                                          diff_map,
+                                                          anno_map,
+                                                          issue_detail_mode)
       if receiver_email_address:
         layouttest_analyzer_helpers.SendStatusEmail(
             prev_time, analyzer_result_map, diff_map, anno_map,
@@ -403,13 +413,13 @@ def UpdateDashboard(dashboard_file_location, test_group_name, data_map,
     sorted_testnames = data_map[tg][0].keys()
     sorted_testnames.sort()
     for testname in sorted_testnames:
-      file_object.write(('<tr><td><a href="%s">%s</a></td>'
-                         '<td><a href="%s">dashboard</a></td>'
-                         '<td>%s</td></tr>') % (
-          layouttest_root_path + testname, testname,
-          ('http://test-results.appspot.com/dashboards/'
-           'flakiness_dashboard.html#tests=%s') % testname,
-          data_map[tg][0][testname]))
+      file_object.write((
+          '<tr><td><a href="%s">%s</a></td><td><a href="%s">dashboard</a>'
+          '</td><td>%s</td></tr>') % (
+              layouttest_root_path + testname, testname,
+              ('http://test-results.appspot.com/dashboards/'
+               'flakiness_dashboard.html#tests=%s') % testname,
+              data_map[tg][0][testname]))
     file_object.write('</table>')
     file_object.close()
   email_content_with_link = ''
@@ -459,7 +469,8 @@ def main():
       SendEmail(prev_time, prev_analyzer_result_map, analyzer_result_map,
                 anno_map, appended_text_to_email,
                 options.email_only_change_mode, options.debug,
-                options.receiver_email_address, options.test_group_name))
+                options.receiver_email_address, options.test_group_name,
+                options.issue_detail_mode))
 
   # Create CSV texts and save them for bug spreadsheet.
   (stats, issues_txt) = analyzer_result_map.ConvertToCSVText(
diff --git a/media/tools/layout_tests/layouttest_analyzer_helpers.py b/media/tools/layout_tests/layouttest_analyzer_helpers.py
index 9f9e47b..bddb862 100644
--- a/media/tools/layout_tests/layouttest_analyzer_helpers.py
+++ b/media/tools/layout_tests/layouttest_analyzer_helpers.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2011 The Chromium Authors. All rights reserved.
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
@@ -74,14 +74,14 @@ class AnalyzerResultMap:
 
     Args:
       diff_map_element: An element of the compared map generated by
-         |CompareResultMaps()|. The element has two lists of test cases. One
-         is for test names that are in the current result but NOT in the
-         previous result. The other is for test names that are in the previous
-         results but NOT in the current result. Please refer to comments in
-         |CompareResultMaps()| for details.
-    type_str: a string indicating the test group to which |diff_map_element|
-        belongs; used for color determination.  Must be 'whole', 'skip', or
-        'nonskip'.
+          |CompareResultMaps()|. The element has two lists of test cases. One
+          is for test names that are in the current result but NOT in the
+          previous result. The other is for test names that are in the previous
+          results but NOT in the current result. Please refer to comments in
+          |CompareResultMaps()| for details.
+      type_str: a string indicating the test group to which |diff_map_element|
+          belongs; used for color determination.  Must be 'whole', 'skip', or
+          'nonskip'.
 
     Returns:
       a string in HTML format (with colors) to show difference between two
@@ -99,20 +99,26 @@ class AnalyzerResultMap:
     if diff > 0:
       diff_sign = '+'
     whole_str = '<font color="%s">%s%d</font>' % (color, diff_sign, diff)
+    colors = ['red', 'green']
+    if type_str == 'whole':
+      # Bug 107773 - when we increase the number of tests,
+      # the name of the tests are in red, it should be green
+      # since it is good thing.
+      colors = ['green', 'red']
     str1 = ''
     for (name, _) in diff_map_element[0]:
-      str1 += '<font color="red">%s,</font> ' % name
-    str1 = str1[:-1]
+      str1 += '<font color="%s">%s,</font>' % (colors[0], name)
     str2 = ''
     for (name, _) in diff_map_element[1]:
-      str2 += '<font color="green">%s,</font> ' % name
-    str2 = str2[:-1]
+      str2 += '<font color="%s">%s,</font>' % (colors[1], name)
     if str1 or str2:
       whole_str += ':'
     if str1:
       whole_str += str1
     if str2:
       whole_str += str2
+    # Remove the last occurrence of ','.
+    whole_str = ''.join(whole_str.rsplit(',', 1))
     return whole_str
 
   def GetPassingRate(self):
@@ -122,8 +128,8 @@ class AnalyzerResultMap:
       layout test passing rate of this result in percent.
 
     Raises:
-      ValueEror when the number of tests in test group "whole" is equal or less
-          than that of "skip".
+      ValueEror when the number of tests in test group "whole" is equal
+          or less than that of "skip".
     """
     delta = len(self.result_map['whole'].keys()) - (
         len(self.result_map['skip'].keys()))
@@ -133,49 +139,50 @@ class AnalyzerResultMap:
     return 100 - len(self.result_map['nonskip'].keys()) * 100 / delta
 
   def ConvertToCSVText(self, current_time):
-   """Convert |self.result_map| into stats and issues text in CSV format.
+    """Convert |self.result_map| into stats and issues text in CSV format.
 
-   Both are used as inputs for Google spreadsheet.
+    Both are used as inputs for Google spreadsheet.
 
-   Args:
-     current_time: a string depicting a time in year-month-day-hour
+    Args:
+      current_time: a string depicting a time in year-month-day-hour
         format (e.g., 2011-11-08-16).
 
-   Returns:
-     a tuple of stats and issues_txt
-        stats: analyzer result in CSV format that shows:
+    Returns:
+      a tuple of stats and issues_txt
+      stats: analyzer result in CSV format that shows:
           (current_time, the number of tests, the number of skipped tests,
            the number of failing tests, passing rate)
           For example,
             "2011-11-10-15,204,22,12,94"
-        issues_txt: issues listed in CSV format that shows:
+       issues_txt: issues listed in CSV format that shows:
           (BUGWK or BUGCR, bug number, the test expectation entry,
            the name of the test)
           For example,
             "BUGWK,71543,TIMEOUT PASS,media/media-element-play-after-eos.html,
              BUGCR,97657,IMAGE CPU MAC TIMEOUT PASS,media/audio-repaint.html,"
-   """
-   stats = ','.join([current_time, str(len(self.result_map['whole'].keys())),
-                     str(len(self.result_map['skip'].keys())),
-                     str(len(self.result_map['nonskip'].keys())),
-                     str(self.GetPassingRate())])
-   issues_txt = ''
-   for bug_txt, test_info_list in (
-       self.GetListOfBugsForNonSkippedTests().iteritems()):
-     matches = re.match(r'(BUG(CR|WK))(\d+)', bug_txt)
-     bug_suffix = ''
-     bug_no = ''
-     if matches:
-       bug_suffix = matches.group(1)
-       bug_no = matches.group(3)
-     issues_txt += bug_suffix + ',' + bug_no + ','
-     for test_info in test_info_list:
-       test_name, te_info = test_info
-       issues_txt += ' '.join(te_info.keys()) + ',' + test_name + ','
-     issues_txt += '\n'
-   return stats, issues_txt
-
-  def ConvertToString(self, prev_time, diff_map, bug_anno_map):
+    """
+    stats = ','.join([current_time, str(len(self.result_map['whole'].keys())),
+                      str(len(self.result_map['skip'].keys())),
+                      str(len(self.result_map['nonskip'].keys())),
+                      str(self.GetPassingRate())])
+    issues_txt = ''
+    for bug_txt, test_info_list in (
+        self.GetListOfBugsForNonSkippedTests().iteritems()):
+      matches = re.match(r'(BUG(CR|WK))(\d+)', bug_txt)
+      bug_suffix = ''
+      bug_no = ''
+      if matches:
+        bug_suffix = matches.group(1)
+        bug_no = matches.group(3)
+      issues_txt += bug_suffix + ',' + bug_no + ','
+      for test_info in test_info_list:
+        test_name, te_info = test_info
+        issues_txt += ' '.join(te_info.keys()) + ',' + test_name + ','
+      issues_txt += '\n'
+    return stats, issues_txt
+
+  def ConvertToString(self, prev_time, diff_map, bug_anno_map,
+                      issue_detail_mode):
     """Convert this result to HTML display for email.
 
     Args:
@@ -183,42 +190,47 @@ class AnalyzerResultMap:
       diff_map: the compared map generated by |CompareResultMaps()|.
       bug_anno_map: a annotation map where keys are bug names and values are
           annotations for the bug.
+      issue_detail_mode: includes the issue details in the output string if
+          this is True.
 
     Returns:
       a analyzer result string in HTML format.
     """
     return_str = ''
     if diff_map:
-      return_str += ('<b>Statistics (Diff Compared to %s):</b><ul>'
-             '<li>The number of tests: %d (%s)</li>'
-             '<li>The number of failing skipped tests: %d (%s)</li>'
-             '<li>The number of failing non-skipped tests: %d (%s)</li>'
-             '<li>Passing rate: %d %%</li></ul>') % (
-                 prev_time, len(self.result_map['whole'].keys()),
-                 AnalyzerResultMap.GetDiffString(diff_map['whole'], 'whole'),
-                 len(self.result_map['skip'].keys()),
-                 AnalyzerResultMap.GetDiffString(diff_map['skip'], 'skip'),
-                 len(self.result_map['nonskip'].keys()),
-                 AnalyzerResultMap.GetDiffString(diff_map['nonskip'],
-                                                 'nonskip'),
-                 self.GetPassingRate())
-    return_str += '<b>Current issues about failing non-skipped tests:</b>'
-    for (bug_txt, test_info_list) in (
-        self.GetListOfBugsForNonSkippedTests().iteritems()):
-      if not bug_txt in bug_anno_map:
-        bug_anno_map[bug_txt] = '<font color="red">Needs investigation!</font>'
-      return_str += '<ul>%s (%s)' % (Bug(bug_txt), bug_anno_map[bug_txt])
-      for test_info in test_info_list:
-        (test_name, te_info) = test_info
-        gpu_link = ''
-        if 'GPU' in te_info:
-          gpu_link = 'group=%40ToT%20GPU%20Mesa%20-%20chromium.org&'
-        dashboard_link = ('http://test-results.appspot.com/dashboards/'
-                          'flakiness_dashboard.html#%stests=%s') % (
-                              gpu_link, test_name)
-        return_str += '<li><a href="%s">%s</a> (%s) </li>' % (
-            dashboard_link, test_name, ' '.join(te_info.keys()))
-      return_str += '</ul>\n'
+      return_str += (
+          '<b>Statistics (Diff Compared to %s):</b><ul>'
+          '<li>The number of tests: %d (%s)</li>'
+          '<li>The number of failing skipped tests: %d (%s)</li>'
+          '<li>The number of failing non-skipped tests: %d (%s)</li>'
+          '<li>Passing rate: %d %%</li></ul>') % (
+              prev_time, len(self.result_map['whole'].keys()),
+              AnalyzerResultMap.GetDiffString(diff_map['whole'], 'whole'),
+              len(self.result_map['skip'].keys()),
+              AnalyzerResultMap.GetDiffString(diff_map['skip'], 'skip'),
+              len(self.result_map['nonskip'].keys()),
+              AnalyzerResultMap.GetDiffString(diff_map['nonskip'], 'nonskip'),
+              self.GetPassingRate())
+    if issue_detail_mode:
+      return_str += '<b>Current issues about failing non-skipped tests:</b>'
+      for (bug_txt, test_info_list) in (
+          self.GetListOfBugsForNonSkippedTests().iteritems()):
+        if not bug_txt in bug_anno_map:
+          bug_anno_map[bug_txt] = ''
+        else:
+          bug_anno_map[bug_txt] = '(' + bug_anno_map[bug_txt] + ')'
+        return_str += '<ul>%s %s' % (Bug(bug_txt), bug_anno_map[bug_txt])
+        for test_info in test_info_list:
+          (test_name, te_info) = test_info
+          gpu_link = ''
+          if 'GPU' in te_info:
+            gpu_link = 'group=%40ToT%20GPU%20Mesa%20-%20chromium.org&'
+          dashboard_link = ('http://test-results.appspot.com/dashboards/'
+                            'flakiness_dashboard.html#%stests=%s') % (
+                                gpu_link, test_name)
+          return_str += '<li><a href="%s">%s</a> (%s) </li>' % (
+              dashboard_link, test_name, ' '.join(te_info.keys()))
+        return_str += '</ul>\n'
     return return_str
 
   def CompareToOtherResultMap(self, other_result_map):
@@ -244,8 +256,8 @@ class AnalyzerResultMap:
         #  Otherwise, only test names are compared to get diff.
         lookIntoTestExpectationInfo = False
       comp_result_map[name] = GetDiffBetweenMaps(
-         self.result_map[name], other_result_map.result_map[name],
-         lookIntoTestExpectationInfo)
+          self.result_map[name], other_result_map.result_map[name],
+          lookIntoTestExpectationInfo)
     return comp_result_map
 
   @staticmethod
@@ -331,7 +343,8 @@ def SendStatusEmail(prev_time, analyzer_result_map, diff_map,
     rev_str: a revision string that contains revision information that is sent
         out in the status email. It is obtained by calling
         |GetRevisionString()|.
-    email_only_change_mode: please refer to |options|.
+    email_only_change_mode: send email only when there is a change if this is
+        True. Otherwise, always send email after each run.
   """
   if rev_str:
     email_content += '<br><b>Revision Information:</b>'
@@ -399,48 +412,66 @@ def GetRevisionString(prev_time, current_time, diff_map):
       rev_str += '<li>%s</li>\n' % author
       rev_str += '<li>%s</li>\n<ul>' % date
       for line in target_lines:
-        rev_str += '<li>%s</li>\n' % line
+        # Find *.html pattern (test name) and replace it with the link to
+        # flakiness dashboard.
+        test_name_pattern = r'(\S+.html)'
+        match = re.search(test_name_pattern, line)
+        if match:
+          test_name = match.group(1)
+          gpu_link = ''
+          if 'GPU' in line:
+            gpu_link = 'group=%40ToT%20GPU%20Mesa%20-%20chromium.org&'
+          dashboard_link = ('http://test-results.appspot.com/dashboards/'
+                            'flakiness_dashboard.html#%stests=%s') % (
+                                gpu_link, test_name)
+          line = line.replace(test_name, '<a href="%s">%s</a>' % (
+              dashboard_link, test_name))
+        # Find bug text and replace it with the link to the bug.
+        bug = Bug(line)
+        if bug.bug_txt:
+          line = '<li>%s</li>\n' % line.replace(bug.bug_txt, str(bug))
+        rev_str += line
       rev_str += '</ul></ul>'
   return (rev_str, simple_rev_str, rev, rev_date)
 
 
 def SendEmail(sender_email_address, receivers_email_addresses, subject,
               message):
-    """Send email using localhost's mail server.
+  """Send email using localhost's mail server.
 
-    Args:
-        sender_email_address: sender's email address.
-        receivers_email_addresses: receiver's email addresses.
-        subject: subject string.
-        message: email message.
+  Args:
+    sender_email_address: sender's email address.
+    receivers_email_addresses: receiver's email addresses.
+    subject: subject string.
+    message: email message.
+  """
+  try:
+    html_top = """
+      <html>
+      <head></head>
+      <body>
     """
-    try:
-      html_top = """
-        <html>
-        <head></head>
-        <body>
-      """
-      html_bot = """
-        </body>
-        </html>
-      """
-      html = html_top + message + html_bot
-      msg = MIMEMultipart('alternative')
-      msg['Subject'] = subject
-      msg['From'] = sender_email_address
-      msg['To'] = receivers_email_addresses[0]
-      part1 = MIMEText(html, 'html')
-      smtp_obj = smtplib.SMTP('localhost')
-      msg.attach(part1)
-      smtp_obj.sendmail(sender_email_address, receivers_email_addresses,
-                        msg.as_string())
-      print 'Successfully sent email'
-    except smtplib.SMTPException, ex:
-      print 'Authentication failed:', ex
-      print 'Error: unable to send email'
-    except (socket.gaierror, socket.error, socket.herror), ex:
-      print ex
-      print 'Error: unable to send email'
+    html_bot = """
+      </body>
+      </html>
+    """
+    html = html_top + message + html_bot
+    msg = MIMEMultipart('alternative')
+    msg['Subject'] = subject
+    msg['From'] = sender_email_address
+    msg['To'] = receivers_email_addresses[0]
+    part1 = MIMEText(html, 'html')
+    smtp_obj = smtplib.SMTP('localhost')
+    msg.attach(part1)
+    smtp_obj.sendmail(sender_email_address, receivers_email_addresses,
+                      msg.as_string())
+    print 'Successfully sent email'
+  except smtplib.SMTPException, ex:
+    print 'Authentication failed:', ex
+    print 'Error: unable to send email'
+  except (socket.gaierror, socket.error, socket.herror), ex:
+    print ex
+    print 'Error: unable to send email'
 
 
 def FindLatestTime(time_list):
@@ -463,7 +494,7 @@ def FindLatestTime(time_list):
   for time_element in time_list:
     try:
       item_date = datetime.strptime(time_element, '%Y-%m-%d-%H')
-      if latest_date == None or latest_date < item_date:
+      if latest_date is None or latest_date < item_date:
         latest_date = item_date
     except ValueError:
       # Do nothing.
@@ -546,7 +577,7 @@ def GetDiffBetweenMaps(map1, map2, lookIntoTestExpectationInfo=False):
           list2 = map2[name]['te_info']
           te_diff = [item for item in list1 if not item in list2]
           if te_diff:
-              name_list.append((name, te_diff))
+            name_list.append((name, te_diff))
       else:
         name_list.append((name, value1))
     return name_list
diff --git a/media/tools/layout_tests/layouttest_analyzer_helpers_unittest.py b/media/tools/layout_tests/layouttest_analyzer_helpers_unittest.py
index 05acf8d..4408be7 100755..100644
--- a/media/tools/layout_tests/layouttest_analyzer_helpers_unittest.py
+++ b/media/tools/layout_tests/layouttest_analyzer_helpers_unittest.py
@@ -1,5 +1,5 @@
 #!/usr/bin/env python
-# Copyright (c) 2011 The Chromium Authors. All rights reserved.
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
@@ -122,9 +122,9 @@ class TestLayoutTestAnalyzerHelpers(unittest.TestCase):
       diff_map = None
     else:
       diff_map = {
-        'whole': [[], []],
-        'skip': [[(testname, 'te_info1')], []],
-        'nonskip': [[], []],
+          'whole': [[], []],
+          'skip': [[(testname, 'te_info1')], []],
+          'nonskip': [[], []],
       }
     (rev_str, simple_rev_str, rev_number, rev_date) = (
         layouttest_analyzer_helpers.GetRevisionString(prev_time, current_time,
@@ -142,9 +142,11 @@ class TestLayoutTestAnalyzerHelpers(unittest.TestCase):
                         '94377</a>\n'
                         '<li>jamesr@google.com</li>\n'
                         '<li>2011-09-01 18:00:23</li>\n'
-                        '<ul><li>-BUGWK63878 : fast/dom/dom-constructors.html'
-                        ' = TEXT</li>\n'
-                        '</ul></ul>')
+                        '<ul><li>-<a href="http://webkit.org/b/63878">'
+                        'BUGWK63878</a> : <a href=\'http://test-results.'
+                        'appspot.com/dashboards/flakiness_dashboard.html#'
+                        'tests=fast/dom/dom-constructors.html\'>fast/dom/'
+                        'dom-constructors.html</a> = TEXT</li>\n</ul></ul>')
     expected_simple_rev_str = ('<a href="http://trac.webkit.org/changeset?'
                                'new=94377@trunk/LayoutTests/platform/chromium/'
                                'test_expectations.txt&old=94366@trunk/'
diff --git a/media/tools/layout_tests/layouttest_analyzer_runner.py b/media/tools/layout_tests/layouttest_analyzer_runner.py
index 868c746..eec0f4d 100755..100644
--- a/media/tools/layout_tests/layouttest_analyzer_runner.py
+++ b/media/tools/layout_tests/layouttest_analyzer_runner.py
@@ -1,5 +1,5 @@
 #!/usr/bin/env python
-# Copyright (c) 2011 The Chromium Authors. All rights reserved.
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
@@ -75,6 +75,12 @@ def ParseOption():
                                  'analyzer result compared to the previous '
                                  'result (off by default)'),
                            action='store_true', default=False)
+  option_parser.add_option('-z', '--issue-detail-mode',
+                           dest='issue_detail_mode',
+                           help=('With this mode, email includes issue details'
+                                 ' including links to the flakiness dashboard'
+                                 ' (off by default)'),
+                           action='store_true', default=False)
   return option_parser.parse_args()[0]
 
 
@@ -194,6 +200,8 @@ def main():
       cmd += ' -b ' + options.email_appended_text_file_location
     if options.email_only_change_mode:
       cmd += ' -c '
+    if options.issue_detail_mode:
+      cmd += ' -z '
     print 'Running ' + cmd
     proc = Popen(cmd, shell=True)
     proc.communicate()
diff --git a/media/tools/layout_tests/layouttests.py b/media/tools/layout_tests/layouttests.py
index 0d9a9c6..c118194 100644
--- a/media/tools/layout_tests/layouttests.py
+++ b/media/tools/layout_tests/layouttests.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2011 The Chromium Authors. All rights reserved.
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
@@ -11,11 +11,11 @@ layout test cases (including description).
 import copy
 import csv
 import locale
-import pysvn
 import re
 import sys
 import urllib2
 
+import pysvn
 
 # Webkit SVN root location.
 DEFAULT_LAYOUTTEST_LOCATION = (
@@ -69,7 +69,7 @@ class LayoutTests(object):
     self.name_map = copy.copy(name_map)
     if filter_names:
       # Filter names.
-      for lt_name in name_map.keys():
+      for lt_name in name_map.iterkeys():
         match = False
         for filter_name in filter_names:
           if re.search(filter_name, lt_name):
@@ -78,7 +78,7 @@ class LayoutTests(object):
         if not match:
           del self.name_map[lt_name]
     # We get description only for the filtered names.
-    for lt_name in self.name_map.keys():
+    for lt_name in self.name_map.iterkeys():
       self.name_map[lt_name] = LayoutTests.GetTestDescriptionFromSVN(lt_name)
 
   @staticmethod
@@ -110,7 +110,7 @@ class LayoutTests(object):
       pattern = r'<p>(.*' + keyword + '.*)</p>'
       matches = re.search(pattern, txt)
       if matches is not None:
-          return matches.group(1).strip()
+        return matches.group(1).strip()
 
     # (2) Try to find it by using more generic keywords such as 'PASS' etc.
     for keyword in KEYWORD_FOR_TEST_DESCRIPTION_FAIL_SAFE:
@@ -118,16 +118,16 @@ class LayoutTests(object):
       pattern = r'\n(.*' + keyword + '.*)\n'
       matches = re.search(pattern, txt)
       if matches is not None:
-          # Remove 'p' tag.
-          text = matches.group(1).strip()
-          return text.replace('<p>', '').replace('</p>', '')
+        # Remove 'p' tag.
+        text = matches.group(1).strip()
+        return text.replace('<p>', '').replace('</p>', '')
 
     # (3) Try to find it by using HTML tag such as title.
     for tag in TAGS_FOR_TEST_DESCRIPTION:
       pattern = r'<' + tag + '>(.*)</' + tag + '>'
       matches = re.search(pattern, txt)
       if matches is not None:
-          return matches.group(1).strip()
+        return matches.group(1).strip()
 
     # (4) Try to find it by using test description and remove 'p' tag.
     for keyword in KEYWORDS_FOR_TEST_DESCRIPTION:
@@ -135,9 +135,9 @@ class LayoutTests(object):
       pattern = r'\n(.*' + keyword + '.*)\n'
       matches = re.search(pattern, txt)
       if matches is not None:
-          # Remove 'p' tag.
-          text = matches.group(1).strip()
-          return text.replace('<p>', '').replace('</p>', '')
+        # Remove 'p' tag.
+        text = matches.group(1).strip()
+        return text.replace('<p>', '').replace('</p>', '')
 
     # (5) cannot find test description using existing rules.
     return 'UNKNOWN'
@@ -184,6 +184,9 @@ class LayoutTests(object):
       csv_file_path: the path for the CSV file containing test names (including
           regular expression patterns). The CSV file content has one column and
           each row contains a test name.
+
+    Returns:
+       a list of test names in string.
     """
     file_object = file(csv_file_path, 'r')
     reader = csv.reader(file_object)
@@ -198,6 +201,9 @@ class LayoutTests(object):
     Args:
       names: a list of test names. The test names also have path information as
           well (e.g., media/video-zoom.html).
+
+    Returns:
+      a list of parent directories for the given test names.
     """
     pd_map = {}
     for name in names:
diff --git a/media/tools/layout_tests/layouttests_unittest.py b/media/tools/layout_tests/layouttests_unittest.py
index 9707348..7fb44df 100755..100644
--- a/media/tools/layout_tests/layouttests_unittest.py
+++ b/media/tools/layout_tests/layouttests_unittest.py
@@ -1,9 +1,8 @@
 #!/usr/bin/env python
-# Copyright (c) 2011 The Chromium Authors. All rights reserved.
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-import os
 import unittest
 
 from layouttests import LayoutTests
@@ -31,7 +30,7 @@ class TestLayoutTests(unittest.TestCase):
     self.assertEquals(desc1,
                       ('Test that play() from EMPTY network state triggers '
                        'load() and async play event.'),
-                       msg='Extracted test description is wrong')
+                      msg='Extracted test description is wrong')
     desc2 = LayoutTests.GetTestDescriptionFromSVN('jquery/data.html')
     self.assertEquals(desc2, 'UNKNOWN',
                       msg='Extracted test description is wrong')
diff --git a/media/tools/layout_tests/test_expectations.py b/media/tools/layout_tests/test_expectations.py
index 3314f87..6421d48 100644
--- a/media/tools/layout_tests/test_expectations.py
+++ b/media/tools/layout_tests/test_expectations.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2011 The Chromium Authors. All rights reserved.
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
diff --git a/media/tools/layout_tests/test_expectations_history.py b/media/tools/layout_tests/test_expectations_history.py
index 27db08b..3799db7 100644
--- a/media/tools/layout_tests/test_expectations_history.py
+++ b/media/tools/layout_tests/test_expectations_history.py
@@ -1,17 +1,17 @@
-# Copyright (c) 2011 The Chromium Authors. All rights reserved.
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """A module for the history of the test expectation file."""
 
+from datetime import datetime
+from datetime import timedelta
+
 import re
 import sys
 import time
 import pysvn
 
-from datetime import datetime
-from datetime import timedelta
-
 # Default Webkit SVN location for chromium test expectation file.
 # TODO(imasaki): support multiple test expectation files.
 DEFAULT_TEST_EXPECTATION_LOCATION = (
@@ -19,7 +19,7 @@ DEFAULT_TEST_EXPECTATION_LOCATION = (
     'LayoutTests/platform/chromium/test_expectations.txt')
 
 
-class TestExpectationsHistory:
+class TestExpectationsHistory(object):
   """A class to represent history of the test expectation file.
 
   The history is obtained by calling PySVN.log()/diff() APIs.
diff --git a/media/tools/layout_tests/test_expectations_history_unittest.py b/media/tools/layout_tests/test_expectations_history_unittest.py
index 56eb945..4602a78 100755..100644
--- a/media/tools/layout_tests/test_expectations_history_unittest.py
+++ b/media/tools/layout_tests/test_expectations_history_unittest.py
@@ -1,10 +1,10 @@
 #!/usr/bin/env python
-# Copyright (c) 2011 The Chromium Authors. All rights reserved.
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 from datetime import datetime
-from datetime import timedelta
+
 import time
 import unittest
 
diff --git a/media/tools/layout_tests/test_expectations_unittest.py b/media/tools/layout_tests/test_expectations_unittest.py
index dc60b7d..92250d7 100755..100644
--- a/media/tools/layout_tests/test_expectations_unittest.py
+++ b/media/tools/layout_tests/test_expectations_unittest.py
@@ -1,5 +1,5 @@
 #!/usr/bin/env python
-# Copyright (c) 2011 The Chromium Authors. All rights reserved.
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
@@ -11,7 +11,7 @@ from test_expectations import TestExpectations
 class TestTestExpectations(unittest.TestCase):
 
   def testParseLine(self):
-    line = "BUGCR86714 MAC GPU : media/video-zoom.html = CRASH IMAGE"
+    line = 'BUGCR86714 MAC GPU : media/video-zoom.html = CRASH IMAGE'
     comments = 'Comments'
     expected_map = {'CRASH': True, 'IMAGE': True, 'Bugs': ['BUGCR86714'],
                     'Comments': 'Comments', 'MAC': True, 'GPU': True}
@@ -19,7 +19,7 @@ class TestTestExpectations(unittest.TestCase):
                       expected_map)
 
   def testParseLineWithLineComments(self):
-    line = "BUGCR86714 MAC GPU : media/video-zoom.html = CRASH IMAGE // foo"
+    line = 'BUGCR86714 MAC GPU : media/video-zoom.html = CRASH IMAGE // foo'
     comments = 'Comments'
     expected_map = {'CRASH': True, 'IMAGE': True, 'Bugs': ['BUGCR86714'],
                     'Comments': 'Comments foo', 'MAC': True, 'GPU': True}
@@ -27,7 +27,7 @@ class TestTestExpectations(unittest.TestCase):
                       expected_map)
 
   def testParseLineWithLineGPUComments(self):
-    line = "BUGCR86714 MAC : media/video-zoom.html = CRASH IMAGE // GPU"
+    line = 'BUGCR86714 MAC : media/video-zoom.html = CRASH IMAGE // GPU'
     comments = 'Comments'
     expected_map = {'CRASH': True, 'IMAGE': True, 'Bugs': ['BUGCR86714'],
                     'Comments': 'Comments GPU', 'MAC': True}
diff --git a/media/tools/layout_tests/testname/media.csv b/media/tools/layout_tests/testname/media.csv
index d0b5982..cf3b561 100644
--- a/media/tools/layout_tests/testname/media.csv
+++ b/media/tools/layout_tests/testname/media.csv
@@ -22,3 +22,4 @@ http/tests/appcache/video.html,
 http/tests/canvas/webgl/origin-clean-conformance.html,
 http/tests/security/contentSecurityPolicy/media-src-allowed.html,
 http/tests/security/contentSecurityPolicy/media-src-blocked.html,
+platform/chromium/media/\S+.html,
diff --git a/media/tools/layout_tests/trend_graph.py b/media/tools/layout_tests/trend_graph.py
index a2bcafa..f6a8f6f 100644
--- a/media/tools/layout_tests/trend_graph.py
+++ b/media/tools/layout_tests/trend_graph.py
@@ -1,11 +1,10 @@
-# Copyright (c) 2011 The Chromium Authors. All rights reserved.
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """A module for manipulating trend graph with analyzer result history."""
 
 import os
-import sys
 
 import layouttest_analyzer_helpers
 
@@ -52,7 +51,7 @@ class TrendGraph(object):
     # After the below conversion, for example, in the case of the year 2008,
     # |datetime_string| ranges from '2008,0,1,0,0,00' to '2008,11,31,23,59,99'.
     str_list = datetime_string.split(',')
-    str_list[1] = str(int(str_list[1])-1) # month
+    str_list[1] = str(int(str_list[1])-1)  # Month
     datetime_string = ','.join(str_list)
     for key in ['whole', 'skip', 'nonskip']:
       joined_str += str(len(data_map[key][0])) + ','
diff --git a/media/tools/layout_tests/trend_graph_unittest.py b/media/tools/layout_tests/trend_graph_unittest.py
index bc9d095..f866aa6 100755..100644
--- a/media/tools/layout_tests/trend_graph_unittest.py
+++ b/media/tools/layout_tests/trend_graph_unittest.py
@@ -1,5 +1,5 @@
 #!/usr/bin/env python
-# Copyright (c) 2011 The Chromium Authors. All rights reserved.
+# Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
@@ -29,11 +29,11 @@ class TestTrendGraph(unittest.TestCase):
     f = open(test_graph_file_path)
     lines2 = f.readlines()
     f.close()
-    lineCount = 0
+    line_count = 0
     for line in lines2:
       if '2008,0,1,13,45,00' in line:
-        lineCount += 1
-    self.assertEqual(lineCount, 2)
+        line_count += 1
+    self.assertEqual(line_count, 2)
 
 
 if __name__ == '__main__':