2011-01-12 Sheriff Bot <webkit.review.bot@gmail.com>
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Jan 2011 19:25:22 +0000 (19:25 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Jan 2011 19:25:22 +0000 (19:25 +0000)
        Unreviewed, rolling out r75576.
        http://trac.webkit.org/changeset/75576
        https://bugs.webkit.org/show_bug.cgi?id=52304

        broke rebaseline-chromium-webkit-test (Requested by thakis on
        #webkit).

        * Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py:
        * Scripts/webkitpy/layout_tests/layout_package/test_expectations.py:
        * Scripts/webkitpy/layout_tests/layout_package/test_expectations_unittest.py:
        * Scripts/webkitpy/layout_tests/layout_package/test_runner.py:
        * Scripts/webkitpy/layout_tests/port/base.py:
        * Scripts/webkitpy/layout_tests/port/chromium.py:
        * Scripts/webkitpy/layout_tests/port/port_testcase.py:
        * Scripts/webkitpy/layout_tests/port/test.py:
        * Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py:
        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
        * Scripts/webkitpy/style/checkers/test_expectations.py:
        * Scripts/webkitpy/style/checkers/test_expectations_unittest.py:

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

13 files changed:
Tools/ChangeLog
Tools/Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py
Tools/Scripts/webkitpy/layout_tests/layout_package/test_expectations.py
Tools/Scripts/webkitpy/layout_tests/layout_package/test_expectations_unittest.py
Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py
Tools/Scripts/webkitpy/layout_tests/port/base.py
Tools/Scripts/webkitpy/layout_tests/port/chromium.py
Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py
Tools/Scripts/webkitpy/layout_tests/port/test.py
Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py
Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
Tools/Scripts/webkitpy/style/checkers/test_expectations.py
Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py

index 2c049d12f2f8f34c1aebaa939a27446dceb24775..1fe88ce2d7afa715f74a9022c1ae26f5a64217d7 100644 (file)
@@ -1,3 +1,25 @@
+2011-01-12  Sheriff Bot  <webkit.review.bot@gmail.com>
+
+        Unreviewed, rolling out r75576.
+        http://trac.webkit.org/changeset/75576
+        https://bugs.webkit.org/show_bug.cgi?id=52304
+
+        broke rebaseline-chromium-webkit-test (Requested by thakis on
+        #webkit).
+
+        * Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py:
+        * Scripts/webkitpy/layout_tests/layout_package/test_expectations.py:
+        * Scripts/webkitpy/layout_tests/layout_package/test_expectations_unittest.py:
+        * Scripts/webkitpy/layout_tests/layout_package/test_runner.py:
+        * Scripts/webkitpy/layout_tests/port/base.py:
+        * Scripts/webkitpy/layout_tests/port/chromium.py:
+        * Scripts/webkitpy/layout_tests/port/port_testcase.py:
+        * Scripts/webkitpy/layout_tests/port/test.py:
+        * Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py:
+        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
+        * Scripts/webkitpy/style/checkers/test_expectations.py:
+        * Scripts/webkitpy/style/checkers/test_expectations_unittest.py:
+
 2011-01-12  Maciej Stachowiak  <mjs@apple.com>
 
         Reviewed by Darin Adler.
index d027e60112d4ae8f0b165d7092a1c54c0912ab3a..9280b0253d7c25e878c730183ef1e9e5a761711d 100644 (file)
@@ -31,6 +31,7 @@
 
 import os
 import optparse
+import pdb
 import sys
 import unittest
 import logging
@@ -145,7 +146,7 @@ class  Testprinter(unittest.TestCase):
                       test in tests]
         expectations = test_expectations.TestExpectations(
             self._port, test_paths, expectations_str,
-            self._port.test_configuration(),
+            self._port.test_platform_name(), is_debug_mode=False,
             is_lint_mode=False)
 
         rs = result_summary.ResultSummary(expectations, test_paths)
@@ -364,7 +365,7 @@ class  Testprinter(unittest.TestCase):
     def test_print_progress__detailed(self):
         tests = ['passes/text.html', 'failures/expected/timeout.html',
                  'failures/expected/crash.html']
-        expectations = 'BUGX : failures/expected/timeout.html = TIMEOUT'
+        expectations = 'failures/expected/timeout.html = TIMEOUT'
 
         # first, test that it is disabled properly
         # should still print one-line-progress
@@ -570,8 +571,8 @@ class  Testprinter(unittest.TestCase):
         self.assertFalse(out.empty())
 
         expectations = """
-BUGX : failures/expected/crash.html = CRASH
-BUGX : failures/expected/timeout.html = TIMEOUT
+failures/expected/crash.html = CRASH
+failures/expected/timeout.html = TIMEOUT
 """
         err.reset()
         out.reset()
index 98752c9e2d124975da8b7105b6dc3bcd5f564c3e..8645fc1dc187d6dc06cffefc7e4b9866b4e50e7c 100644 (file)
@@ -31,7 +31,6 @@
 for layout tests.
 """
 
-import itertools
 import logging
 import os
 import re
@@ -87,16 +86,18 @@ def remove_pixel_failures(expected_results):
 class TestExpectations:
     TEST_LIST = "test_expectations.txt"
 
-    def __init__(self, port, tests, expectations, test_config,
-                 is_lint_mode, overrides=None):
+    def __init__(self, port, tests, expectations, test_platform_name,
+                 is_debug_mode, is_lint_mode, overrides=None):
         """Loads and parses the test expectations given in the string.
         Args:
             port: handle to object containing platform-specific functionality
             test: list of all of the test files
             expectations: test expectations as a string
-            test_config: specific values to check against when
-                parsing the file (usually port.test_config(),
-                but may be different when linting or doing other things).
+            test_platform_name: name of the platform to match expectations
+                against. Note that this may be different than
+                port.test_platform_name() when is_lint_mode is True.
+            is_debug_mode: whether to use the DEBUG or RELEASE modifiers
+                in the expectations
             is_lint_mode: If True, just parse the expectations string
                 looking for errors.
             overrides: test expectations that are allowed to override any
@@ -105,7 +106,7 @@ class TestExpectations:
                 and downstream expectations).
         """
         self._expected_failures = TestExpectationsFile(port, expectations,
-            tests, test_config, is_lint_mode,
+            tests, test_platform_name, is_debug_mode, is_lint_mode,
             overrides=overrides)
 
     # TODO(ojan): Allow for removing skipped tests when getting the list of
@@ -198,7 +199,7 @@ class ParseError(Exception):
         return '\n'.join(map(str, self.errors))
 
     def __repr__(self):
-        return 'ParseError(fatal=%s, errors=%s)' % (self.fatal, self.errors)
+        return 'ParseError(fatal=%s, errors=%s)' % (fatal, errors)
 
 
 class ModifiersAndExpectations:
@@ -303,14 +304,28 @@ class TestExpectationsFile:
                     'fail': FAIL,
                     'flaky': FLAKY}
 
-    def __init__(self, port, expectations, full_test_list,
-                 test_config, is_lint_mode, overrides=None):
-        # See argument documentation in TestExpectation(), above.
+    def __init__(self, port, expectations, full_test_list, test_platform_name,
+        is_debug_mode, is_lint_mode, overrides=None):
+        """
+        expectations: Contents of the expectations file
+        full_test_list: The list of all tests to be run pending processing of
+            the expections for those tests.
+        test_platform_name: name of the platform to match expectations
+            against. Note that this may be different than
+            port.test_platform_name() when is_lint_mode is True.
+        is_debug_mode: Whether we testing a test_shell built debug mode.
+        is_lint_mode: Whether this is just linting test_expecatations.txt.
+        overrides: test expectations that are allowed to override any
+            entries in |expectations|. This is used by callers
+            that need to manage two sets of expectations (e.g., upstream
+            and downstream expectations).
+        """
 
         self._port = port
         self._expectations = expectations
         self._full_test_list = full_test_list
-        self._test_config = test_config
+        self._test_platform_name = test_platform_name
+        self._is_debug_mode = is_debug_mode
         self._is_lint_mode = is_lint_mode
         self._overrides = overrides
         self._errors = []
@@ -318,9 +333,7 @@ class TestExpectationsFile:
 
         # Maps relative test paths as listed in the expectations file to a
         # list of maps containing modifiers and expectations for each time
-        # the test is listed in the expectations file. We use this to
-        # keep a representation of the entire list of expectations, even
-        # invalid ones.
+        # the test is listed in the expectations file.
         self._all_expectations = {}
 
         # Maps a test to its list of expectations.
@@ -333,8 +346,7 @@ class TestExpectationsFile:
         # the options minus any bug or platform strings
         self._test_to_modifiers = {}
 
-        # Maps a test to the base path that it was listed with in the list and
-        # the number of matches that base path had.
+        # Maps a test to the base path that it was listed with in the list.
         self._test_list_paths = {}
 
         self._modifier_to_tests = self._dict_of_sets(self.MODIFIERS)
@@ -361,7 +373,13 @@ class TestExpectationsFile:
 
     def _handle_any_read_errors(self):
         if len(self._errors) or len(self._non_fatal_errors):
-            _log.error("FAILURES FOR %s" % str(self._test_config))
+            if self._is_debug_mode:
+                build_type = 'DEBUG'
+            else:
+                build_type = 'RELEASE'
+            _log.error('')
+            _log.error("FAILURES FOR PLATFORM: %s, BUILD_TYPE: %s" %
+                       (self._test_platform_name.upper(), build_type))
 
             for error in self._errors:
                 _log.error(error)
@@ -377,12 +395,11 @@ class TestExpectationsFile:
         expectations = set([PASS])
         options = []
         modifiers = []
-        num_matches = 0
         if self._full_test_list:
             for test in self._full_test_list:
                 if not test in self._test_list_paths:
-                    self._add_test(test, modifiers, num_matches, expectations,
-                                   options, overrides_allowed=False)
+                    self._add_test(test, modifiers, expectations, options,
+                        overrides_allowed=False)
 
     def _dict_of_sets(self, strings_to_constants):
         """Takes a dict of strings->constants and returns a dict mapping
@@ -521,15 +538,12 @@ class TestExpectationsFile:
 
         options = []
         if line.find(":") is -1:
-            self._add_error(lineno, "Missing a ':'", line)
-            return (None, None, None)
-
-        parts = line.split(':')
-
-        # FIXME: verify that there is exactly one colon in the line.
+            test_and_expectation = line.split("=")
+        else:
+            parts = line.split(":")
+            options = self._get_options_list(parts[0])
+            test_and_expectation = parts[1].split('=')
 
-        options = self._get_options_list(parts[0])
-        test_and_expectation = parts[1].split('=')
         test = test_and_expectation[0].strip()
         if (len(test_and_expectation) is not 2):
             self._add_error(lineno, "Missing expectations.",
@@ -575,6 +589,69 @@ class TestExpectationsFile:
 
         return REMOVE_TEST
 
+    def _has_valid_modifiers_for_current_platform(self, options, lineno,
+        test_and_expectations, modifiers):
+        """Returns true if the current platform is in the options list or if
+        no platforms are listed and if there are no fatal errors in the
+        options list.
+
+        Args:
+          options: List of lowercase options.
+          lineno: The line in the file where the test is listed.
+          test_and_expectations: The path and expectations for the test.
+          modifiers: The set to populate with modifiers.
+        """
+        has_any_platform = False
+        has_bug_id = False
+        for option in options:
+            if option in self.MODIFIERS:
+                modifiers.add(option)
+            elif option in self._port.test_platform_names():
+                has_any_platform = True
+            elif re.match(r'bug\d', option) != None:
+                self._add_error(lineno, 'Bug must be either BUGCR, BUGWK, or BUGV8_ for test: %s' %
+                                option, test_and_expectations)
+            elif option.startswith('bug'):
+                has_bug_id = True
+            elif option not in self.BUILD_TYPES:
+                self._add_error(lineno, 'Invalid modifier for test: %s' %
+                                option, test_and_expectations)
+
+        if has_any_platform and not self._match_platform(options):
+            return False
+
+        if not has_bug_id and 'wontfix' not in options:
+            # TODO(ojan): Turn this into an AddError call once all the
+            # tests have BUG identifiers.
+            self._log_non_fatal_error(lineno, 'Test lacks BUG modifier.',
+                test_and_expectations)
+
+        if 'release' in options or 'debug' in options:
+            if self._is_debug_mode and 'debug' not in options:
+                return False
+            if not self._is_debug_mode and 'release' not in options:
+                return False
+
+        if self._is_lint_mode and 'rebaseline' in options:
+            self._add_error(lineno,
+                'REBASELINE should only be used for running rebaseline.py. '
+                'Cannot be checked in.', test_and_expectations)
+
+        return True
+
+    def _match_platform(self, options):
+        """Match the list of options against our specified platform. If any
+        of the options prefix-match self._platform, return True. This handles
+        the case where a test is marked WIN and the platform is WIN-VISTA.
+
+        Args:
+          options: list of options
+        """
+        for opt in options:
+            if self._test_platform_name.startswith(opt):
+                return True
+        return False
+
     def _add_to_all_expectations(self, test, options, expectations):
         # Make all paths unix-style so the dashboard doesn't need to.
         test = test.replace('\\', '/')
@@ -587,43 +664,54 @@ class TestExpectationsFile:
         """For each test in an expectations iterable, generate the
         expectations for it."""
         lineno = 0
-        matcher = ModifierMatcher(self._test_config)
         for line in expectations:
             lineno += 1
-            self._process_line(line, lineno, matcher, overrides_allowed)
 
-    def _process_line(self, line, lineno, matcher, overrides_allowed):
-        test_list_path, options, expectations = \
-            self.parse_expectations_line(line, lineno)
-        if not expectations:
-            return
+            test_list_path, options, expectations = \
+                self.parse_expectations_line(line, lineno)
+            if not expectations:
+                continue
 
-        self._add_to_all_expectations(test_list_path,
-                                        " ".join(options).upper(),
-                                        " ".join(expectations).upper())
+            self._add_to_all_expectations(test_list_path,
+                                          " ".join(options).upper(),
+                                          " ".join(expectations).upper())
 
-        num_matches = self._check_options(matcher, options, lineno,
-                                          test_list_path)
-        if num_matches == ModifierMatcher.NO_MATCH:
-            return
+            modifiers = set()
+            if options and not self._has_valid_modifiers_for_current_platform(
+                options, lineno, test_list_path, modifiers):
+                continue
 
-        expectations = self._parse_expectations(expectations, lineno,
-            test_list_path)
+            expectations = self._parse_expectations(expectations, lineno,
+                test_list_path)
 
-        self._check_options_against_expectations(options, expectations,
-            lineno, test_list_path)
+            if 'slow' in options and TIMEOUT in expectations:
+                self._add_error(lineno,
+                    'A test can not be both slow and timeout. If it times out '
+                    'indefinitely, then it should be just timeout.',
+                    test_list_path)
 
-        if self._check_path_does_not_exist(lineno, test_list_path):
-            return
+            full_path = os.path.join(self._port.layout_tests_dir(),
+                                     test_list_path)
+            full_path = os.path.normpath(full_path)
+            # WebKit's way of skipping tests is to add a -disabled suffix.
+            # So we should consider the path existing if the path or the
+            # -disabled version exists.
+            if (not self._port.path_exists(full_path)
+                and not self._port.path_exists(full_path + '-disabled')):
+                # Log a non fatal error here since you hit this case any
+                # time you update test_expectations.txt without syncing
+                # the LayoutTests directory
+                self._log_non_fatal_error(lineno, 'Path does not exist.',
+                                       test_list_path)
+                continue
 
-        if not self._full_test_list:
-            tests = [test_list_path]
-        else:
-            tests = self._expand_tests(test_list_path)
+            if not self._full_test_list:
+                tests = [test_list_path]
+            else:
+                tests = self._expand_tests(test_list_path)
 
-        modifiers = [o for o in options if o in self.MODIFIERS]
-        self._add_tests(tests, expectations, test_list_path, lineno,
-                        modifiers, num_matches, options, overrides_allowed)
+            self._add_tests(tests, expectations, test_list_path, lineno,
+                           modifiers, options, overrides_allowed)
 
     def _get_options_list(self, listString):
         return [part.strip().lower() for part in listString.strip().split(' ')]
@@ -639,66 +727,6 @@ class TestExpectationsFile:
             result.add(expectation)
         return result
 
-    def _check_options(self, matcher, options, lineno, test_list_path):
-        match_result = self._check_syntax(matcher, options, lineno,
-                                          test_list_path)
-        self._check_semantics(options, lineno, test_list_path)
-        return match_result.num_matches
-
-    def _check_syntax(self, matcher, options, lineno, test_list_path):
-        match_result = matcher.match(options)
-        for error in match_result.errors:
-            self._add_error(lineno, error, test_list_path)
-        for warning in match_result.warnings:
-            self._log_non_fatal_error(lineno, warning, test_list_path)
-        return match_result
-
-    def _check_semantics(self, options, lineno, test_list_path):
-        has_wontfix = 'wontfix' in options
-        has_bug = False
-        for opt in options:
-            if opt.startswith('bug'):
-                has_bug = True
-                if re.match('bug\d+', opt):
-                    self._add_error(lineno,
-                        'BUG\d+ is not allowed, must be one of '
-                        'BUGCR\d+, BUGWK\d+, BUGV8_\d+, '
-                        'or a non-numeric bug identifier.', test_list_path)
-
-        if not has_bug and not has_wontfix:
-            self._log_non_fatal_error(lineno, 'Test lacks BUG modifier.',
-            test_list_path)
-
-        if self._is_lint_mode and 'rebaseline' in options:
-            self._add_error(lineno,
-            'REBASELINE should only be used for running rebaseline.py. '
-            'Cannot be checked in.', test_list_path)
-
-    def _check_options_against_expectations(self, options, expectations,
-                                            lineno, test_list_path):
-        if 'slow' in options and TIMEOUT in expectations:
-            self._add_error(lineno,
-                'A test can not be both slow and timeout. If it times out '
-                'indefinitely, then it should be just timeout.',
-                test_list_path)
-
-    def _check_path_does_not_exist(self, lineno, test_list_path):
-        full_path = os.path.join(self._port.layout_tests_dir(),
-                                    test_list_path)
-        full_path = os.path.normpath(full_path)
-        # WebKit's way of skipping tests is to add a -disabled suffix.
-            # So we should consider the path existing if the path or the
-        # -disabled version exists.
-        if (not self._port.path_exists(full_path)
-            and not self._port.path_exists(full_path + '-disabled')):
-            # Log a non fatal error here since you hit this case any
-            # time you update test_expectations.txt without syncing
-            # the LayoutTests directory
-            self._log_non_fatal_error(lineno, 'Path does not exist.',
-                                    test_list_path)
-            return True
-        return False
-
     def _expand_tests(self, test_list_path):
         """Convert the test specification to an absolute, normalized
         path and make sure directories end with the OS path separator."""
@@ -724,19 +752,17 @@ class TestExpectationsFile:
         return result
 
     def _add_tests(self, tests, expectations, test_list_path, lineno,
-                   modifiers, num_matches, options, overrides_allowed):
+                   modifiers, options, overrides_allowed):
         for test in tests:
-            if self._already_seen_better_match(test, test_list_path,
-                num_matches, lineno, overrides_allowed):
+            if self._already_seen_test(test, test_list_path, lineno,
+                                       overrides_allowed):
                 continue
 
             self._clear_expectations_for_test(test, test_list_path)
-            self._test_list_paths[test] = (os.path.normpath(test_list_path),
-                num_matches, lineno)
-            self._add_test(test, modifiers, num_matches, expectations, options,
+            self._add_test(test, modifiers, expectations, options,
                            overrides_allowed)
 
-    def _add_test(self, test, modifiers, num_matches, expectations, options,
+    def _add_test(self, test, modifiers, expectations, options,
                   overrides_allowed):
         """Sets the expected state for a given test.
 
@@ -747,7 +773,6 @@ class TestExpectationsFile:
         Args:
           test: test to add
           modifiers: sequence of modifier keywords ('wontfix', 'slow', etc.)
-          num_matches: number of modifiers that matched the configuration
           expectations: sequence of expectations (PASS, IMAGE, etc.)
           options: sequence of keywords and bug identifiers.
           overrides_allowed: whether we're parsing the regular expectations
@@ -786,13 +811,13 @@ class TestExpectationsFile:
         than a previous listing.
         """
         if test in self._test_list_paths:
-            base_path = self._test_list_paths[test][0]
-            self._test_to_expectations.pop(base_path, '')
-            self._remove_from_sets(base_path, self._expectation_to_tests)
-            self._remove_from_sets(base_path, self._modifier_to_tests)
-            self._remove_from_sets(base_path, self._timeline_to_tests)
-            self._remove_from_sets(base_path, self._result_type_to_tests)
+            self._test_to_expectations.pop(test, '')
+            self._remove_from_sets(test, self._expectation_to_tests)
+            self._remove_from_sets(test, self._modifier_to_tests)
+            self._remove_from_sets(test, self._timeline_to_tests)
+            self._remove_from_sets(test, self._result_type_to_tests)
 
+        self._test_list_paths[test] = os.path.normpath(test_list_path)
 
     def _remove_from_sets(self, test, dict):
         """Removes the given test from the sets in the dictionary.
@@ -804,70 +829,32 @@ class TestExpectationsFile:
             if test in set_of_tests:
                 set_of_tests.remove(test)
 
-    def _already_seen_better_match(self, test, test_list_path, num_matches,
-                                   lineno, overrides_allowed):
-        """Returns whether we've seen a better match already in the file.
-
-        Returns True if we've already seen a test_list_path that matches more of the test
-            than this path does
+    def _already_seen_test(self, test, test_list_path, lineno,
+                           allow_overrides):
+        """Returns true if we've already seen a more precise path for this test
+        than the test_list_path.
         """
-        # FIXME: See comment below about matching test configs and num_matches.
-
         if not test in self._test_list_paths:
-            # We've never seen this test before.
             return False
 
-        prev_base_path, prev_num_matches, prev_lineno = self._test_list_paths[test]
-        base_path = os.path.normpath(test_list_path)
-
-        if len(prev_base_path) > len(base_path):
-            # The previous path matched more of the test.
-            return True
-
-        if len(prev_base_path) < len(base_path):
-            # This path matches more of the test.
-            return False
-
-        if overrides_allowed and test not in self._overridding_tests:
-            # We have seen this path, but that's okay because it is
-            # in the overrides and the earlier path was in the
-            # expectations (not the overrides).
-            return False
-
-        # At this point we know we have seen a previous exact match on this
-        # base path, so we need to check the two sets of modifiers.
-
-        if overrides_allowed:
-            expectation_source = "override"
-        else:
-            expectation_source = "expectation"
-
-        # FIXME: This code was originally designed to allow lines that matched
-        # more modifiers to override lines that matched fewer modifiers.
-        # However, we currently view these as errors. If we decide to make
-        # this policy permanent, we can probably simplify this code
-        # and the ModifierMatcher code a fair amount.
-        #
-        # To use the "more modifiers wins" policy, change the "_add_error" lines for overrides
-        # to _log_non_fatal_error() and change the commented-out "return False".
-
-        if prev_num_matches == num_matches:
-            self._add_error(lineno,
-                'Duplicate or ambiguous %s.' % expectation_source,
-                test)
-            return True
+        prev_base_path = self._test_list_paths[test]
+        if (prev_base_path == os.path.normpath(test_list_path)):
+            if (not allow_overrides or test in self._overridding_tests):
+                if allow_overrides:
+                    expectation_source = "override"
+                else:
+                    expectation_source = "expectation"
+                self._add_error(lineno, 'Duplicate %s.' % expectation_source,
+                                   test)
+                return True
+            else:
+                # We have seen this path, but that's okay because its
+                # in the overrides and the earlier path was in the
+                # expectations.
+                return False
 
-        if prev_num_matches < num_matches:
-            self._add_error(lineno,
-                'More specific entry on line %d overrides line %d' %
-                (lineno, prev_lineno), test_list_path)
-            # FIXME: return False if we want more specific to win.
-            return True
-
-        self._add_error(lineno,
-            'More specific entry on line %d overrides line %d' %
-            (prev_lineno, lineno), test_list_path)
-        return True
+        # Check if we've already seen a more precise path.
+        return prev_base_path.startswith(os.path.normpath(test_list_path))
 
     def _add_error(self, lineno, msg, path):
         """Reports an error that will prevent running the tests. Does not
@@ -879,188 +866,3 @@ class TestExpectationsFile:
         """Reports an error that will not prevent running the tests. These are
         still errors, but not bad enough to warrant breaking test running."""
         self._non_fatal_errors.append('Line:%s %s %s' % (lineno, msg, path))
-
-
-class ModifierMatchResult(object):
-    def __init__(self, options):
-        self.num_matches = ModifierMatcher.NO_MATCH
-        self.options = options
-        self.errors = []
-        self.warnings = []
-        self.modifiers = []
-        self._matched_regexes = set()
-        self._matched_macros = set()
-
-
-class ModifierMatcher(object):
-
-    """
-    This class manages the interpretation of the "modifiers" for a given
-    line in the expectations file. Modifiers are the tokens that appear to the
-    left of the colon on a line. For example, "BUG1234", "DEBUG", and "WIN" are
-    all modifiers. This class gets what the valid modifiers are, and which
-    modifiers are allowed to exist together on a line, from the
-    TestConfiguration object that is passed in to the call.
-
-    This class detects *intra*-line errors like unknown modifiers, but
-    does not detect *inter*-line modifiers like duplicate expectations.
-
-    More importantly, this class is also used to determine if a given line
-    matches the port in question. Matches are ranked according to the number
-    of modifiers that match on a line. A line with no modifiers matches
-    everything and has a score of zero. A line with one modifier matches only
-    ports that have that modifier and gets a score of 1, and so one. Ports
-    that don't match at all get a score of -1.
-
-    Given two lines in a file that apply to the same test, if both expectations
-    match the current config, then the expectation is considered ambiguous,
-    even if one expectation matches more of the config than the other. For
-    example, in:
-
-    BUG1 RELEASE : foo.html = FAIL
-    BUG1 WIN RELEASE : foo.html = PASS
-    BUG2 WIN : bar.html = FAIL
-    BUG2 DEBUG : bar.html = PASS
-
-    lines 1 and 2 would produce an error on a Win XP Release bot (the scores
-    would be 1 and 2, respectively), and lines three and four would produce
-    a duplicate expectation on a Win Debug bot since both the 'win' and the
-    'debug' expectations would apply (both had scores of 1).
-
-    In addition to the definitions of all of the modifiers, the class
-    supports "macros" that are expanded prior to interpretation, and "ignore
-    regexes" that can be used to skip over modifiers like the BUG* modifiers.
-    """
-    MACROS = {
-        'mac-snowleopard': ['mac', 'snowleopard'],
-        'mac-leopard': ['mac', 'leopard'],
-        'win-xp': ['win', 'xp'],
-        'win-vista': ['win', 'vista'],
-        'win-7': ['win', 'win7'],
-    }
-
-    # We don't include the "none" modifier because it isn't actually legal.
-    REGEXES_TO_IGNORE = (['bug\w+'] +
-                         TestExpectationsFile.MODIFIERS.keys()[:-1])
-    DUPLICATE_REGEXES_ALLOWED = ['bug\w+']
-
-    # Magic value returned when the options don't match.
-    NO_MATCH = -1
-
-    # FIXME: The code currently doesn't detect combinations of modifiers
-    # that are syntactically valid but semantically invalid, like
-    # 'MAC XP'. See ModifierMatchTest.test_invalid_combinations() in the
-    # _unittest.py file.
-
-    def __init__(self, test_config):
-        """Initialize a ModifierMatcher argument with the TestConfiguration it
-        should be matched against."""
-        self.test_config = test_config
-        self.allowed_configurations = test_config.all_test_configurations()
-        self.macros = self.MACROS
-
-        self.regexes_to_ignore = {}
-        for regex_str in self.REGEXES_TO_IGNORE:
-            self.regexes_to_ignore[regex_str] = re.compile(regex_str)
-
-        # Keep a set of all of the legal modifiers for quick checking.
-        self._all_modifiers = set()
-
-        # Keep a dict mapping values back to their categories.
-        self._categories_for_modifiers = {}
-        for config in self.allowed_configurations:
-            for category, modifier in config.items():
-                self._categories_for_modifiers[modifier] = category
-                self._all_modifiers.add(modifier)
-
-    def match(self, options):
-        """Checks a list of options against the config set in the constructor.
-        Options may be either actual modifier strings, "macro" strings
-        that get expanded to a list of modifiers, or strings that are allowed
-        to be ignored. All of the options must be passed in in lower case.
-
-        Returns the number of matching categories, or NO_MATCH (-1) if it
-        doesn't match or there were errors found. Matches are prioritized
-        by the number of matching categories, because the more specific
-        the options list, the more categories will match.
-
-        The results of the most recent match are available in the 'options',
-        'modifiers', 'num_matches', 'errors', and 'warnings' properties.
-        """
-        result = ModifierMatchResult(options)
-        self._parse(result)
-        if result.errors:
-            return result
-        self._count_matches(result)
-        return result
-
-    def _parse(self, result):
-        # FIXME: Should we warn about lines having every value in a category?
-        for option in result.options:
-            self._parse_one(option, result)
-
-    def _parse_one(self, option, result):
-        if option in self._all_modifiers:
-            self._add_modifier(option, result)
-        elif option in self.macros:
-            self._expand_macro(option, result)
-        elif not self._matches_any_regex(option, result):
-            result.errors.append("Unrecognized option '%s'" % option)
-
-    def _add_modifier(self, option, result):
-        if option in result.modifiers:
-            result.errors.append("More than one '%s'" % option)
-        else:
-            result.modifiers.append(option)
-
-    def _expand_macro(self, macro, result):
-        if macro in result._matched_macros:
-            result.errors.append("More than one '%s'" % macro)
-            return
-
-        mods = []
-        for modifier in self.macros[macro]:
-            if modifier in result.options:
-                result.errors.append("Can't specify both modifier '%s' and "
-                                     "macro '%s'" % (modifier, macro))
-            else:
-                mods.append(modifier)
-        result._matched_macros.add(macro)
-        result.modifiers.extend(mods)
-
-    def _matches_any_regex(self, option, result):
-        for regex_str, pattern in self.regexes_to_ignore.iteritems():
-            if pattern.match(option):
-                self._handle_regex_match(regex_str, result)
-                return True
-        return False
-
-    def _handle_regex_match(self, regex_str, result):
-        if (regex_str in result._matched_regexes and
-            regex_str not in self.DUPLICATE_REGEXES_ALLOWED):
-            result.errors.append("More than one option matching '%s'" %
-                                 regex_str)
-        else:
-            result._matched_regexes.add(regex_str)
-
-    def _count_matches(self, result):
-        """Returns the number of modifiers that match the test config."""
-        categorized_modifiers = self._group_by_category(result.modifiers)
-        result.num_matches = 0
-        for category, modifier in self.test_config.items():
-            if category in categorized_modifiers:
-                if modifier in categorized_modifiers[category]:
-                    result.num_matches += 1
-                else:
-                    result.num_matches = self.NO_MATCH
-                    return
-
-    def _group_by_category(self, modifiers):
-        # Returns a dict of category name -> list of modifiers.
-        modifiers_by_category = {}
-        for m in modifiers:
-            modifiers_by_category.setdefault(self._category(m), []).append(m)
-        return modifiers_by_category
-
-    def _category(self, modifier):
-        return self._categories_for_modifiers[modifier]
index c74cf3148a9a750ade7b7d4ddca79d74a0ec208f..34771f3039aff10a0fd220efa4836755c7728ee3 100644 (file)
@@ -34,7 +34,6 @@ import sys
 import unittest
 
 from webkitpy.layout_tests import port
-from webkitpy.layout_tests.port import base
 from webkitpy.layout_tests.layout_package.test_expectations import *
 
 class FunctionsTest(unittest.TestCase):
@@ -81,9 +80,6 @@ class FunctionsTest(unittest.TestCase):
 
 
 class Base(unittest.TestCase):
-    # Note that all of these tests are written assuming the configuration
-    # being tested is Mac Leopard, Release build.
-
     def __init__(self, testFunc, setUp=None, tearDown=None, description=None):
         self._port = port.get('test', None)
         self._exp = None
@@ -109,12 +105,13 @@ BUG_TEST WONTFIX : failures/expected/image_checksum.html = IMAGE
 BUG_TEST WONTFIX WIN : failures/expected/image.html = IMAGE
 """
 
-    def parse_exp(self, expectations, overrides=None, is_lint_mode=False):
-        test_config = self._port.test_configuration()
+    def parse_exp(self, expectations, overrides=None, is_lint_mode=False,
+                  is_debug_mode=False):
         self._exp = TestExpectations(self._port,
              tests=self.get_basic_tests(),
              expectations=expectations,
-             test_config=test_config,
+             test_platform_name=self._port.test_platform_name(),
+             is_debug_mode=is_debug_mode,
              is_lint_mode=is_lint_mode,
              overrides=overrides)
 
@@ -123,7 +120,7 @@ BUG_TEST WONTFIX WIN : failures/expected/image.html = IMAGE
                           set([result]))
 
 
-class BasicTests(Base):
+class TestExpectationsTest(Base):
     def test_basic(self):
         self.parse_exp(self.get_basic_expectations())
         self.assert_exp('failures/expected/text.html', TEXT)
@@ -131,14 +128,23 @@ class BasicTests(Base):
         self.assert_exp('passes/text.html', PASS)
         self.assert_exp('failures/expected/image.html', PASS)
 
-
-class MiscTests(Base):
     def test_multiple_results(self):
         self.parse_exp('BUGX : failures/expected/text.html = TEXT CRASH')
         self.assertEqual(self._exp.get_expectations(
             self.get_test('failures/expected/text.html')),
             set([TEXT, CRASH]))
 
+    def test_precedence(self):
+        # This tests handling precedence of specific lines over directories
+        # and tests expectations covering entire directories.
+        exp_str = """
+BUGX : failures/expected/text.html = TEXT
+BUGX WONTFIX : failures/expected = IMAGE
+"""
+        self.parse_exp(exp_str)
+        self.assert_exp('failures/expected/text.html', TEXT)
+        self.assert_exp('failures/expected/crash.html', IMAGE)
+
     def test_category_expectations(self):
         # This test checks unknown tests are not present in the
         # expectations and that known test part of a test category is
@@ -153,6 +159,20 @@ BUGX WONTFIX : failures/expected = IMAGE
                           unknown_test)
         self.assert_exp('failures/expected/crash.html', IMAGE)
 
+    def test_release_mode(self):
+        self.parse_exp('BUGX DEBUG : failures/expected/text.html = TEXT',
+                       is_debug_mode=True)
+        self.assert_exp('failures/expected/text.html', TEXT)
+        self.parse_exp('BUGX RELEASE : failures/expected/text.html = TEXT',
+                       is_debug_mode=True)
+        self.assert_exp('failures/expected/text.html', PASS)
+        self.parse_exp('BUGX DEBUG : failures/expected/text.html = TEXT',
+                       is_debug_mode=False)
+        self.assert_exp('failures/expected/text.html', PASS)
+        self.parse_exp('BUGX RELEASE : failures/expected/text.html = TEXT',
+                       is_debug_mode=False)
+        self.assert_exp('failures/expected/text.html', TEXT)
+
     def test_get_options(self):
         self.parse_exp(self.get_basic_expectations())
         self.assertEqual(self._exp.get_options(
@@ -197,7 +217,7 @@ SKIP : failures/expected/image.html""")
             self.assertFalse(True, "ParseError wasn't raised")
         except ParseError, e:
             self.assertTrue(e.fatal)
-            exp_errors = [u"Line:1 Unrecognized option 'foo' failures/expected/text.html",
+            exp_errors = [u'Line:1 Invalid modifier for test: foo failures/expected/text.html',
                           u"Line:2 Missing expectations. [' failures/expected/image.html']"]
             self.assertEqual(str(e), '\n'.join(map(str, exp_errors)))
             self.assertEqual(e.errors, exp_errors)
@@ -213,160 +233,77 @@ SKIP : failures/expected/image.html""")
             self.assertEqual(str(e), '\n'.join(map(str, exp_errors)))
             self.assertEqual(e.errors, exp_errors)
 
-    def test_overrides(self):
-        self.parse_exp("BUG_EXP: failures/expected/text.html = TEXT",
-                       "BUG_OVERRIDE : failures/expected/text.html = IMAGE")
-        self.assert_exp('failures/expected/text.html', IMAGE)
-
-    def test_overrides__duplicate(self):
-        self.assertRaises(ParseError, self.parse_exp,
-             "BUG_EXP: failures/expected/text.html = TEXT",
-             """
-BUG_OVERRIDE : failures/expected/text.html = IMAGE
-BUG_OVERRIDE : failures/expected/text.html = CRASH
-""")
-
-    def test_pixel_tests_flag(self):
-        def match(test, result, pixel_tests_enabled):
-            return self._exp.matches_an_expected_result(
-                self.get_test(test), result, pixel_tests_enabled)
-
-        self.parse_exp(self.get_basic_expectations())
-        self.assertTrue(match('failures/expected/text.html', TEXT, True))
-        self.assertTrue(match('failures/expected/text.html', TEXT, False))
-        self.assertFalse(match('failures/expected/text.html', CRASH, True))
-        self.assertFalse(match('failures/expected/text.html', CRASH, False))
-        self.assertTrue(match('failures/expected/image_checksum.html', IMAGE,
-                              True))
-        self.assertTrue(match('failures/expected/image_checksum.html', PASS,
-                              False))
-        self.assertTrue(match('failures/expected/crash.html', SKIP, False))
-        self.assertTrue(match('passes/text.html', PASS, False))
-
-
-class ExpectationSyntaxTests(Base):
-    def test_missing_expectation(self):
+    def test_syntax_missing_expectation(self):
         # This is missing the expectation.
         self.assertRaises(ParseError, self.parse_exp,
-                          'BUG_TEST: failures/expected/text.html')
+                          'BUG_TEST: failures/expected/text.html',
+                          is_debug_mode=True)
 
-    def test_missing_colon(self):
-        # This is missing the modifiers and the ':'
+    def test_syntax_invalid_option(self):
         self.assertRaises(ParseError, self.parse_exp,
-                          'failures/expected/text.html = TEXT')
+                          'BUG_TEST FOO: failures/expected/text.html = PASS')
 
-    def disabled_test_too_many_colons(self):
-        # FIXME: Enable this test and fix the underlying bug.
-        self.assertRaises(ParseError, self.parse_exp,
-                          'BUG_TEST: failures/expected/text.html = PASS :')
-
-    def test_too_many_equals_signs(self):
-        self.assertRaises(ParseError, self.parse_exp,
-                          'BUG_TEST: failures/expected/text.html = TEXT = IMAGE')
-
-    def test_unrecognized_expectation(self):
+    def test_syntax_invalid_expectation(self):
+        # This is missing the expectation.
         self.assertRaises(ParseError, self.parse_exp,
-                          'BUG_TEST: failures/expected/text.html = UNKNOWN')
-
-    def test_macro(self):
-        exp_str = """
-BUG_TEST MAC-LEOPARD : failures/expected/text.html = TEXT
-"""
-        self.parse_exp(exp_str)
-        self.assert_exp('failures/expected/text.html', TEXT)
-
-
-class SemanticTests(Base):
-    def test_bug_format(self):
-        self.assertRaises(ParseError, self.parse_exp, 'BUG1234 : failures/expected/text.html = TEXT')
+                          'BUG_TEST: failures/expected/text.html = FOO')
 
-    def test_missing_bugid(self):
+    def test_syntax_missing_bugid(self):
         # This should log a non-fatal error.
         self.parse_exp('SLOW : failures/expected/text.html = TEXT')
         self.assertEqual(
             len(self._exp._expected_failures.get_non_fatal_errors()), 1)
 
-    def test_slow_and_timeout(self):
+    def test_semantic_slow_and_timeout(self):
         # A test cannot be SLOW and expected to TIMEOUT.
         self.assertRaises(ParseError, self.parse_exp,
             'BUG_TEST SLOW : failures/expected/timeout.html = TIMEOUT')
 
-    def test_rebaseline(self):
+    def test_semantic_rebaseline(self):
         # Can't lint a file w/ 'REBASELINE' in it.
         self.assertRaises(ParseError, self.parse_exp,
             'BUG_TEST REBASELINE : failures/expected/text.html = TEXT',
             is_lint_mode=True)
 
-    def test_duplicates(self):
+    def test_semantic_duplicates(self):
         self.assertRaises(ParseError, self.parse_exp, """
-BUG_EXP : failures/expected/text.html = TEXT
-BUG_EXP : failures/expected/text.html = IMAGE""")
+BUG_TEST : failures/expected/text.html = TEXT
+BUG_TEST : failures/expected/text.html = IMAGE""")
 
         self.assertRaises(ParseError, self.parse_exp,
-            self.get_basic_expectations(), overrides="""
-BUG_OVERRIDE : failures/expected/text.html = TEXT
-BUG_OVERRIDE : failures/expected/text.html = IMAGE""", )
+            self.get_basic_expectations(), """
+BUG_TEST : failures/expected/text.html = TEXT
+BUG_TEST : failures/expected/text.html = IMAGE""")
 
-    def test_missing_file(self):
+    def test_semantic_missing_file(self):
         # This should log a non-fatal error.
         self.parse_exp('BUG_TEST : missing_file.html = TEXT')
         self.assertEqual(
             len(self._exp._expected_failures.get_non_fatal_errors()), 1)
 
 
-class PrecedenceTests(Base):
-    def test_file_over_directory(self):
-        # This tests handling precedence of specific lines over directories
-        # and tests expectations covering entire directories.
-        exp_str = """
-BUGX : failures/expected/text.html = TEXT
-BUGX WONTFIX : failures/expected = IMAGE
-"""
-        self.parse_exp(exp_str)
-        self.assert_exp('failures/expected/text.html', TEXT)
-        self.assert_exp('failures/expected/crash.html', IMAGE)
-
-        exp_str = """
-BUGX WONTFIX : failures/expected = IMAGE
-BUGX : failures/expected/text.html = TEXT
-"""
-        self.parse_exp(exp_str)
-        self.assert_exp('failures/expected/text.html', TEXT)
-        self.assert_exp('failures/expected/crash.html', IMAGE)
-
-    def test_ambiguous(self):
-        self.assertRaises(ParseError, self.parse_exp, """
-BUG_TEST RELEASE : passes/text.html = PASS
-BUG_TEST MAC : passes/text.html = FAIL
-""")
-
-    def test_more_modifiers(self):
-        exp_str = """
-BUG_TEST RELEASE : passes/text.html = PASS
-BUG_TEST MAC RELEASE : passes/text.html = TEXT
-"""
-        self.assertRaises(ParseError, self.parse_exp, exp_str)
+    def test_overrides(self):
+        self.parse_exp(self.get_basic_expectations(), """
+BUG_OVERRIDE : failures/expected/text.html = IMAGE""")
+        self.assert_exp('failures/expected/text.html', IMAGE)
 
-    def test_order_in_file(self):
-        exp_str = """
-BUG_TEST MAC RELEASE : passes/text.html = TEXT
-BUG_TEST RELEASE : passes/text.html = PASS
-"""
-        self.assertRaises(ParseError, self.parse_exp, exp_str)
+    def test_matches_an_expected_result(self):
 
-    def test_version_overrides(self):
-        exp_str = """
-BUG_TEST MAC : passes/text.html = PASS
-BUG_TEST MAC LEOPARD : passes/text.html = TEXT
-"""
-        self.assertRaises(ParseError, self.parse_exp, exp_str)
+        def match(test, result, pixel_tests_enabled):
+            return self._exp.matches_an_expected_result(
+                self.get_test(test), result, pixel_tests_enabled)
 
-    def test_macro_overrides(self):
-        exp_str = """
-BUG_TEST MAC : passes/text.html = PASS
-BUG_TEST MAC-LEOPARD : passes/text.html = TEXT
-"""
-        self.assertRaises(ParseError, self.parse_exp, exp_str)
+        self.parse_exp(self.get_basic_expectations())
+        self.assertTrue(match('failures/expected/text.html', TEXT, True))
+        self.assertTrue(match('failures/expected/text.html', TEXT, False))
+        self.assertFalse(match('failures/expected/text.html', CRASH, True))
+        self.assertFalse(match('failures/expected/text.html', CRASH, False))
+        self.assertTrue(match('failures/expected/image_checksum.html', IMAGE,
+                              True))
+        self.assertTrue(match('failures/expected/image_checksum.html', PASS,
+                              False))
+        self.assertTrue(match('failures/expected/crash.html', SKIP, False))
+        self.assertTrue(match('passes/text.html', PASS, False))
 
 
 class RebaseliningTest(Base):
@@ -409,86 +346,5 @@ BUG_TEST REBASELINE : failures/expected/text.html = TEXT
             '\n\n')
 
 
-class ModifierTests(unittest.TestCase):
-    def setUp(self):
-        port_obj = port.get('test', None)
-        self.config = port_obj.test_configuration()
-        self.matcher = ModifierMatcher(self.config)
-
-    def match(self, modifiers, expected_num_matches=-1, values=None, num_errors=0):
-        matcher = self.matcher
-        if values:
-            matcher = ModifierMatcher(self.FakeTestConfiguration(values))
-        match_result = matcher.match(modifiers)
-        self.assertEqual(len(match_result.warnings), 0)
-        self.assertEqual(len(match_result.errors), num_errors)
-        self.assertEqual(match_result.num_matches, expected_num_matches,
-             'match(%s, %s) returned -> %d, expected %d' %
-             (modifiers, str(self.config.values()),
-              match_result.num_matches, expected_num_matches))
-
-    def test_bad_match_modifier(self):
-        self.match(['foo'], num_errors=1)
-
-    def test_none(self):
-        self.match([], 0)
-
-    def test_one(self):
-        self.match(['leopard'], 1)
-        self.match(['mac'], 1)
-        self.match(['release'], 1)
-        self.match(['cpu'], 1)
-        self.match(['x86'], 1)
-        self.match(['xp'], -1)
-        self.match(['gpu'], -1)
-        self.match(['debug'], -1)
-
-    def test_two(self):
-        self.match(['leopard', 'release'], 2)
-        self.match(['xp', 'release'], -1)
-        self.match(['xp', 'release'], -1)
-        self.match(['snowleopard', 'leopard'], 1)
-
-    def test_three(self):
-        self.match(['snowleopard', 'leopard', 'release'], 2)
-        self.match(['leopard', 'debug', 'x86'], -1)
-        self.match(['leopard', 'release', 'x86'], 3)
-        self.match(['leopard', 'cpu', 'release'], 3)
-
-    def test_four(self):
-        self.match(['leopard', 'release', 'cpu', 'x86'], 4)
-        self.match(['snowleopard', 'leopard', 'release', 'cpu'], 3)
-        self.match(['snowleopard', 'leopard', 'debug', 'cpu'], -1)
-
-    def test_case_insensitivity(self):
-        self.match(['Mac'], num_errors=1)
-        self.match(['MAC'], num_errors=1)
-        self.match(['mac'], 1)
-
-    def test_duplicates(self):
-        self.match(['release', 'release'], num_errors=1)
-        self.match(['mac-leopard', 'leopard'], num_errors=1)
-        self.match(['mac-leopard', 'mac-leopard'], num_errors=1)
-        self.match(['xp', 'release', 'xp', 'release'], num_errors=2)
-        self.match(['rebaseline', 'rebaseline'], num_errors=1)
-
-    def test_unknown_option(self):
-        self.match(['vms'], num_errors=1)
-
-    def test_duplicate_bugs(self):
-        # BUG* regexes can appear multiple times.
-        self.match(['bugfoo', 'bugbar'], 0)
-
-    def test_invalid_combinations(self):
-        # FIXME: This should probably raise an error instead of NO_MATCH.
-        self.match(['mac', 'xp'], num_errors=0)
-
-    def test_regexes_are_ignored(self):
-        self.match(['bug123xy', 'rebaseline', 'wontfix', 'slow', 'skip'], 0)
-
-    def test_none_is_invalid(self):
-        self.match(['none'], num_errors=1)
-
-
 if __name__ == '__main__':
     unittest.main()
index 2f48777c4b3e6ea703d9550e8359498874624adf..5b02a006d98c50424fb577499b9103868899959a 100644 (file)
@@ -212,13 +212,21 @@ class TestRunner:
 
     def lint(self):
         lint_failed = False
-        for test_configuration in self._port.all_test_configurations():
+
+        # Creating the expecations for each platform/configuration pair does
+        # all the test list parsing and ensures it's correct syntax (e.g. no
+        # dupes).
+        for platform_name in self._port.test_platform_names():
+            try:
+                self.parse_expectations(platform_name, is_debug_mode=True)
+            except test_expectations.ParseError:
+                lint_failed = True
             try:
-                self.lint_expectations(test_configuration)
+                self.parse_expectations(platform_name, is_debug_mode=False)
             except test_expectations.ParseError:
                 lint_failed = True
-                self._printer.write("")
 
+        self._printer.write("")
         if lint_failed:
             _log.error("Lint failed.")
             return -1
@@ -226,28 +234,22 @@ class TestRunner:
         _log.info("Lint succeeded.")
         return 0
 
-    def lint_expectations(self, config):
-        port = self._port
-        test_expectations.TestExpectations(
-            port,
-            None,
-            port.test_expectations(),
-            config,
-            self._options.lint_test_files,
-            port.test_expectations_overrides())
-
-    def parse_expectations(self):
+    def parse_expectations(self, test_platform_name, is_debug_mode):
         """Parse the expectations from the test_list files and return a data
         structure holding them. Throws an error if the test_list files have
         invalid syntax."""
-        port = self._port
+        if self._options.lint_test_files:
+            test_files = None
+        else:
+            test_files = self._test_files
+
+        expectations_str = self._port.test_expectations()
+        overrides_str = self._port.test_expectations_overrides()
         self._expectations = test_expectations.TestExpectations(
-            port,
-            self._test_files,
-            port.test_expectations(),
-            port.test_configuration(),
-            self._options.lint_test_files,
-            port.test_expectations_overrides())
+            self._port, test_files, expectations_str, test_platform_name,
+            is_debug_mode, self._options.lint_test_files,
+            overrides=overrides_str)
+        return self._expectations
 
     # FIXME: This method is way too long and needs to be broken into pieces.
     def prepare_lists_and_print_output(self):
@@ -355,7 +357,9 @@ class TestRunner:
             self._test_files_list = files + skip_chunk_list
             self._test_files = set(self._test_files_list)
 
-            self.parse_expectations()
+            self._expectations = self.parse_expectations(
+                self._port.test_platform_name(),
+                self._options.configuration == 'Debug')
 
             self._test_files = set(files)
             self._test_files_list = files
index 062257d58d7fac0de8345f28d22f70206ade1ece..97b54c97d81fff939991e6aeb2a3331608888152 100644 (file)
@@ -121,7 +121,6 @@ class Port(object):
         self.set_option_default('configuration', None)
         if self._options.configuration is None:
             self._options.configuration = self.default_configuration()
-        self._test_configuration = None
 
     def default_child_processes(self):
         """Return the number of DumpRenderTree instances to use for this
@@ -575,19 +574,6 @@ class Port(object):
         if self._http_lock:
             self._http_lock.cleanup_http_lock()
 
-    #
-    # TEST EXPECTATION-RELATED METHODS
-    #
-
-    def test_configuration(self):
-        """Returns the current TestConfiguration for the port."""
-        if not self._test_configuration:
-            self._test_configuration = TestConfiguration(self)
-        return self._test_configuration
-
-    def all_test_configurations(self):
-        return self.test_configuration().all_test_configurations()
-
     def test_expectations(self):
         """Returns the test expectations for this port.
 
@@ -874,68 +860,3 @@ class Driver:
 
     def stop(self):
         raise NotImplementedError('Driver.stop')
-
-
-class TestConfiguration(object):
-    def __init__(self, port=None, os=None, version=None, architecture=None,
-                 build_type=None, graphics_type=None):
-
-        # FIXME: We can get the O/S and version from test_platform_name()
-        # and version() for now, but those should go away and be cleaned up
-        # with more generic methods like operation_system() and os_version()
-        # or something. Note that we need to strip the leading '-' off the
-        # version string if it is present.
-        if port:
-            port_version = port.version()
-        self.os = os or port.test_platform_name().replace(port_version, '')
-        self.version = version or port_version[1:]
-        self.architecture = architecture or 'x86'
-        self.build_type = build_type or port._options.configuration.lower()
-        self.graphics_type = graphics_type or 'cpu'
-
-    def items(self):
-        return self.__dict__.items()
-
-    def keys(self):
-        return self.__dict__.keys()
-
-    def __str__(self):
-        return ("<%(version)s, %(build_type)s, %(graphics_type)s>" %
-                self.__dict__)
-
-    def __repr__(self):
-        return "TestConfig(os='%(os)s', version='%(version)s', architecture='%(architecture)s', build_type='%(build_type)s', graphics_type='%(graphics_type)s')" % self.__dict__
-
-    def values(self):
-        """Returns the configuration values of this instance as a tuple."""
-        return self.__dict__.values()
-
-    def all_test_configurations(self):
-        """Returns a sequence of the TestConfigurations the port supports."""
-        # By default, we assume we want to test every graphics type in
-        # every configuration on every system.
-        test_configurations = []
-        for system in self.all_systems():
-            for build_type in self.all_build_types():
-                for graphics_type in self.all_graphics_types():
-                    test_configurations.append(TestConfiguration(
-                        os=system[0],
-                        version=system[1],
-                        architecture=system[2],
-                        build_type=build_type,
-                        graphics_type=graphics_type))
-        return test_configurations
-
-    def all_systems(self):
-        return (('mac', 'leopard', 'x86'),
-                ('mac', 'snowleopard', 'x86'),
-                ('win', 'xp', 'x86'),
-                ('win', 'vista', 'x86'),
-                ('win', 'win7', 'x86'),
-                ('linux', 'hardy', 'x86'))
-
-    def all_build_types(self):
-        return ('debug', 'release')
-
-    def all_graphics_types(self):
-        return ('cpu', 'gpu')
index 5f2c15f8c0d0fdb6ec6ab235826b7e460aa57c65..b90421adfeb08b09430119ff2efbde28485c328f 100644 (file)
@@ -274,8 +274,8 @@ class ChromiumPort(base.Port):
             all_test_files.update(extra_test_files)
 
         expectations = test_expectations.TestExpectations(
-            self, all_test_files, expectations_str, self.test_configuration(),
-            is_lint_mode=False, overrides=overrides_str)
+            self, all_test_files, expectations_str, test_platform_name,
+            is_debug_mode, is_lint_mode=False, overrides=overrides_str)
         tests_dir = self.layout_tests_dir()
         return [self.relative_test_filename(test)
                 for test in expectations.get_tests_with_result_type(test_expectations.SKIP)]
index 04eca63028368b88cd765b2bba5c3cbd002645ec..c4b36acd9079fd2fbe90901202d5f9c025e387e5 100644 (file)
@@ -95,15 +95,3 @@ class PortTestCase(unittest.TestCase):
             return
         port.start_websocket_server()
         port.stop_websocket_server()
-
-    def test_test_configuration(self):
-        port = self.make_port()
-        if not port:
-            return
-        self.assertTrue(port.test_configuration())
-
-    def test_all_test_configurations(self):
-        port = self.make_port()
-        if not port:
-            return
-        self.assertTrue(len(port.all_test_configurations()) > 0)
index 67e687c09330476f810c3d785b065308eac2856f..935881c276a27645562e43e511b2f4f6e1cce911 100644 (file)
@@ -151,9 +151,6 @@ class TestPort(base.Port):
     def check_build(self, needs_http):
         return True
 
-    def default_configuration(self):
-        return 'Release'
-
     def diff_image(self, expected_contents, actual_contents,
                    diff_filename=None):
         diffed = actual_contents != expected_contents
@@ -309,7 +306,7 @@ WONTFIX SKIP : failures/expected/exception.html = CRASH
         return test_platform_name
 
     def version(self):
-        return '-leopard'
+        return ''
 
 
 class TestDriver(base.Driver):
index b5a4ef5edfb6a0eeec91eacca6926874d11977f8..4d8b7c9f5a2bd4c74ed56fd6312ba195c16c8a6d 100644 (file)
@@ -183,7 +183,8 @@ class Rebaseliner(object):
             test_expectations.TestExpectations(self._rebaseline_port,
                                                None,
                                                expectations_str,
-                                               self._target_port.test_configuration(),
+                                               self._platform,
+                                               False,
                                                False)
         self._scm = scm.default_scm()
 
index 4a93db178d5d6197757867092e98add169de8e6a..c431765ae2cfd9c70710d83c9d599a978b52e316 100755 (executable)
@@ -101,11 +101,11 @@ def run(port, options, args, regular_output=sys.stderr,
                 return -1
             raise
 
+        printer.print_update("Parsing expectations ...")
         if options.lint_test_files:
             return runner.lint()
-
-        printer.print_update("Parsing expectations ...")
-        runner.parse_expectations()
+        runner.parse_expectations(port.test_platform_name(),
+                                  options.configuration == 'Debug')
 
         printer.print_update("Checking build ...")
         if not port.check_build(runner.needs_http()):
index e37f908b64e7d5cd2203e659c7b58840cb5cf6eb..c86b32ce5657245df7d3f28093693359e5986ba5 100644 (file)
@@ -75,6 +75,7 @@ class TestExpectationsChecker(object):
                       "Using 'test' port, but platform-specific expectations "
                       "will fail the check." % self._file_path)
             self._port_obj = port.get('test')
+        self._port_to_check = self._port_obj.test_platform_name()
         # Suppress error messages of test_expectations module since they will be
         # reported later.
         log = logging.getLogger("webkitpy.layout_tests.layout_package."
@@ -90,7 +91,7 @@ class TestExpectationsChecker(object):
         try:
             expectations = test_expectations.TestExpectationsFile(
                 port=self._port_obj, expectations=expectations_str, full_test_list=tests,
-                test_config=self._port_obj.test_configuration(),
+                test_platform_name=self._port_to_check, is_debug_mode=False,
                 is_lint_mode=True, overrides=overrides)
         except test_expectations.ParseError, error:
             err = error
index 8691557e717c6f95994822461dfe441da47192b6..9817c5d702d3e661e06cb6af76c78a75e87c072d 100644 (file)
@@ -83,6 +83,15 @@ class TestExpectationsTestCase(unittest.TestCase):
         self.assertEqual(expected, self._error_collector.get_errors())
 
     def test_valid_expectations(self):
+        self.assert_lines_lint(
+            ["passes/text.html = PASS"],
+            "")
+        self.assert_lines_lint(
+            ["passes/text.html = FAIL PASS"],
+            "")
+        self.assert_lines_lint(
+            ["passes/text.html = CRASH TIMEOUT FAIL PASS"],
+            "")
         self.assert_lines_lint(
             ["BUGCR1234 MAC : passes/text.html = PASS FAIL"],
             "")
@@ -111,12 +120,12 @@ class TestExpectationsTestCase(unittest.TestCase):
     def test_modifier_errors(self):
         self.assert_lines_lint(
             ["BUG1234 : passes/text.html = FAIL"],
-            "BUG\\d+ is not allowed, must be one of BUGCR\\d+, BUGWK\\d+, BUGV8_\\d+, or a non-numeric bug identifier. passes/text.html  [test/expectations] [5]")
+            'Bug must be either BUGCR, BUGWK, or BUGV8_ for test: bug1234 passes/text.html  [test/expectations] [5]')
 
     def test_valid_modifiers(self):
         self.assert_lines_lint(
             ["INVALID-MODIFIER : passes/text.html = PASS"],
-            "Unrecognized option 'invalid-modifier' "
+            "Invalid modifier for test: invalid-modifier "
             "passes/text.html  [test/expectations] [5]")
         self.assert_lines_lint(
             ["SKIP : passes/text.html = PASS"],
@@ -126,38 +135,38 @@ class TestExpectationsTestCase(unittest.TestCase):
     def test_expectation_errors(self):
         self.assert_lines_lint(
             ["missing expectations"],
-            "Missing a ':' missing expectations  [test/expectations] [5]")
+            "Missing expectations. ['missing expectations']  [test/expectations] [5]")
         self.assert_lines_lint(
             ["SLOW : passes/text.html = TIMEOUT"],
             "A test can not be both slow and timeout. "
             "If it times out indefinitely, then it should be just timeout. "
             "passes/text.html  [test/expectations] [5]")
         self.assert_lines_lint(
-            ["BUGWK1 : does/not/exist.html = FAIL"],
+            ["does/not/exist.html = FAIL"],
             "Path does not exist. does/not/exist.html  [test/expectations] [2]")
 
     def test_parse_expectations(self):
         self.assert_lines_lint(
-            ["BUGWK1 : passes/text.html = PASS"],
+            ["passes/text.html = PASS"],
             "")
         self.assert_lines_lint(
-            ["BUGWK1 : passes/text.html = UNSUPPORTED"],
+            ["passes/text.html = UNSUPPORTED"],
             "Unsupported expectation: unsupported "
             "passes/text.html  [test/expectations] [5]")
         self.assert_lines_lint(
-            ["BUGWK1 : passes/text.html = PASS UNSUPPORTED"],
+            ["passes/text.html = PASS UNSUPPORTED"],
             "Unsupported expectation: unsupported "
             "passes/text.html  [test/expectations] [5]")
 
     def test_already_seen_test(self):
         self.assert_lines_lint(
-            ["BUGWK1 : passes/text.html = PASS",
-             "BUGWK1 : passes/text.html = TIMEOUT"],
-            "Duplicate or ambiguous expectation. %s  [test/expectations] [5]" % self._test_file)
+            ["passes/text.html = PASS",
+             "passes/text.html = TIMEOUT"],
+            "Duplicate expectation. %s  [test/expectations] [5]" % self._test_file)
 
     def test_tab(self):
         self.assert_lines_lint(
-            ["\tBUGWK1 : passes/text.html = PASS"],
+            ["\tpasses/text.html = PASS"],
             "Line contains tab character.  [whitespace/tab] [5]")
 
 if __name__ == '__main__':