Updating Layout test analyzer to add control to show issue detail or not.

media/tools/layout_tests/layouttest_analyzer_helpers.py

Index: media/tools/layout_tests/layouttest_analyzer_helpers.py
diff --git a/media/tools/layout_tests/layouttest_analyzer_helpers.py b/media/tools/layout_tests/layouttest_analyzer_helpers.py
index 9f9e47b701a43c37af080f9bfe3294ba09a5cae4..75d03efe8601dabdbe78358fd433956e431cfb63 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.

[dennis_jeffrey, 2012/03/02 20:55:15]

nit: 'true' --> 'True'

[imasaki1, 2012/03/02 21:50:50]

Done.

   """
   if rev_str:
     email_content += '<br><b>Revision Information:</b>'
@@ -394,53 +407,71 @@ def GetRevisionString(prev_time, current_time, diff_map):
                             'test_expectations.txt&old=%d%40trunk%2F'
                             'LayoutTests%2Fplatform%2Fchromium%2F'
                             'test_expectations.txt') % (new_rev, old_rev)
-      rev_str += '<ul><a href="%s">%s->%s</a>\n' % (link, old_rev, new_rev)
+      rev_str += "<ul><a href='%s'>%s->%s</a>\n" % (link, old_rev, new_rev)

[dennis_jeffrey, 2012/03/02 20:55:15]

I think this line was fine before you swapped the quotes here: we'd like the
python string to be defined with single quotes, and then the html string inside
of it can use double quotes:

rev_str += '<ul><a href="%s">%s->%s</a>\n' % (link, old_rev, new_rev)

[imasaki1, 2012/03/02 21:50:50]

Done.

       simple_rev_str = '<a href="%s">%s->%s</a>,' % (link, old_rev, new_rev)
       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