- Revision
- 99377
- Author
- [email protected]
- Date
- 2011-11-06 11:48:20 -0800 (Sun, 06 Nov 2011)
Log Message
Clarify how the symbol and runtime-feature based test exclusion works and cleanup the code a bit
https://bugs.webkit.org/show_bug.cgi?id=66078
Reviewed by Adam Barth.
The runtime feature detection was fixed to work in bug 64472.
In this bug I moved the symbol-based feature detection from
popen() to Executive.run_command and cleaned up the callers
and unittests to make sure that we're correctly parsing the
nm output correctly. The old code happened to work even though
the runtime-features path was using "str in list" and the
symbol features path was using "str in str" and it just happened
to do what we wanted to. Now runtime features and symbol feature
blacklists are computed separately (and with better documentation).
This system remains confusing, partially because these are black-lists
which are amended to whatever static blacklist may exist for the
port as part of a Skipped list file.
For example, notice how the runtime feature list only has directory
blacklists for a couple features. If all features are off,
how do we skip enough tests with only 2 entries in the blacklist map?
The answer is that Windows is the only port to use runtime feature
detection, and the win/Skipped file turns off all the other features
statically (like mathml, mhtml, wss, etc.) where as some other ports (like AppleMac)
which use symbol-based feature detection turn of mathml, wcss, etc
using the blacklists found in _missing_symbol_to_skipped_tests.
I also noticed a couple places where we still referenced xhtmlmp
even though support for such has been removed from WebKit. Removed those.
This should result in no functional change.
* Scripts/webkitpy/layout_tests/port/gtk.py:
- Use self._filesystem instead of os.path
* Scripts/webkitpy/layout_tests/port/webkit.py:
* Scripts/webkitpy/layout_tests/port/webkit_unittest.py:
Modified Paths
Diff
Modified: trunk/Tools/ChangeLog (99376 => 99377)
--- trunk/Tools/ChangeLog 2011-11-06 17:22:14 UTC (rev 99376)
+++ trunk/Tools/ChangeLog 2011-11-06 19:48:20 UTC (rev 99377)
@@ -1,3 +1,43 @@
+2011-11-06 Eric Seidel <[email protected]>
+
+ Clarify how the symbol and runtime-feature based test exclusion works and cleanup the code a bit
+ https://bugs.webkit.org/show_bug.cgi?id=66078
+
+ Reviewed by Adam Barth.
+
+ The runtime feature detection was fixed to work in bug 64472.
+ In this bug I moved the symbol-based feature detection from
+ popen() to Executive.run_command and cleaned up the callers
+ and unittests to make sure that we're correctly parsing the
+ nm output correctly. The old code happened to work even though
+ the runtime-features path was using "str in list" and the
+ symbol features path was using "str in str" and it just happened
+ to do what we wanted to. Now runtime features and symbol feature
+ blacklists are computed separately (and with better documentation).
+
+ This system remains confusing, partially because these are black-lists
+ which are amended to whatever static blacklist may exist for the
+ port as part of a Skipped list file.
+
+ For example, notice how the runtime feature list only has directory
+ blacklists for a couple features. If all features are off,
+ how do we skip enough tests with only 2 entries in the blacklist map?
+ The answer is that Windows is the only port to use runtime feature
+ detection, and the win/Skipped file turns off all the other features
+ statically (like mathml, mhtml, wss, etc.) where as some other ports (like AppleMac)
+ which use symbol-based feature detection turn of mathml, wcss, etc
+ using the blacklists found in _missing_symbol_to_skipped_tests.
+
+ I also noticed a couple places where we still referenced xhtmlmp
+ even though support for such has been removed from WebKit. Removed those.
+
+ This should result in no functional change.
+
+ * Scripts/webkitpy/layout_tests/port/gtk.py:
+ - Use self._filesystem instead of os.path
+ * Scripts/webkitpy/layout_tests/port/webkit.py:
+ * Scripts/webkitpy/layout_tests/port/webkit_unittest.py:
+
2011-11-06 Dan Bernstein <[email protected]>
Made show-pretty-diff work when the input path contains spaces.
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/gtk.py (99376 => 99377)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/gtk.py 2011-11-06 17:22:14 UTC (rev 99376)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/gtk.py 2011-11-06 19:48:20 UTC (rev 99377)
@@ -117,7 +117,7 @@
for library in gtk_library_names:
full_library = self._build_path(".libs", library)
- if os.path.isfile(full_library):
+ if self._filesystem.isfile(full_library):
return full_library
return None
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/test_files.py (99376 => 99377)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/test_files.py 2011-11-06 17:22:14 UTC (rev 99376)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/test_files.py 2011-11-06 19:48:20 UTC (rev 99377)
@@ -43,7 +43,7 @@
# When collecting test cases, we include any file with these extensions.
-_supported_file_extensions = set(['.html', '.shtml', '.xml', '.xhtml', '.xhtmlmp', '.pl',
+_supported_file_extensions = set(['.html', '.shtml', '.xml', '.xhtml', '.pl',
'.htm', '.php', '.svg', '.mht'])
# When collecting test cases, skip these directories
_skipped_directories = set(['.svn', '_svn', 'resources', 'script-tests'])
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py (99376 => 99377)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py 2011-11-06 17:22:14 UTC (rev 99376)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py 2011-11-06 19:48:20 UTC (rev 99377)
@@ -248,64 +248,72 @@
try:
output = self._executive.run_command(supported_features_command, error_handler=Executive.ignore_error)
except OSError, e:
- _log.warn("Exception runnig driver: %s, %s. Driver must be built before calling WebKitPort.test_expectations()." % (supported_features_command, e))
- return []
+ _log.warn("Exception running driver: %s, %s. Driver must be built before calling WebKitPort.test_expectations()." % (supported_features_command, e))
+ return None
# Note: win/DumpRenderTree.cpp does not print a leading space before the features_string.
match_object = re.match("SupportedFeatures:\s*(?P<features_string>.*)\s*", output)
if not match_object:
- return []
+ return None
return match_object.group('features_string').split(' ')
- def _supported_symbol_list(self):
- """Return the supported symbols of WebCore."""
+ def _webcore_symbols_string(self):
webcore_library_path = self._path_to_webcore_library()
if not webcore_library_path:
- return []
- symbol_list = ' '.join(os.popen("nm " + webcore_library_path).readlines())
- return symbol_list
+ return None
+ try:
+ return self._executive.run_command('nm', webcore_library_path, error_handler=Executive.ignore_error)
+ except OSError, e:
+ _log.warn("Failed to run nm: %s. Can't determine WebCore supported features." % e)
+ return None
- def _directories_for_features(self):
- """Return the supported feature dictionary. The keys are the
- features and the values are the directories in lists."""
- directories_for_features = {
+ # Ports which use run-time feature detection should define this method and return
+ # a dictionary mapping from Feature Names to skipped directoires. NRWT will
+ # run DumpRenderTree --print-supported-features and parse the output.
+ # If the Feature Names are not found in the output, the corresponding directories
+ # will be skipped.
+ def _missing_feature_to_skipped_tests(self):
+ """Return the supported feature dictionary. Keys are feature names and values
+ are the lists of directories to skip if the feature name is not matched."""
+ # FIXME: This list matches WebKitWin and should be moved onto the Win port.
+ return {
"Accelerated Compositing": ["compositing"],
"3D Rendering": ["animations/3d", "transforms/3d"],
}
- return directories_for_features
- def _directories_for_symbols(self):
- """Return the supported feature dictionary. The keys are the
- symbols and the values are the directories in lists."""
- directories_for_symbol = {
+ # Ports which use compile-time feature detection should define this method and return
+ # a dictionary mapping from symbol substrings to possibly disabled test directories.
+ # When the symbol substrings are not matched, the directories will be skipped.
+ # If ports don't ever enable certain features, then those directories can just be
+ # in the Skipped list instead of compile-time-checked here.
+ def _missing_symbol_to_skipped_tests(self):
+ """Return the supported feature dictionary. The keys are symbol-substrings
+ and the values are the lists of directories to skip if that symbol is missing."""
+ return {
"MathMLElement": ["mathml"],
"GraphicsLayer": ["compositing"],
"WebCoreHas3DRendering": ["animations/3d", "transforms/3d"],
"WebGLShader": ["fast/canvas/webgl", "compositing/webgl", "http/tests/canvas/webgl"],
"MHTMLArchive": ["mhtml"],
}
- return directories_for_symbol
def _skipped_tests_for_unsupported_features(self):
- """Return the directories of unsupported tests. Search for the
- symbols in the symbol_list, if found add the corresponding
- directories to the skipped directory list."""
- feature_list = self._runtime_feature_list()
- directories = self._directories_for_features()
+ # If the port supports runtime feature detection, disable any tests
+ # for features missing from the runtime feature list.
+ supported_feature_list = self._runtime_feature_list()
+ # If _runtime_feature_list returns a non-None value, then prefer
+ # runtime feature detection over static feature detection.
+ if supported_feature_list is not None:
+ return reduce(operator.add, [directories for feature, directories in self._missing_feature_to_skipped_tests().items() if feature not in supported_feature_list])
- # if DRT feature detection not supported
- if not feature_list:
- feature_list = self._supported_symbol_list()
- directories = self._directories_for_symbols()
+ # Runtime feature detection not supported, fallback to static dectection:
+ # Disable any tests for symbols missing from the webcore symbol string.
+ webcore_symbols_string = self._webcore_symbols_string()
+ if webcore_symbols_string is not None:
+ return reduce(operator.add, [directories for symbol_substring, directories in self._missing_symbol_to_skipped_tests().items() if symbol_substring not in webcore_symbols_string], [])
+ # Failed to get any runtime or symbol information, don't skip any tests.
+ return []
- if not feature_list:
- return []
-
- skipped_directories = [directories[feature]
- for feature in directories.keys()
- if feature not in feature_list]
- return reduce(operator.add, skipped_directories)
-
def _tests_from_skipped_file_contents(self, skipped_file_contents):
tests_to_skip = []
for line in skipped_file_contents.split('\n'):
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py (99376 => 99377)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py 2011-11-06 17:22:14 UTC (rev 99376)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py 2011-11-06 19:48:20 UTC (rev 99377)
@@ -43,12 +43,12 @@
class TestWebKitPort(WebKitPort):
port_name = "testwebkitport"
- def __init__(self, symbol_list=None, feature_list=None,
+ def __init__(self, symbols_string=None, feature_list=None,
expectations_file=None, skips_file=None,
executive=None, filesystem=None, user=None,
**kwargs):
- self.symbol_list = symbol_list
- self.feature_list = feature_list
+ self.symbols_string = symbols_string # Passing "" disables all staticly-detectable features.
+ self.feature_list = feature_list # Passing [] disables all runtime-detectable features.
executive = executive or MockExecutive(should_log=False)
filesystem = filesystem or MockFileSystem()
user = user or MockUser()
@@ -60,8 +60,8 @@
def _runtime_feature_list(self):
return self.feature_list
- def _supported_symbol_list(self):
- return self.symbol_list
+ def _webcore_symbols_string(self):
+ return self.symbols_string
def _tests_for_other_platforms(self):
return ["media", ]
@@ -99,15 +99,35 @@
pass
def test_skipped_directories_for_symbols(self):
- supported_symbols = ["GraphicsLayer", "WebCoreHas3DRendering", "fooSymbol"]
- expected_directories = set(["mathml", "fast/canvas/webgl", "compositing/webgl", "http/tests/canvas/webgl", "mhtml"])
- result_directories = set(TestWebKitPort(supported_symbols, None)._skipped_tests_for_unsupported_features())
+ # This first test confirms that the commonly found symbols result in the expected skipped directories.
+ symbols_string = " ".join(["GraphicsLayer", "WebCoreHas3DRendering", "isXHTMLMPDocument", "fooSymbol"])
+ expected_directories = set([
+ "mathml", # Requires MathMLElement
+ "fast/canvas/webgl", # Requires WebGLShader
+ "compositing/webgl", # Requires WebGLShader
+ "http/tests/canvas/webgl", # Requires WebGLShader
+ "mhtml", # Requires MHTMLArchive
+ ])
+
+ result_directories = set(TestWebKitPort(symbols_string, None)._skipped_tests_for_unsupported_features())
self.assertEqual(result_directories, expected_directories)
+ # Test that the nm string parsing actually works:
+ symbols_string = """
+000000000124f498 s __ZZN7WebCore13GraphicsLayer12replaceChildEPS0_S1_E19__PRETTY_FUNCTION__
+000000000124f500 s __ZZN7WebCore13GraphicsLayer13addChildAboveEPS0_S1_E19__PRETTY_FUNCTION__
+000000000124f670 s __ZZN7WebCore13GraphicsLayer13addChildBelowEPS0_S1_E19__PRETTY_FUNCTION__
+"""
+ # Note 'compositing' is not in the list of skipped directories (hence the parsing of GraphicsLayer worked):
+ expected_directories = set(['mathml', 'transforms/3d', 'compositing/webgl', 'fast/canvas/webgl', 'animations/3d', 'mhtml', 'http/tests/canvas/webgl'])
+ result_directories = set(TestWebKitPort(symbols_string, None)._skipped_tests_for_unsupported_features())
+ self.assertEqual(result_directories, expected_directories)
+
def test_runtime_feature_list(self):
port = WebKitPort(executive=MockExecutive())
port._executive.run_command = lambda command, cwd=None, error_handler=None: "Nonsense"
- self.assertEquals(port._runtime_feature_list(), [])
+ # runtime_features_list returns None when its results are meaningless (it couldn't run DRT or parse the output, etc.)
+ self.assertEquals(port._runtime_feature_list(), None)
port._executive.run_command = lambda command, cwd=None, error_handler=None: "SupportedFeatures:foo bar"
self.assertEquals(port._runtime_feature_list(), ['foo', 'bar'])
@@ -118,7 +138,7 @@
self.assertEqual(result_directories, expected_directories)
def test_skipped_layout_tests(self):
- self.assertEqual(TestWebKitPort(None, None).skipped_layout_tests(), set(["media"]))
+ self.assertEqual(TestWebKitPort(None, None).skipped_layout_tests(), set(['media']))
def test_skipped_file_search_paths(self):
port = TestWebKitPort()
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/servers/lighttpd.conf (99376 => 99377)
--- trunk/Tools/Scripts/webkitpy/layout_tests/servers/lighttpd.conf 2011-11-06 17:22:14 UTC (rev 99376)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/servers/lighttpd.conf 2011-11-06 19:48:20 UTC (rev 99377)
@@ -21,7 +21,6 @@
".html" => "text/html",
".htm" => "text/html",
".xhtml" => "application/xhtml+xml",
- ".xhtmlmp" => "application/vnd.wap.xhtml+xml",
".js" => "application/x-_javascript_",
".log" => "text/plain",
".conf" => "text/plain",
