2010-02-14 Dirk Pranke <dpranke@chromium.org>
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Feb 2010 04:58:26 +0000 (04:58 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Feb 2010 04:58:26 +0000 (04:58 +0000)
        Reviewed by Eric Seidel.

        Update rebaseline-chromium-webkit-tests to work with the new code
        structure (port objects instead of path_utils and platform_utils).

        Added a path_to_test_expectations_file() to the Port interface.

        Fixed a bug in the chromium_* platform implementations where the
        'target' option was assumed to be set.

        * Scripts/rebaseline-chromium-webkit-tests:
        * Scripts/webkitpy/layout_tests/layout_package/test_expectations.py:
        * Scripts/webkitpy/layout_tests/port/base.py:
        * Scripts/webkitpy/layout_tests/port/chromium.py:
        * Scripts/webkitpy/layout_tests/port/chromium_linux.py:
        * Scripts/webkitpy/layout_tests/port/chromium_mac.py:
        * Scripts/webkitpy/layout_tests/port/chromium_win.py:
        * Scripts/webkitpy/layout_tests/port/test.py:
        * Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@54760 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebKitTools/ChangeLog
WebKitTools/Scripts/rebaseline-chromium-webkit-tests
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_expectations.py
WebKitTools/Scripts/webkitpy/layout_tests/port/base.py
WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py
WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_linux.py
WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py
WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_win.py
WebKitTools/Scripts/webkitpy/layout_tests/port/test.py
WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py

index 67b8806..693a568 100644 (file)
@@ -1,3 +1,25 @@
+2010-02-14  Dirk Pranke  <dpranke@chromium.org>
+
+        Reviewed by Eric Seidel.
+
+        Update rebaseline-chromium-webkit-tests to work with the new code
+        structure (port objects instead of path_utils and platform_utils).
+
+        Added a path_to_test_expectations_file() to the Port interface.
+  
+        Fixed a bug in the chromium_* platform implementations where the
+        'target' option was assumed to be set.
+
+        * Scripts/rebaseline-chromium-webkit-tests:
+        * Scripts/webkitpy/layout_tests/layout_package/test_expectations.py:
+        * Scripts/webkitpy/layout_tests/port/base.py:
+        * Scripts/webkitpy/layout_tests/port/chromium.py:
+        * Scripts/webkitpy/layout_tests/port/chromium_linux.py:
+        * Scripts/webkitpy/layout_tests/port/chromium_mac.py:
+        * Scripts/webkitpy/layout_tests/port/chromium_win.py:
+        * Scripts/webkitpy/layout_tests/port/test.py:
+        * Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py:
+
 2010-02-14  Eric Seidel  <eric@webkit.org>
 
         Reviewed by Darin Adler.
index aea0edc..302995c 100755 (executable)
@@ -33,6 +33,7 @@ import sys
 
 sys.path.append(os.path.join(os.path.dirname(os.path.abspath(sys.argv[0])),
                              "webkitpy", "layout_tests"))
+sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(sys.argv[0]))))
 
 import rebaseline_chromium_webkit_tests
 
index a3650ed..01add62 100644 (file)
@@ -35,7 +35,6 @@ import logging
 import os
 import re
 import sys
-import time
 
 import simplejson
 
@@ -133,10 +132,9 @@ class TestExpectations:
     def has_modifier(self, test, modifier):
         return self._expected_failures.has_modifier(test, modifier)
 
-    def remove_platform_from_file(self, tests, platform, backup=False):
-        return self._expected_failures.remove_platform_from_file(tests,
-                                                                 platform,
-                                                                 backup)
+    def remove_platform_from_expectations(self, tests, platform):
+        return self._expected_failures.remove_platform_from_expectations(
+            tests, platform)
 
 
 def strip_comments(line):
@@ -373,8 +371,9 @@ class TestExpectationsFile:
     def contains(self, test):
         return test in self._test_to_expectations
 
-    def remove_platform_from_file(self, tests, platform, backup=False):
-        """Remove the platform option from test expectations file.
+    def remove_platform_from_expectations(self, tests, platform):
+        """Returns a copy of the expectations with the tests matching the
+        platform remove.
 
         If a test is in the test list and has an option that matches the given
         platform, remove the matching platform and save the updated test back
@@ -384,24 +383,13 @@ class TestExpectationsFile:
         Args:
           tests: list of tests that need to update..
           platform: which platform option to remove.
-          backup: if true, the original test expectations file is saved as
-                  [self.TEST_LIST].orig.YYYYMMDDHHMMSS
 
         Returns:
-          no
+          the updated string.
         """
 
-        # FIXME - remove_platform_from file worked by writing a new
-        # test_expectations.txt file over the old one. Now that we're just
-        # parsing strings, we need to change this to return the new
-        # expectations string.
-        raise NotImplementedException('remove_platform_from_file')
-
-        new_file = self._path + '.new'
-        logging.debug('Original file: "%s"', self._path)
-        logging.debug('New file: "%s"', new_file)
         f_orig = self._get_iterable_expectations()
-        f_new = open(new_file, 'w')
+        f_new = []
 
         tests_removed = 0
         tests_updated = 0
@@ -413,7 +401,7 @@ class TestExpectationsFile:
             if action == NO_CHANGE:
                 # Save the original line back to the file
                 logging.debug('No change to test: %s', line)
-                f_new.write(line)
+                f_new.append(line)
             elif action == REMOVE_TEST:
                 tests_removed += 1
                 logging.info('Test removed: %s', line)
@@ -421,7 +409,7 @@ class TestExpectationsFile:
                 parts = line.split(':')
                 new_options = parts[0].replace(platform.upper() + ' ', '', 1)
                 new_line = ('%s:%s' % (new_options, parts[1]))
-                f_new.write(new_line)
+                f_new.append(new_line)
                 tests_updated += 1
                 logging.info('Test updated: ')
                 logging.info('  old: %s', line)
@@ -440,7 +428,7 @@ class TestExpectationsFile:
                     if not p in (platform.upper(), 'WIN-VISTA', 'WIN-7'):
                         new_options += p + ' '
                 new_line = ('%s:%s' % (new_options, parts[1]))
-                f_new.write(new_line)
+                f_new.append(new_line)
                 tests_updated += 1
                 logging.info('Test updated: ')
                 logging.info('  old: %s', line)
@@ -452,23 +440,7 @@ class TestExpectationsFile:
         logging.info('Total tests removed: %d', tests_removed)
         logging.info('Total tests updated: %d', tests_updated)
 
-        f_orig.close()
-        f_new.close()
-
-        if backup:
-            date_suffix = time.strftime('%Y%m%d%H%M%S',
-                                        time.localtime(time.time()))
-            backup_file = ('%s.orig.%s' % (self._path, date_suffix))
-            if os.path.exists(backup_file):
-                os.remove(backup_file)
-            logging.info('Saving original file to "%s"', backup_file)
-            os.rename(self._path, backup_file)
-        else:
-            os.remove(self._path)
-
-        logging.debug('Saving new file to "%s"', self._path)
-        os.rename(new_file, self._path)
-        return True
+        return "".join(f_new)
 
     def parse_expectations_line(self, line, lineno):
         """Parses a line from test_expectations.txt and returns a tuple
index ce06b44..e0c7a3f 100644 (file)
@@ -83,18 +83,20 @@ class Port(object):
         interface so that it can be overriden for testing purposes."""
         return actual_text != expected_text
 
-    def diff_image(self, actual_filename, expected_filename, diff_filename):
+    def diff_image(self, actual_filename, expected_filename,
+                   diff_filename=None):
         """Compare two image files and produce a delta image file.
 
         Return 1 if the two files are different, 0 if they are the same.
         Also produce a delta image of the two images and write that into
-        |diff_filename|.
+        |diff_filename| if it is not None.
 
         While this is a generic routine, we include it in the Port
         interface so that it can be overriden for testing purposes."""
         executable = self._path_to_image_diff()
-        cmd = [executable, '--diff', actual_filename, expected_filename,
-               diff_filename]
+        cmd = [executable, '--diff', actual_filename, expected_filename]
+        if diff_filename:
+            cmd.append(diff_filename)
         result = 1
         try:
             result = subprocess.call(cmd)
@@ -275,6 +277,13 @@ class Port(object):
             self._webkit_base_dir = abspath[0:abspath.find('WebKitTools')]
         return os.path.join(self._webkit_base_dir, *comps)
 
+    def path_to_test_expectations_file(self):
+        """Update the test expectations to the passed-in string.
+
+        This is used by the rebaselining tool. Raises NotImplementedError
+        if the port does not use expectations files."""
+        raise NotImplementedError('Port.path_to_test_expectations_file')
     def remove_directory(self, *path):
         """Recursively removes a directory, even if it's marked read-only.
 
index 70a8dea..a175343 100644 (file)
@@ -88,6 +88,10 @@ class ChromiumPort(base.Port):
             self._chromium_base_dir = abspath[0:abspath.find('third_party')]
         return os.path.join(self._chromium_base_dir, *comps)
 
+    def path_to_test_expectations_file(self):
+        return self.path_from_chromium_base('webkit', 'tools', 'layout_tests',
+                                            'test_expectations.txt')
+
     def results_directory(self):
         return self.path_from_chromium_base('webkit', self._options.target,
                                             self._options.results_directory)
@@ -133,8 +137,7 @@ class ChromiumPort(base.Port):
 
         Basically this string should contain the equivalent of a
         test_expectations file. See test_expectations.py for more details."""
-        expectations_file = self.path_from_chromium_base('webkit', 'tools',
-            'layout_tests', 'test_expectations.txt')
+        expectations_file = self.path_to_test_expectations_file()
         return file(expectations_file, "r").read()
 
     def test_platform_names(self):
@@ -154,7 +157,6 @@ class ChromiumPort(base.Port):
         return self.path_from_chromium_base('webkit', 'data', 'layout_tests',
             'platform', platform, 'LayoutTests')
 
-
 class ChromiumDriver(base.Driver):
     """Abstract interface for the DumpRenderTree interface."""
 
index 8fd5343..b817251 100644 (file)
@@ -43,6 +43,8 @@ class ChromiumLinuxPort(chromium.ChromiumPort):
     def __init__(self, port_name=None, options=None):
         if port_name is None:
             port_name = 'chromium-linux'
+        if options and not hasattr(options, 'target'):
+            options.target = 'Release'
         chromium.ChromiumPort.__init__(self, port_name, options)
 
     def baseline_search_path(self):
index 7e7b4ca..bcffcf8 100644 (file)
@@ -43,6 +43,8 @@ class ChromiumMacPort(chromium.ChromiumPort):
     def __init__(self, port_name=None, options=None):
         if port_name is None:
             port_name = 'chromium-mac'
+        if options and not hasattr(options, 'target'):
+            options.target = 'Release'
         chromium.ChromiumPort.__init__(self, port_name, options)
 
     def baseline_search_path(self):
index 352916c..5eb0ba1 100644 (file)
@@ -44,6 +44,8 @@ class ChromiumWinPort(chromium.ChromiumPort):
     def __init__(self, port_name=None, options=None):
         if port_name is None:
             port_name = 'chromium-win' + self.version()
+        if options and not hasattr(options, 'target'):
+            options.target = 'Release'
         chromium.ChromiumPort.__init__(self, port_name, options)
 
     def baseline_search_path(self):
index 0bc6e7c..c3e97be 100644 (file)
@@ -55,7 +55,8 @@ class TestPort(base.Port):
     def check_sys_deps(self):
         return True
 
-    def diff_image(self, actual_filename, expected_filename, diff_filename):
+    def diff_image(self, actual_filename, expected_filename,
+                   diff_filename=None):
         return False
 
     def compare_text(self, actual_text, expected_text):
index 83cf99d..4604a1a 100644 (file)
@@ -54,7 +54,7 @@ import urllib
 import webbrowser
 import zipfile
 
-from layout_package import path_utils
+import port
 from layout_package import test_expectations
 from test_types import image_diff
 from test_types import text_diff
@@ -200,9 +200,10 @@ class Rebaseliner(object):
 
     REVISION_REGEX = r'<a href=\"(\d+)/\">'
 
-    def __init__(self, platform, options):
-        self._file_dir = path_utils.path_from_base('webkit', 'tools',
+    def __init__(self, port, platform, options):
+        self._file_dir = port.path_from_chromium_base('webkit', 'tools',
             'layout_tests')
+        self._port = port
         self._platform = platform
         self._options = options
         self._rebaselining_tests = []
@@ -212,10 +213,12 @@ class Rebaseliner(object):
         #   -. compile list of tests that need rebaselining.
         #   -. update the tests in test_expectations file after rebaseline
         #      is done.
+        expectations_str = self._port.test_expectations()
         self._test_expectations = \
-            test_expectations.TestExpectations(None,
-                                               self._file_dir,
-                                               platform,
+            test_expectations.TestExpectations(self._port,
+                                               None,
+                                               expectations_str,
+                                               self._platform,
                                                False,
                                                False)
 
@@ -359,7 +362,6 @@ class Rebaseliner(object):
         latest_revision = self._get_latest_revision(url_base)
         if latest_revision is None or latest_revision <= 0:
             return None
-
         archive_url = ('%s%s/layout-test-results.zip' % (url_base,
                                                          latest_revision))
         logging.info('Archive url: "%s"', archive_url)
@@ -399,7 +401,7 @@ class Rebaseliner(object):
         for name in zip_namelist:
             logging.debug('  ' + name)
 
-        platform = path_utils.platform_name(self._platform)
+        platform = self._port.name()
         logging.debug('Platform dir: "%s"', platform)
 
         test_no = 1
@@ -412,7 +414,7 @@ class Rebaseliner(object):
             test_basename = os.path.splitext(test)[0]
             for suffix in BASELINE_SUFFIXES:
                 archive_test_name = ('layout-test-results/%s-actual%s' %
-                                     (test_basename, suffix))
+                                      (test_basename, suffix))
                 logging.debug('  Archive test file name: "%s"',
                               archive_test_name)
                 if not archive_test_name in zip_namelist:
@@ -431,7 +433,7 @@ class Rebaseliner(object):
 
                 expected_filename = '%s-expected%s' % (test_basename, suffix)
                 expected_fullpath = os.path.join(
-                    path_utils.chromium_baseline_path(platform),
+                    self._port._chromium_baseline_path(platform),
                     expected_filename)
                 expected_fullpath = os.path.normpath(expected_fullpath)
                 logging.debug('  Expected file full path: "%s"',
@@ -443,17 +445,17 @@ class Rebaseliner(object):
                 # and lower
                 # levels and remove all duplicated baselines.
                 if self._is_dup_baseline(temp_name,
-                                       expected_fullpath,
-                                       test,
-                                       suffix,
-                                       self._platform):
+                                        expected_fullpath,
+                                        test,
+                                        suffix,
+                                        self._platform):
                     os.remove(temp_name)
                     self._delete_baseline(expected_fullpath)
                     continue
 
                 # Create the new baseline directory if it doesn't already
                 # exist.
-                path_utils.maybe_make_directory(
+                self._port.maybe_make_directory(
                     os.path.dirname(expected_fullpath))
 
                 shutil.move(temp_name, expected_fullpath)
@@ -497,9 +499,9 @@ class Rebaseliner(object):
           True if the baseline is unnecessary.
           False otherwise.
         """
-        test_filepath = os.path.join(path_utils.layout_tests_dir(), test)
-        all_baselines = path_utils.expected_baselines(test_filepath,
-                                                      suffix, platform, True)
+        test_filepath = os.path.join(self._port.layout_tests_dir(), test)
+        all_baselines = self._port.expected_baselines(test_filepath,
+                                                      suffix, True)
         for (fallback_dir, fallback_file) in all_baselines:
             if fallback_dir and fallback_file:
                 fallback_fullpath = os.path.normpath(
@@ -534,11 +536,11 @@ class Rebaseliner(object):
             return True
 
         if ext1 == '.PNG':
-            return image_diff.ImageDiff(self._platform, '').diff_files(file1,
-                                                                       file2)
+            return image_diff.ImageDiff(self._port, self._platform,
+                '').diff_files(self._port, file1, file2)
         else:
-            return text_diff.TestTextDiff(self._platform, '').diff_files(file1,
-                                                                         file2)
+            return text_diff.TestTextDiff(self._port, self._platform,
+                '').diff_files(self._port, file1, file2)
 
     def _delete_baseline(self, filename):
         """Remove the file from repository and delete it from disk.
@@ -570,8 +572,21 @@ class Rebaseliner(object):
         """
 
         if self._rebaselined_tests:
-            self._test_expectations.remove_platform_from_file(
-                self._rebaselined_tests, self._platform, backup)
+            new_expectations = (
+                self._test_expectations.remove_platform_from_expectations(
+                self._rebaselined_tests, self._platform))
+            path = self._port.path_to_test_expectations_file()
+            if backup:
+                date_suffix = time.strftime('%Y%m%d%H%M%S',
+                                            time.localtime(time.time()))
+                backup_file = ('%s.orig.%s' % (path, date_suffix))
+                if os.path.exists(backup_file):
+                    os.remove(backup_file)
+                logging.info('Saving original file to "%s"', backup_file)
+                os.rename(path, backup_file)
+            f = open(path, "w")
+            f.write(new_expectations)
+            f.close()
         else:
             logging.info('No test was rebaselined so nothing to remove.')
 
@@ -777,8 +792,9 @@ class HtmlGenerator(object):
                         '<img style="width: 200" src="%(uri)s" /></a></td>')
     HTML_TR = '<tr>%s</tr>'
 
-    def __init__(self, options, platforms, rebaselining_tests):
+    def __init__(self, port, options, platforms, rebaselining_tests):
         self._html_directory = options.html_directory
+        self._port = port
         self._platforms = platforms
         self._rebaselining_tests = rebaselining_tests
         self._html_file = os.path.join(options.html_directory,
@@ -817,7 +833,7 @@ class HtmlGenerator(object):
 
         logging.info('Launching html: "%s"', self._html_file)
 
-        html_uri = path_utils.filename_to_uri(self._html_file)
+        html_uri = self._port.filename_to_uri(self._html_file)
         webbrowser.open(html_uri, 1)
 
         logging.info('Html launched.')
@@ -855,13 +871,13 @@ class HtmlGenerator(object):
         links = ''
         if os.path.exists(old_file):
             links += html_td_link % {
-                'uri': path_utils.filename_to_uri(old_file),
+                'uri': self._port.filename_to_uri(old_file),
                 'name': baseline_filename}
         else:
             logging.info('    No old baseline file: "%s"', old_file)
             links += self.HTML_TD_NOLINK % ''
 
-        links += html_td_link % {'uri': path_utils.filename_to_uri(new_file),
+        links += html_td_link % {'uri': self._port.filename_to_uri(new_file),
                                  'name': baseline_filename}
 
         diff_file = get_result_file_fullpath(self._html_directory,
@@ -869,7 +885,7 @@ class HtmlGenerator(object):
                                              'diff')
         logging.info('    Baseline diff file: "%s"', diff_file)
         if os.path.exists(diff_file):
-            links += html_td_link % {'uri': path_utils.filename_to_uri(
+            links += html_td_link % {'uri': self._port.filename_to_uri(
                 diff_file), 'name': 'Diff'}
         else:
             logging.info('    No baseline diff file: "%s"', diff_file)
@@ -908,8 +924,8 @@ class HtmlGenerator(object):
                     rows.append(self.HTML_TR % row)
 
         if rows:
-            test_path = os.path.join(path_utils.layout_tests_dir(), test)
-            html = self.HTML_TR_TEST % (path_utils.filename_to_uri(test_path),
+            test_path = os.path.join(self._port.layout_tests_dir(), test)
+            html = self.HTML_TR_TEST % (self._port.filename_to_uri(test_path),
                 test)
             html += self.HTML_TEST_DETAIL % ' '.join(rows)
 
@@ -967,6 +983,7 @@ def main():
                                    ' rebaselining comparison.'))
 
     options = option_parser.parse_args()[0]
+    port_obj = port.get(None, options)
 
     # Set up our logging format.
     log_level = logging.INFO
@@ -1002,7 +1019,7 @@ def main():
     rebaselining_tests = set()
     backup = options.backup
     for platform in rebaseline_platforms:
-        rebaseliner = Rebaseliner(platform, options)
+        rebaseliner = Rebaseliner(port_obj, platform, options)
 
         logging.info('')
         log_dashed_string('Rebaseline started', platform)
@@ -1017,7 +1034,8 @@ def main():
 
     logging.info('')
     log_dashed_string('Rebaselining result comparison started', None)
-    html_generator = HtmlGenerator(options,
+    html_generator = HtmlGenerator(port_obj,
+                                   options,
                                    rebaseline_platforms,
                                    rebaselining_tests)
     html_generator.generate_html()