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..b8d3e1ae2b93ed71f68120087efade2444d53d4d 100644
--- a/media/tools/layout_tests/layouttest_analyzer_helpers.py
+++ b/media/tools/layout_tests/layouttest_analyzer_helpers.py
@@ -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|

[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
@@ -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))

[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):
@@ -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,46 @@ 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: 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):
@@ -244,8 +255,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
@@ -399,48 +410,65 @@ 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
+        # 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):
@@ -463,7 +491,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 +574,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