[SCM] WebKit Debian packaging branch, webkit-1.3, updated. upstream/1.3.7-4207-g178b198
dpranke at chromium.org
dpranke at chromium.org
Sun Feb 20 22:50:22 UTC 2011
The following commit has been merged in the webkit-1.3 branch:
commit ac5b4d972cf1ab93d7a5d6d87c1dd85038fa5218
Author: dpranke at chromium.org <dpranke at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed Jan 12 01:15:46 2011 +0000
2011-01-11 Dirk Pranke <dpranke at chromium.org>
Reviewed by Ojan Vafai.
The current modifier parsing code in test_expectations is
fragile and hard-coded, so it's not easy to understand the logic
or easily add new types of modifiers (like GPU vs. CPU testing
for graphics tests, or 32-bit vs. 64-bit differences).
This is the first of two patches that will add in more generic
support and then eliminate the GPU-specific test expectations
files for Chromium.
This patch adds two standalone objects for handling modifiers. The
rules for interpreting modifiers, precedence, and conflicts are
given in the docstring to the ModifierMatcher class, which
returns ModifierMatchResult objects.
This patch also adds routines to the Port interface and a
default set of values in the base object, in order to obtain the
values needed on a given test run. These values are then passed
to the expectation parser. This also allows us to clean up the
logic used to lint all of the different configurations in a
single test_expectations.txt file.
The next patch will merge in the separate GPU expectations file.
https://bugs.webkit.org/show_bug.cgi?id=51222
* 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/port/base.py:
* Scripts/webkitpy/layout_tests/port/base_unittest.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/run_webkit_tests.py:
* Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@75576 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index e200f2c..bdd502c 100644
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,43 @@
+2011-01-11 Dirk Pranke <dpranke at chromium.org>
+
+ Reviewed by Ojan Vafai.
+
+ The current modifier parsing code in test_expectations is
+ fragile and hard-coded, so it's not easy to understand the logic
+ or easily add new types of modifiers (like GPU vs. CPU testing
+ for graphics tests, or 32-bit vs. 64-bit differences).
+
+ This is the first of two patches that will add in more generic
+ support and then eliminate the GPU-specific test expectations
+ files for Chromium.
+
+ This patch adds two standalone objects for handling modifiers. The
+ rules for interpreting modifiers, precedence, and conflicts are
+ given in the docstring to the ModifierMatcher class, which
+ returns ModifierMatchResult objects.
+
+ This patch also adds routines to the Port interface and a
+ default set of values in the base object, in order to obtain the
+ values needed on a given test run. These values are then passed
+ to the expectation parser. This also allows us to clean up the
+ logic used to lint all of the different configurations in a
+ single test_expectations.txt file.
+
+ The next patch will merge in the separate GPU expectations file.
+
+ https://bugs.webkit.org/show_bug.cgi?id=51222
+
+ * 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/port/base.py:
+ * Scripts/webkitpy/layout_tests/port/base_unittest.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/run_webkit_tests.py:
+ * Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:
+
2011-01-11 Maciej Stachowiak <mjs at apple.com>
Reviewed by Adam Barth.
diff --git a/Tools/Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py b/Tools/Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py
index 9280b02..d027e60 100644
--- a/Tools/Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py
+++ b/Tools/Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py
@@ -31,7 +31,6 @@
import os
import optparse
-import pdb
import sys
import unittest
import logging
@@ -146,7 +145,7 @@ class Testprinter(unittest.TestCase):
test in tests]
expectations = test_expectations.TestExpectations(
self._port, test_paths, expectations_str,
- self._port.test_platform_name(), is_debug_mode=False,
+ self._port.test_configuration(),
is_lint_mode=False)
rs = result_summary.ResultSummary(expectations, test_paths)
@@ -365,7 +364,7 @@ class Testprinter(unittest.TestCase):
def test_print_progress__detailed(self):
tests = ['passes/text.html', 'failures/expected/timeout.html',
'failures/expected/crash.html']
- expectations = 'failures/expected/timeout.html = TIMEOUT'
+ expectations = 'BUGX : failures/expected/timeout.html = TIMEOUT'
# first, test that it is disabled properly
# should still print one-line-progress
@@ -571,8 +570,8 @@ class Testprinter(unittest.TestCase):
self.assertFalse(out.empty())
expectations = """
-failures/expected/crash.html = CRASH
-failures/expected/timeout.html = TIMEOUT
+BUGX : failures/expected/crash.html = CRASH
+BUGX : failures/expected/timeout.html = TIMEOUT
"""
err.reset()
out.reset()
diff --git a/Tools/Scripts/webkitpy/layout_tests/layout_package/test_expectations.py b/Tools/Scripts/webkitpy/layout_tests/layout_package/test_expectations.py
index 8645fc1..98752c9 100644
--- a/Tools/Scripts/webkitpy/layout_tests/layout_package/test_expectations.py
+++ b/Tools/Scripts/webkitpy/layout_tests/layout_package/test_expectations.py
@@ -31,6 +31,7 @@
for layout tests.
"""
+import itertools
import logging
import os
import re
@@ -86,18 +87,16 @@ def remove_pixel_failures(expected_results):
class TestExpectations:
TEST_LIST = "test_expectations.txt"
- def __init__(self, port, tests, expectations, test_platform_name,
- is_debug_mode, is_lint_mode, overrides=None):
+ def __init__(self, port, tests, expectations, test_config,
+ 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_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
+ 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).
is_lint_mode: If True, just parse the expectations string
looking for errors.
overrides: test expectations that are allowed to override any
@@ -106,7 +105,7 @@ class TestExpectations:
and downstream expectations).
"""
self._expected_failures = TestExpectationsFile(port, expectations,
- tests, test_platform_name, is_debug_mode, is_lint_mode,
+ tests, test_config, is_lint_mode,
overrides=overrides)
# TODO(ojan): Allow for removing skipped tests when getting the list of
@@ -199,7 +198,7 @@ class ParseError(Exception):
return '\n'.join(map(str, self.errors))
def __repr__(self):
- return 'ParseError(fatal=%s, errors=%s)' % (fatal, errors)
+ return 'ParseError(fatal=%s, errors=%s)' % (self.fatal, self.errors)
class ModifiersAndExpectations:
@@ -304,28 +303,14 @@ class TestExpectationsFile:
'fail': FAIL,
'flaky': FLAKY}
- 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).
- """
+ def __init__(self, port, expectations, full_test_list,
+ test_config, is_lint_mode, overrides=None):
+ # See argument documentation in TestExpectation(), above.
self._port = port
self._expectations = expectations
self._full_test_list = full_test_list
- self._test_platform_name = test_platform_name
- self._is_debug_mode = is_debug_mode
+ self._test_config = test_config
self._is_lint_mode = is_lint_mode
self._overrides = overrides
self._errors = []
@@ -333,7 +318,9 @@ 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.
+ # the test is listed in the expectations file. We use this to
+ # keep a representation of the entire list of expectations, even
+ # invalid ones.
self._all_expectations = {}
# Maps a test to its list of expectations.
@@ -346,7 +333,8 @@ 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.
+ # Maps a test to the base path that it was listed with in the list and
+ # the number of matches that base path had.
self._test_list_paths = {}
self._modifier_to_tests = self._dict_of_sets(self.MODIFIERS)
@@ -373,13 +361,7 @@ class TestExpectationsFile:
def _handle_any_read_errors(self):
if len(self._errors) or len(self._non_fatal_errors):
- 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))
+ _log.error("FAILURES FOR %s" % str(self._test_config))
for error in self._errors:
_log.error(error)
@@ -395,11 +377,12 @@ 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, expectations, options,
- overrides_allowed=False)
+ self._add_test(test, modifiers, num_matches, expectations,
+ options, overrides_allowed=False)
def _dict_of_sets(self, strings_to_constants):
"""Takes a dict of strings->constants and returns a dict mapping
@@ -538,12 +521,15 @@ class TestExpectationsFile:
options = []
if line.find(":") is -1:
- test_and_expectation = line.split("=")
- else:
- parts = line.split(":")
- options = self._get_options_list(parts[0])
- test_and_expectation = parts[1].split('=')
+ 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.
+
+ 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.",
@@ -589,69 +575,6 @@ 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('\\', '/')
@@ -664,54 +587,43 @@ 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)
- test_list_path, options, expectations = \
- self.parse_expectations_line(line, lineno)
- if not expectations:
- continue
+ def _process_line(self, line, lineno, matcher, overrides_allowed):
+ test_list_path, options, expectations = \
+ self.parse_expectations_line(line, lineno)
+ if not expectations:
+ return
- 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())
- modifiers = set()
- if options and not self._has_valid_modifiers_for_current_platform(
- options, lineno, test_list_path, modifiers):
- continue
+ num_matches = self._check_options(matcher, options, lineno,
+ test_list_path)
+ if num_matches == ModifierMatcher.NO_MATCH:
+ return
- expectations = self._parse_expectations(expectations, lineno,
- test_list_path)
+ expectations = self._parse_expectations(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)
+ self._check_options_against_expectations(options, expectations,
+ 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)
- continue
+ if self._check_path_does_not_exist(lineno, test_list_path):
+ return
- 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)
- self._add_tests(tests, expectations, test_list_path, lineno,
- modifiers, options, overrides_allowed)
+ 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)
def _get_options_list(self, listString):
return [part.strip().lower() for part in listString.strip().split(' ')]
@@ -727,6 +639,66 @@ 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."""
@@ -752,17 +724,19 @@ class TestExpectationsFile:
return result
def _add_tests(self, tests, expectations, test_list_path, lineno,
- modifiers, options, overrides_allowed):
+ modifiers, num_matches, options, overrides_allowed):
for test in tests:
- if self._already_seen_test(test, test_list_path, lineno,
- overrides_allowed):
+ if self._already_seen_better_match(test, test_list_path,
+ num_matches, lineno, overrides_allowed):
continue
self._clear_expectations_for_test(test, test_list_path)
- self._add_test(test, modifiers, expectations, options,
+ self._test_list_paths[test] = (os.path.normpath(test_list_path),
+ num_matches, lineno)
+ self._add_test(test, modifiers, num_matches, expectations, options,
overrides_allowed)
- def _add_test(self, test, modifiers, expectations, options,
+ def _add_test(self, test, modifiers, num_matches, expectations, options,
overrides_allowed):
"""Sets the expected state for a given test.
@@ -773,6 +747,7 @@ 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
@@ -811,13 +786,13 @@ class TestExpectationsFile:
than a previous listing.
"""
if test in self._test_list_paths:
- 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)
+ 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_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.
@@ -829,32 +804,70 @@ class TestExpectationsFile:
if test in set_of_tests:
set_of_tests.remove(test)
- 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.
+ 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
"""
+ # 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 = 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
+ 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
- # Check if we've already seen a more precise path.
- return prev_base_path.startswith(os.path.normpath(test_list_path))
+ 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
def _add_error(self, lineno, msg, path):
"""Reports an error that will prevent running the tests. Does not
@@ -866,3 +879,188 @@ 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]
diff --git a/Tools/Scripts/webkitpy/layout_tests/layout_package/test_expectations_unittest.py b/Tools/Scripts/webkitpy/layout_tests/layout_package/test_expectations_unittest.py
index 34771f3..c74cf31 100644
--- a/Tools/Scripts/webkitpy/layout_tests/layout_package/test_expectations_unittest.py
+++ b/Tools/Scripts/webkitpy/layout_tests/layout_package/test_expectations_unittest.py
@@ -34,6 +34,7 @@ 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):
@@ -80,6 +81,9 @@ 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
@@ -105,13 +109,12 @@ 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,
- is_debug_mode=False):
+ def parse_exp(self, expectations, overrides=None, is_lint_mode=False):
+ test_config = self._port.test_configuration()
self._exp = TestExpectations(self._port,
tests=self.get_basic_tests(),
expectations=expectations,
- test_platform_name=self._port.test_platform_name(),
- is_debug_mode=is_debug_mode,
+ test_config=test_config,
is_lint_mode=is_lint_mode,
overrides=overrides)
@@ -120,7 +123,7 @@ BUG_TEST WONTFIX WIN : failures/expected/image.html = IMAGE
set([result]))
-class TestExpectationsTest(Base):
+class BasicTests(Base):
def test_basic(self):
self.parse_exp(self.get_basic_expectations())
self.assert_exp('failures/expected/text.html', TEXT)
@@ -128,23 +131,14 @@ class TestExpectationsTest(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
@@ -159,20 +153,6 @@ 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(
@@ -217,7 +197,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 Invalid modifier for test: foo failures/expected/text.html',
+ exp_errors = [u"Line:1 Unrecognized option '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)
@@ -233,77 +213,160 @@ SKIP : failures/expected/image.html""")
self.assertEqual(str(e), '\n'.join(map(str, exp_errors)))
self.assertEqual(e.errors, exp_errors)
- def test_syntax_missing_expectation(self):
+ 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):
# This is missing the expectation.
self.assertRaises(ParseError, self.parse_exp,
- 'BUG_TEST: failures/expected/text.html',
- is_debug_mode=True)
+ 'BUG_TEST: failures/expected/text.html')
- def test_syntax_invalid_option(self):
+ def test_missing_colon(self):
+ # This is missing the modifiers and the ':'
self.assertRaises(ParseError, self.parse_exp,
- 'BUG_TEST FOO: failures/expected/text.html = PASS')
+ 'failures/expected/text.html = TEXT')
- def test_syntax_invalid_expectation(self):
- # This is missing the expectation.
+ 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 = FOO')
+ 'BUG_TEST: failures/expected/text.html = TEXT = IMAGE')
+
+ def test_unrecognized_expectation(self):
+ 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')
- def test_syntax_missing_bugid(self):
+ def test_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_semantic_slow_and_timeout(self):
+ def test_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_semantic_rebaseline(self):
+ def test_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_semantic_duplicates(self):
+ def test_duplicates(self):
self.assertRaises(ParseError, self.parse_exp, """
-BUG_TEST : failures/expected/text.html = TEXT
-BUG_TEST : failures/expected/text.html = IMAGE""")
+BUG_EXP : failures/expected/text.html = TEXT
+BUG_EXP : failures/expected/text.html = IMAGE""")
self.assertRaises(ParseError, self.parse_exp,
- self.get_basic_expectations(), """
-BUG_TEST : failures/expected/text.html = TEXT
-BUG_TEST : failures/expected/text.html = IMAGE""")
+ self.get_basic_expectations(), overrides="""
+BUG_OVERRIDE : failures/expected/text.html = TEXT
+BUG_OVERRIDE : failures/expected/text.html = IMAGE""", )
- def test_semantic_missing_file(self):
+ def test_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)
- 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)
+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)
- def test_matches_an_expected_result(self):
+ 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 match(test, result, pixel_tests_enabled):
- return self._exp.matches_an_expected_result(
- self.get_test(test), result, pixel_tests_enabled)
+ def test_ambiguous(self):
+ self.assertRaises(ParseError, self.parse_exp, """
+BUG_TEST RELEASE : passes/text.html = PASS
+BUG_TEST MAC : passes/text.html = FAIL
+""")
- 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))
+ 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_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_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 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)
class RebaseliningTest(Base):
@@ -346,5 +409,86 @@ 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()
diff --git a/Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py b/Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py
index 5b02a00..2f48777 100644
--- a/Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py
+++ b/Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py
@@ -212,21 +212,13 @@ class TestRunner:
def lint(self):
lint_failed = False
-
- # 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
+ for test_configuration in self._port.all_test_configurations():
try:
- self.parse_expectations(platform_name, is_debug_mode=False)
+ self.lint_expectations(test_configuration)
except test_expectations.ParseError:
lint_failed = True
+ self._printer.write("")
- self._printer.write("")
if lint_failed:
_log.error("Lint failed.")
return -1
@@ -234,22 +226,28 @@ class TestRunner:
_log.info("Lint succeeded.")
return 0
- def parse_expectations(self, test_platform_name, is_debug_mode):
+ 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):
"""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."""
- 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()
+ port = self._port
self._expectations = test_expectations.TestExpectations(
- self._port, test_files, expectations_str, test_platform_name,
- is_debug_mode, self._options.lint_test_files,
- overrides=overrides_str)
- return self._expectations
+ port,
+ self._test_files,
+ port.test_expectations(),
+ port.test_configuration(),
+ self._options.lint_test_files,
+ port.test_expectations_overrides())
# FIXME: This method is way too long and needs to be broken into pieces.
def prepare_lists_and_print_output(self):
@@ -357,9 +355,7 @@ class TestRunner:
self._test_files_list = files + skip_chunk_list
self._test_files = set(self._test_files_list)
- self._expectations = self.parse_expectations(
- self._port.test_platform_name(),
- self._options.configuration == 'Debug')
+ self.parse_expectations()
self._test_files = set(files)
self._test_files_list = files
diff --git a/Tools/Scripts/webkitpy/layout_tests/port/base.py b/Tools/Scripts/webkitpy/layout_tests/port/base.py
index 97b54c9..062257d 100644
--- a/Tools/Scripts/webkitpy/layout_tests/port/base.py
+++ b/Tools/Scripts/webkitpy/layout_tests/port/base.py
@@ -121,6 +121,7 @@ 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
@@ -574,6 +575,19 @@ 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.
@@ -860,3 +874,68 @@ 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')
diff --git a/Tools/Scripts/webkitpy/layout_tests/port/chromium.py b/Tools/Scripts/webkitpy/layout_tests/port/chromium.py
index b90421a..5f2c15f 100644
--- a/Tools/Scripts/webkitpy/layout_tests/port/chromium.py
+++ b/Tools/Scripts/webkitpy/layout_tests/port/chromium.py
@@ -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, test_platform_name,
- is_debug_mode, is_lint_mode=False, overrides=overrides_str)
+ self, all_test_files, expectations_str, self.test_configuration(),
+ 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)]
diff --git a/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py b/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py
index c4b36ac..04eca63 100644
--- a/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py
+++ b/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py
@@ -95,3 +95,15 @@ 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)
diff --git a/Tools/Scripts/webkitpy/layout_tests/port/test.py b/Tools/Scripts/webkitpy/layout_tests/port/test.py
index 935881c..67e687c 100644
--- a/Tools/Scripts/webkitpy/layout_tests/port/test.py
+++ b/Tools/Scripts/webkitpy/layout_tests/port/test.py
@@ -151,6 +151,9 @@ 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
@@ -306,7 +309,7 @@ WONTFIX SKIP : failures/expected/exception.html = CRASH
return test_platform_name
def version(self):
- return ''
+ return '-leopard'
class TestDriver(base.Driver):
diff --git a/Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py b/Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py
index 4d8b7c9..b5a4ef5 100644
--- a/Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py
+++ b/Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py
@@ -183,8 +183,7 @@ class Rebaseliner(object):
test_expectations.TestExpectations(self._rebaseline_port,
None,
expectations_str,
- self._platform,
- False,
+ self._target_port.test_configuration(),
False)
self._scm = scm.default_scm()
diff --git a/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py b/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
index c431765..4a93db1 100755
--- a/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
+++ b/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
@@ -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()
- runner.parse_expectations(port.test_platform_name(),
- options.configuration == 'Debug')
+
+ printer.print_update("Parsing expectations ...")
+ runner.parse_expectations()
printer.print_update("Checking build ...")
if not port.check_build(runner.needs_http()):
diff --git a/Tools/Scripts/webkitpy/style/checkers/test_expectations.py b/Tools/Scripts/webkitpy/style/checkers/test_expectations.py
index c86b32c..e37f908 100644
--- a/Tools/Scripts/webkitpy/style/checkers/test_expectations.py
+++ b/Tools/Scripts/webkitpy/style/checkers/test_expectations.py
@@ -75,7 +75,6 @@ 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."
@@ -91,7 +90,7 @@ class TestExpectationsChecker(object):
try:
expectations = test_expectations.TestExpectationsFile(
port=self._port_obj, expectations=expectations_str, full_test_list=tests,
- test_platform_name=self._port_to_check, is_debug_mode=False,
+ test_config=self._port_obj.test_configuration(),
is_lint_mode=True, overrides=overrides)
except test_expectations.ParseError, error:
err = error
diff --git a/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py b/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py
index 9817c5d..8691557 100644
--- a/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py
+++ b/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py
@@ -84,15 +84,6 @@ class TestExpectationsTestCase(unittest.TestCase):
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"],
"")
self.assert_lines_lint(
@@ -120,12 +111,12 @@ class TestExpectationsTestCase(unittest.TestCase):
def test_modifier_errors(self):
self.assert_lines_lint(
["BUG1234 : passes/text.html = FAIL"],
- 'Bug must be either BUGCR, BUGWK, or BUGV8_ for test: bug1234 passes/text.html [test/expectations] [5]')
+ "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]")
def test_valid_modifiers(self):
self.assert_lines_lint(
["INVALID-MODIFIER : passes/text.html = PASS"],
- "Invalid modifier for test: invalid-modifier "
+ "Unrecognized option 'invalid-modifier' "
"passes/text.html [test/expectations] [5]")
self.assert_lines_lint(
["SKIP : passes/text.html = PASS"],
@@ -135,38 +126,38 @@ class TestExpectationsTestCase(unittest.TestCase):
def test_expectation_errors(self):
self.assert_lines_lint(
["missing expectations"],
- "Missing expectations. ['missing expectations'] [test/expectations] [5]")
+ "Missing a ':' 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(
- ["does/not/exist.html = FAIL"],
+ ["BUGWK1 : 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(
- ["passes/text.html = PASS"],
+ ["BUGWK1 : passes/text.html = PASS"],
"")
self.assert_lines_lint(
- ["passes/text.html = UNSUPPORTED"],
+ ["BUGWK1 : passes/text.html = UNSUPPORTED"],
"Unsupported expectation: unsupported "
"passes/text.html [test/expectations] [5]")
self.assert_lines_lint(
- ["passes/text.html = PASS UNSUPPORTED"],
+ ["BUGWK1 : passes/text.html = PASS UNSUPPORTED"],
"Unsupported expectation: unsupported "
"passes/text.html [test/expectations] [5]")
def test_already_seen_test(self):
self.assert_lines_lint(
- ["passes/text.html = PASS",
- "passes/text.html = TIMEOUT"],
- "Duplicate expectation. %s [test/expectations] [5]" % self._test_file)
+ ["BUGWK1 : passes/text.html = PASS",
+ "BUGWK1 : passes/text.html = TIMEOUT"],
+ "Duplicate or ambiguous expectation. %s [test/expectations] [5]" % self._test_file)
def test_tab(self):
self.assert_lines_lint(
- ["\tpasses/text.html = PASS"],
+ ["\tBUGWK1 : passes/text.html = PASS"],
"Line contains tab character. [whitespace/tab] [5]")
if __name__ == '__main__':
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list