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

media/tools/layout_tests/layouttest_analyzer_helpers.py

 # Copyright (c) 2011 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.
 
 """Helper functions for the layout test analyzer."""
 
 from datetime import datetime
 from email.mime.multipart import MIMEMultipart
 from email.mime.text import MIMEText
 import fileinput
@@ skipping 56 matching lines @@
   @staticmethod
   def GetDiffString(diff_map_element, type_str):
     """Get difference string out of diff map element.
 
     The difference string shows difference between two analyzer results
     (for example, a result for now and a result for sometime in the past)
     in HTML format (with colors). This is used for generating email messages.
 
     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|

[dennis_jeffrey, 2012/02/29 17:50:32]

indent this line by 1 more space

[imasaki1, 2012/03/02 19:12:07]

Done.

+          belongs; used for color determination.  Must be 'whole', 'skip', or
+          'nonskip'.
 
     Returns:
       a string in HTML format (with colors) to show difference between two
           analyzer results.
     """
     diff = len(diff_map_element[0]) - len(diff_map_element[1])
     if diff == 0:
       return 'No Change'
     color = ''
     if diff > 0 and type_str != 'whole':
       color = 'red'
     else:
       color = 'green'
     diff_sign = ''
     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))

[dennis_jeffrey, 2012/02/29 17:50:32]

Another way to do this without splitting and joining might be to use rfind to
find the index of the right-most comma.  Then you can concatenate the string
right before and right after that.  But I think it's also ok to leave it like
this.

[imasaki1, 2012/03/02 19:12:07]

I prefer to leave as it is. but, thanks for the comments.

     return whole_str
 
   def GetPassingRate(self):
     """Get passing rate.
 
     Returns:
       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()))
     if delta <= 0:
       raise ValueError('The number of tests in test group "whole" is equal or '
                        'less than that of "skip"')
     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
+    """
+    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):
+  def ConvertToString(self, prev_time, diff_map, bug_anno_map,
+                      issue_detail_mode):
     """Convert this result to HTML display for email.
 
     Args:
       prev_time: the previous time string that are compared against.
       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: Please see |options|.

[dennis_jeffrey, 2012/02/29 17:50:32]

Since "options" is not defined in this file, it might be better to just describe
what this parameter is here, rather than referring to "options".

[imasaki1, 2012/03/02 19:12:07]

Done.

 
     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):
     """Compare this result map with the other to see if there are any diff.
 
     The comparison is done for layouttests which belong to 'whole', 'skip',
     or 'nonskip'.
 
     Args:
       other_result_map: another result map to be compared against the result
           map of the current object.
 
     Returns:
       a map that has 'whole', 'skip' and 'nonskip' as keys.
           Please refer to |diff_map| in |SendStatusEmail()|.
     """
     comp_result_map = {}
     for name in ['whole', 'skip', 'nonskip']:
       if name == 'nonskip':
         # Look into expectation to get diff only for non-skipped tests.
         lookIntoTestExpectationInfo = True
       else:
         #  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
   def Load(file_path):
     """Load the object from |file_path| using pickle library.
 
     Args:
       file_path: the string path to the file from which to read the result.
 
     Returns:
@@ skipping 65 matching lines @@
     bug_anno_map: bug annotation map where bug name and annotations are
         stored.
     receiver_email_address: receiver's email address.
     test_group_name: string representing the test group name (e.g., 'media').
     appended_text_to_email: a text which is appended at the end of the status
         email.
     email_content: an email content string that will be shown on the dashboard.
     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|.

[dennis_jeffrey, 2012/02/29 17:50:32]

Similar comment as line 193 above.

[imasaki1, 2012/03/02 19:12:07]

Done.

   """
   if rev_str:
     email_content += '<br><b>Revision Information:</b>'
     email_content += rev_str
   localtime = time.asctime(time.localtime(time.time()))
   change_str = ''
   if email_only_change_mode:
     change_str = 'Status Change '
   subject = 'Layout Test Analyzer Result %s(%s): %s' % (change_str,
                                                         test_group_name,
@@ skipping 47 matching lines @@
       link = urllib.unquote('http://trac.webkit.org/changeset?new=%d%40trunk'
                             '%2FLayoutTests%2Fplatform%2Fchromium%2F'
                             '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)
       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
+        # frakiness dashboard.

[dennis_jeffrey, 2012/02/29 17:50:32]

frakiness --> flakiness

[imasaki1, 2012/03/02 19:12:07]

Done.

+        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>" % (

[dennis_jeffrey, 2012/02/29 17:50:32]

nit: I recommend swapping the single quotes and double quotes in this line. 
That way, the outer string in python will be defined with single quotes, and the
inner HTML string will have the double quotes.

[imasaki1, 2012/03/02 19:12:07]

Done.

+              dashboard_link, test_name))
+        # Find bug text and replace it with the link to the bug.
+        bug = Bug(line)
+        new_line = '<li>%s</li>\n' % line.replace(bug.bug_txt, bug.__str__())

[dennis_jeffrey, 2012/02/29 17:50:32]

could we replace bug.__str__() with str(bug)?

[imasaki1, 2012/03/02 19:12:07]

Done.

+        rev_str += new_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):
   """Find latest time from |time_list|.
 
   The current status is compared to the status of the latest file in
   |RESULT_DIR|.
 
   Args:
     time_list: a list of time string in the form of 'Year-Month-Day-Hour'
         (e.g., 2011-10-23-23). Strings not in this format are ignored.
 
   Returns:
      a string representing latest time among the time_list or None if
          |time_list| is empty or no valid date string in |time_list|.
   """
   if not time_list:
     return None
   latest_date = None
   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.
       pass
   if latest_date:
     return latest_date.strftime('%Y-%m-%d-%H')
   else:
     return None
 
 
@@ skipping 62 matching lines @@
       a list of tuples (name, te_info) that are in |map1| but not in |map2|.
     """
     name_list = []
     for (name, value1) in map1.iteritems():
       if name in map2:
         if lookIntoTestExpectationInfo and 'te_info' in value1:
           list1 = value1['te_info']
           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
 
   return (GetDiffBetweenMapsHelper(map1, map2, lookIntoTestExpectationInfo),
           GetDiffBetweenMapsHelper(map2, map1, lookIntoTestExpectationInfo))