Adds support for excluding WONTFIX from layout test formatting and sorting of test failures by path.

Also fixes other minor issues like not being able to pass absolute paths for output. 

R=ojan
BUG=none
TEST=run test_output_formatter.py, tests should not include WONFIX and sort properly

Review URL: http://codereview.chromium.org/174097

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

4 files changed, 101 insertions, 63 deletions

diff --git a/webkit/tools/layout_tests/layout_package/failure.py b/webkit/tools/layout_tests/layout_package/failure.py
index de3157c..2c2f0d8 100644
--- a/webkit/tools/layout_tests/layout_package/failure.py
+++ b/webkit/tools/layout_tests/layout_package/failure.py
@@ -34,6 +34,12 @@ WEBKIT_TRAC_IMAGE_BASELINE_BASE_URL_WIN = WEBKIT_PLATFORM_URL_BASE + "/win/"
 
 LAYOUT_TEST_RESULTS_DIR = "layout-test-results"
 
+FAIL = "FAIL"
+TIMEOUT = "TIMEOUT"
+CRASH = "CRASH"
+PASS = "PASS"
+WONTFIX = "WONTFIX"
+
 class Failure(object):
   """
   This class represents a failure in the test output, and is
@@ -163,16 +169,19 @@ class Failure(object):
   # in order to know whether to retrieve expected test results for it.
   # (test results dont exist for tests expected to fail/crash.)
   def IsExpectedToFail(self):
-    return self._FindKeywordInExpectations("FAIL")
+    return self._FindKeywordInExpectations(FAIL)
 
   def IsExpectedToTimeout(self):
-    return self._FindKeywordInExpectations("TIMEOUT")
+    return self._FindKeywordInExpectations(TIMEOUT)
 
   def IsExpectedToCrash(self):
-    return self._FindKeywordInExpectations("CRASH")
+    return self._FindKeywordInExpectations(CRASH)
 
   def IsExpectedToPass(self):
-    return self._FindKeywordInExpectations("PASS")
+    return self._FindKeywordInExpectations(PASS)
+
+  def IsWontFix(self):
+    return self._FindKeywordInExpectations(WONTFIX)
 
   def _FindKeywordInExpectations(self, keyword):
     if (not self.test_expectations_line or
diff --git a/webkit/tools/layout_tests/layout_package/failure_finder.py b/webkit/tools/layout_tests/layout_package/failure_finder.py
index 65b5564..392d2f9 100644
--- a/webkit/tools/layout_tests/layout_package/failure_finder.py
+++ b/webkit/tools/layout_tests/layout_package/failure_finder.py
@@ -81,6 +81,8 @@ WEBKIT_FILE_AGE_REGEX = ('<a class="file" title="View File" href="%s">.*?</a>.'
 
 UPSTREAM_IMAGE_FILE_ENDING = "-upstream.png"
 
+TEST_EXPECTATIONS_WONTFIX = "WONTFIX"
+
 def GetURLBase(use_fyi):
   if use_fyi:
     return FYI_BUILDER_BASE
@@ -109,16 +111,15 @@ def IsLinuxPlatform(platform):
 def IsMacPlatform(platform):
   return (platform and platform.find("Mac") > -1)
 
-def CreateDirectory(dirname):
+def CreateDirectory(dir):
   """
   Method that creates the directory structure given.
   This will create directories recursively until the given dir exists.
   """
-  dir = "./" + dirname
   if not os.path.isdir(dir):
     if not os.path.isdir(dir[0:dir.rfind("/")]):
       CreateDirectory(dir[0:dir.rfind("/")])
-    os.mkdir("./" + dirname + "/")
+    os.mkdir(dir)
 
 def ExtractFirstValue(string, regex):
   m = re.search(regex, string)
@@ -170,6 +171,18 @@ def GenerateTextDiff(file1, file2, output_file):
   output.write("\n".join(diffs))
   output.close()
 
+class BaselineCandidate(object):
+  """Simple data object for holding the URL and local file path of a
+  possible baseline.  The local file path is meant to refer to the locally-
+  cached version of the file at the URL."""
+
+  def __init__(self, local, url):
+    self.local_file = local
+    self.baseline_url = url
+
+  def IsValid(self):
+    return self.local_file != None and self.baseline_url != None
+
 class FailureFinder(object):
 
   def __init__(self,
@@ -188,6 +201,7 @@ class FailureFinder(object):
     # TODO(gwilson): add full url-encoding for the platform.
     self.SetPlatform(builder_name)
     self.exclude_known_failures = exclude_known_failures
+    self.exclude_wontfix = True
     self.test_regex = test_regex
     self.output_dir = output_dir
     self.max_failures = max_failures
@@ -252,7 +266,7 @@ class FailureFinder(object):
       print "%s failures found." % len(matches)
 
     failures = []
-
+    matches.sort()
     for match in matches:
       if (len(failures) < self.max_failures and
           (not self.test_regex or match[0].find(self.test_regex) > -1)):
@@ -359,15 +373,19 @@ class FailureFinder(object):
         print 'Extracting files...'
       directory = "%s/layout-test-results-%s" % (self.output_dir, self.build)
       CreateDirectory(directory)
+
       for failure in self.failures:
+        failure.test_expectations_line = (
+          self._GetTestExpectationsLine(failure.test_path))
+        if self.exclude_wontfix and failure.IsWontFix():
+          self.failures.remove(failure)
+          continue
         if failure.text_diff_mismatch or failure.simplified_text_diff_mismatch:
           self._PopulateTextFailure(failure, directory, zip)
         if failure.image_mismatch:
           self._PopulateImageFailure(failure, directory, zip)
         failure.test_age = self._GetFileAge(failure.GetTestHome())
         failure.flakiness = self._GetFlakiness(failure.test_path, self.platform)
-        failure.test_expectations_line = (
-          self._GetTestExpectationsLine(failure.test_path, self.platform))
       zip.close()
       if self.verbose:
         print "Files extracted."
@@ -393,15 +411,15 @@ class FailureFinder(object):
     return (revision, build_name)
 
   def _PopulateTextFailure(self, failure, directory, zip):
-    baselines = self._GetBaseline(failure.GetExpectedTextFilename(),
-                                  directory)
-    failure.text_baseline_local = baselines[0]
-    failure.text_baseline_url = baselines[1]
+    baseline = self._GetBaseline(failure.GetExpectedTextFilename(), directory)
+    failure.text_baseline_local = baseline.local_file
+    failure.text_baseline_url = baseline.baseline_url
     failure.text_baseline_age = (
       self._GetFileAge(failure.GetTextBaselineTracHome()))
     failure.text_actual_local = "%s/%s" % (directory,
                                            failure.GetActualTextFilename())
-    if (not self.dont_download and
+    if (baseline and baseline.IsValid() and
+        not self.dont_download and
         self._ExtractFileFromZip(zip,
                                  failure.GetTextResultLocationInZipFile(),
                                  failure.text_actual_local)):
@@ -410,11 +428,10 @@ class FailureFinder(object):
                        directory + "/" + failure.GetTextDiffFilename())
 
   def _PopulateImageFailure(self, failure, directory, zip):
-    baselines = self._GetBaseline(failure.GetExpectedImageFilename(),
-                                   directory)
-    failure.image_baseline_local = baselines[0]
-    failure.image_baseline_url = baselines[1]
-    if baselines[0] and baselines[1]:
+    baseline = self._GetBaseline(failure.GetExpectedImageFilename(), directory)
+    failure.image_baseline_local = baseline.local_file
+    failure.image_baseline_url = baseline.baseline_url
+    if baseline and baseline.IsValid():
       failure.image_baseline_age = (
         self._GetFileAge(failure.GetImageBaselineTracHome()))
       failure.image_actual_local = "%s/%s" % (directory,
@@ -422,18 +439,18 @@ class FailureFinder(object):
       self._ExtractFileFromZip(zip,
                                failure.GetImageResultLocationInZipFile(),
                                failure.image_actual_local)
-      if (not GeneratePNGDiff("./" + failure.image_baseline_local,
-                             "./" + failure.image_actual_local,
-                             "./%s/%s" %
+      if (not GeneratePNGDiff(failure.image_baseline_local,
+                              failure.image_actual_local,
+                             "%s/%s" %
                              (directory, failure.GetImageDiffFilename()))
           and self.verbose):
         print "Could not generate PNG diff for %s" % failure.test_path
       if failure.IsImageBaselineInChromium():
-        upstream_baselines = (
+        upstream_baseline = (
           self._GetUpstreamBaseline(failure.GetExpectedImageFilename(),
                                     directory))
-        failure.image_baseline_upstream_local = upstream_baselines[0]
-        failure.image_baseline_upstream_url = upstream_baselines[1]
+        failure.image_baseline_upstream_local = upstream_baseline.local_file
+        failure.image_baseline_upstream_url = upstream_baseline.baseline_url
 
   def _GetBaseline(self, filename, directory, upstream_only = False):
     """ Search and download the baseline for the given test (put it in the
@@ -472,7 +489,7 @@ class FailureFinder(object):
     if local_baseline and self.verbose:
       print "Found baseline: %s" % url_of_baseline
 
-    return (local_baseline, url_of_baseline)
+    return BaselineCandidate(local_baseline, url_of_baseline)
 
   # TODO(gwilson): Refactor this with path_utils' candidate directory
   # generation.
@@ -583,7 +600,7 @@ class FailureFinder(object):
       try:
         if self.test_expectations_file:
           log = open(self.test_expectations_file, 'r')
-          self._test_expectations_cache = "".join(log.readlines())
+          self._test_expectations_cache = "\n".join(log.readlines())
         else:
           self._test_expectations_cache = ScrapeURL(TEST_EXPECTATIONS_URL)
       except HTTPError:
@@ -592,32 +609,19 @@ class FailureFinder(object):
 
     return self._test_expectations_cache
 
-  def _GetTestExpectationsLine(self, test_path, target_platform):
+  def _GetTestExpectationsLine(self, test_path):
     content = self._GetTestExpectations()
 
     if not content:
       return None
 
-    translated_platform = "WIN"
-    if IsMacPlatform(target_platform):
-      translated_platform = "MAC"
-    if IsLinuxPlatform(target_platform):
-      translated_platform = "LINUX"
-
-    test_parent_dir = test_path[:test_path.rfind("/")]
-    # TODO(gwilson): change this to look in (recursively?) higher paths.
-    possible_lines = [
-      TEST_EXPECTATIONS_PLATFORM_REGEX % (translated_platform, test_path),
-      TEST_EXPECTATIONS_NO_PLATFORM_REGEX % (test_path),
-      TEST_EXPECTATIONS_PLATFORM_REGEX % (translated_platform, test_parent_dir),
-      TEST_EXPECTATIONS_NO_PLATFORM_REGEX % (test_parent_dir)
-    ]
-
-    line = None
-    for regex in possible_lines:
-      if not line:
-        line = ExtractFirstValue(content, regex)
-    return line
+    for match in content.splitlines():
+      line = re.search(".*? : (.*?) = .*", match)
+      if line and test_path.find(line.group(1)) > -1:
+        return match
+
+    return None
+
 
   def _ExtractFileFromZip(self, zip, file_in_zip, file_to_create):
     modifiers = ""
diff --git a/webkit/tools/layout_tests/layout_package/failure_finder_test.py b/webkit/tools/layout_tests/layout_package/failure_finder_test.py
index 936be3e..61f5460 100644
--- a/webkit/tools/layout_tests/layout_package/failure_finder_test.py
+++ b/webkit/tools/layout_tests/layout_package/failure_finder_test.py
@@ -99,6 +99,16 @@ DOM_TEST_EXPECTATION_UPSTREAM = ("LayoutTests/fast/dom/"
                                  "attribute-downcast-right-"
                                  "expected-upstream.png")
 
+TEST_EXPECTATIONS = """
+BUG1234 WONTFIX : LayoutTests/fast/backgrounds/svg-as-background-3.html = FAIL
+BUG3456 WIN : LayoutTests/fast/backgrounds/svg-as-background-6.html = CRASH
+BUG4567 : LayoutTests/fast/backgrounds/svg-as-mask.html = PASS
+WONTFIX : LayoutTests/fast/block/ = FAIL
+"""
+
+EXPECT_EXACT_MATCH = "LayoutTests/fast/backgrounds/svg-as-background-6.html"
+EXPECT_GENERAL_MATCH = "LayoutTests/fast/block/float/013.html"
+EXPECT_NO_MATCH = "LayoutTests/fast/backgrounds/svg-as-background-99.html"
 
 WEBKIT_ORG = "webkit.org"
 CHROMIUM_ORG = "chromium.org"
@@ -118,6 +128,7 @@ class FailureFinderTest(object):
              "testUseLocalOutput",
              "testTranslateBuildToZip",
              "testGetBaseline",
+             "testFindTestExpectations",
              "testFull"]
 
     for test in tests:
@@ -191,10 +202,11 @@ def testFindMatchesInBuilderOutput():
 def _testBaseline(test_name, expected_local, expected_url):
   test = _getBasicFailureFinder()
   # Test baseline that is obviously in Chromium's tree.
-  local, url = test._GetBaseline(test_name, ".", False)
+  baseline = test._GetBaseline(test_name, ".", False)
   try:
-    os.remove(local)
-    if (local != expected_local or url != expected_url):
+    os.remove(baseline.local_file)
+    if (baseline.local_file != expected_local or
+        baseline.baseline_url != expected_url):
       return False
   except:
     return False
@@ -265,23 +277,22 @@ def testTranslateBuildToZip():
 def testGetBaseline():
   test = _getBasicFailureFinder()
   result = True
-  #test.dont_download = True
   test.platform = "chromium-mac"
-  local, url = test._GetBaseline(WEBARCHIVE_TEST_EXPECTATION, ".")
-  if not local or url.find(WEBKIT_ORG) == -1:
+  baseline = test._GetBaseline(WEBARCHIVE_TEST_EXPECTATION, ".")
+  if not baseline.local_file or baseline.baseline_url.find(WEBKIT_ORG) == -1:
     result = False
     print "Webarchive layout test not found at webkit.org: %s" % url
   test.platform = "chromium-win"
-  local, url = test._GetBaseline(SVG_TEST_EXPECTATION, ".")
-  if not local or url.find(CHROMIUM_ORG) == -1:
+  baseline = test._GetBaseline(SVG_TEST_EXPECTATION, ".")
+  if not baseline.local_file or baseline.baseline_url.find(CHROMIUM_ORG) == -1:
     result = False
     print "SVG layout test found at %s, not chromium.org" % url
-  local, url = test._GetBaseline(SVG_TEST_EXPECTATION, ".", True)
-  if not local or url.find(WEBKIT_ORG) == -1:
+  baseline = test._GetBaseline(SVG_TEST_EXPECTATION, ".", True)
+  if not baseline.local_file or baseline.baseline_url.find(WEBKIT_ORG) == -1:
     result = False
     print "Upstream SVG layout test NOT found at webkit.org!"
-  local, url = test._GetBaseline(DOM_TEST_EXPECTATION, ".", True)
-  if not local or url.find("/platform/") > -1:
+  baseline = test._GetBaseline(DOM_TEST_EXPECTATION, ".", True)
+  if not baseline.local_file or baseline.baseline_url.find("/platform/") > -1:
     result = False
     print "Upstream SVG layout test found in a platform directory: %s" % url
   os.remove(WEBARCHIVE_TEST_EXPECTATION)
@@ -321,6 +332,19 @@ def testFull():
 
   return True
 
+def testFindTestExpectations():
+  test = _getBasicFailureFinder()
+  test._test_expectations_cache = TEST_EXPECTATIONS
+  match = test._GetTestExpectationsLine(EXPECT_EXACT_MATCH)
+  if not match:
+    return False
+  match = test._GetTestExpectationsLine(EXPECT_GENERAL_MATCH)
+  if not match:
+    return False
+  match = test._GetTestExpectationsLine(EXPECT_NO_MATCH)
+  return not match
+
+
 if __name__ == "__main__":
   fft = FailureFinderTest()
   result = fft.runTests()
diff --git a/webkit/tools/layout_tests/layout_package/html_generator.py b/webkit/tools/layout_tests/layout_package/html_generator.py
index 9136018..ccf2604 100644
--- a/webkit/tools/layout_tests/layout_package/html_generator.py
+++ b/webkit/tools/layout_tests/layout_package/html_generator.py
@@ -76,9 +76,10 @@ class HTMLGenerator(object):
                 <tr>
                   <td style="background-color: #CDECDE;
                     border-bottom: 1px solid black;">
-                    <span class="titlelink">%s</span></td></tr>
+                    <span class="titlelink">%s.&nbsp;&nbsp;%s</span></td></tr>
                 <tr><td>&nbsp;&nbsp;Last modified: <a href="%s">%s</a>
-              """ % (failure.test_path,
+              """ % (test_number,
+                     failure.test_path,
                      failure.GetTestHome(),
                      failure.test_age)
       html += "<div class='detail'>"