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

media/tools/layout_tests/bug.py

-# 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.

media/tools/layout_tests/layouttest_analyzer.py

 #!/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(

media/tools/layout_tests/layouttest_analyzer_helpers_unittest.py

 #!/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/'

media/tools/layout_tests/layouttest_analyzer_runner.py

 #!/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()

media/tools/layout_tests/layouttests.py

-# 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:

media/tools/layout_tests/layouttests_unittest.py

 #!/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')

media/tools/layout_tests/test_expectations.py

-# 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.
 

media/tools/layout_tests/test_expectations_history.py

-# 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.

media/tools/layout_tests/test_expectations_history_unittest.py

 #!/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
 

media/tools/layout_tests/test_expectations_unittest.py

 #!/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}

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,

media/tools/layout_tests/trend_graph.py

-# 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])) + ','

media/tools/layout_tests/trend_graph_unittest.py

 #!/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__':