From 015ce17ede0be4ee7ee186592dd0f105e1d02a6b Mon Sep 17 00:00:00 2001 From: "dpranke@chromium.org" Date: Wed, 27 Jun 2012 20:52:58 +0000 Subject: [PATCH] Derive ChromiumPort from WebKitPort to add support for missing symbols to skip tests https://bugs.webkit.org/show_bug.cgi?id=89706 Reviewed by Adam Barth. Based on the original patch by Raymond Toy. This patch changes ChromiumPort to derive from webkit.WebKitPort instead of base.Port. This is a long-awaited change and a precursor to merging base.Port and webkit.WebKitPort, but is driven by the desire to dynamically detect whether the MP3 and AAC codecs are compiled into DRT on Chromium, for which we wanted to re-use the existing logic in WebKit port for determine what to skip at compile time. Most of the changes are shuffling things around so that we don't change any other logic and so we override the necessary methods in WebKitPort (and try to follow the same method definition order where possible). Also, on the Chromium port the mp3 and aac codecs are actually defined in a separate library, so scanning webcore isn't sufficient. This patch generalizes the symbol lookup to handle multiple libraries, and uses different libraries as appropriate for Chromium. The only functional/visible changes should be in the value returned for skipped_layout_tests(). * Scripts/webkitpy/layout_tests/port/chromium.py: (ChromiumPort): (ChromiumPort.__init__): (ChromiumPort.driver_name): (ChromiumPort._driver_class): (ChromiumPort._missing_symbol_to_skipped_tests): (ChromiumPort.skipped_layout_tests): (ChromiumPort.setup_test_run): (ChromiumPort._path_to_image_diff): (ChromiumPort._convert_path): * Scripts/webkitpy/layout_tests/port/chromium_unittest.py: (ChromiumPortTest.test_missing_symbol_to_skipped_tests): * Scripts/webkitpy/layout_tests/port/chromium_linux.py: (ChromiumLinuxPort._modules_to_search_for_symbols): * Scripts/webkitpy/layout_tests/port/chromium_mac.py: (ChromiumLinuxPort._modules_to_search_for_symbols): * Scripts/webkitpy/layout_tests/port/chromium_win.py: (ChromiumLinuxPort._modules_to_search_for_symbols): * Scripts/webkitpy/layout_tests/port/webkit.py: (WebKitPort): (WebKitPort.__init__): (WebKitPort._symbols_string): (WebKitPort._modules_to_search_for_symbols): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@121363 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Tools/ChangeLog | 55 ++++++++++++++++++++++ .../Scripts/webkitpy/layout_tests/port/chromium.py | 38 +++++++++++---- .../webkitpy/layout_tests/port/chromium_linux.py | 3 ++ .../webkitpy/layout_tests/port/chromium_mac.py | 3 ++ .../layout_tests/port/chromium_unittest.py | 7 +++ .../webkitpy/layout_tests/port/chromium_win.py | 5 ++ Tools/Scripts/webkitpy/layout_tests/port/webkit.py | 43 ++++++++++------- .../webkitpy/layout_tests/port/webkit_unittest.py | 2 +- 8 files changed, 127 insertions(+), 29 deletions(-) diff --git a/Tools/ChangeLog b/Tools/ChangeLog index 4438503..584a6e8 100644 --- a/Tools/ChangeLog +++ b/Tools/ChangeLog @@ -1,3 +1,58 @@ +2012-06-27 Dirk Pranke + + Derive ChromiumPort from WebKitPort to add support for missing symbols to skip tests + https://bugs.webkit.org/show_bug.cgi?id=89706 + + Reviewed by Adam Barth. + + Based on the original patch by Raymond Toy. + + This patch changes ChromiumPort to derive from webkit.WebKitPort + instead of base.Port. This is a long-awaited change and a + precursor to merging base.Port and webkit.WebKitPort, but is + driven by the desire to dynamically detect whether the MP3 and + AAC codecs are compiled into DRT on Chromium, for which we + wanted to re-use the existing logic in WebKit port for determine + what to skip at compile time. + + Most of the changes are shuffling things around so that we don't + change any other logic and so we override the necessary methods + in WebKitPort (and try to follow the same method definition + order where possible). + + Also, on the Chromium port the mp3 and aac codecs are actually + defined in a separate library, so scanning webcore isn't + sufficient. This patch generalizes the symbol lookup to handle + multiple libraries, and uses different libraries as appropriate + for Chromium. + + The only functional/visible changes should be in the value + returned for skipped_layout_tests(). + + * Scripts/webkitpy/layout_tests/port/chromium.py: + (ChromiumPort): + (ChromiumPort.__init__): + (ChromiumPort.driver_name): + (ChromiumPort._driver_class): + (ChromiumPort._missing_symbol_to_skipped_tests): + (ChromiumPort.skipped_layout_tests): + (ChromiumPort.setup_test_run): + (ChromiumPort._path_to_image_diff): + (ChromiumPort._convert_path): + * Scripts/webkitpy/layout_tests/port/chromium_unittest.py: + (ChromiumPortTest.test_missing_symbol_to_skipped_tests): + * Scripts/webkitpy/layout_tests/port/chromium_linux.py: + (ChromiumLinuxPort._modules_to_search_for_symbols): + * Scripts/webkitpy/layout_tests/port/chromium_mac.py: + (ChromiumLinuxPort._modules_to_search_for_symbols): + * Scripts/webkitpy/layout_tests/port/chromium_win.py: + (ChromiumLinuxPort._modules_to_search_for_symbols): + * Scripts/webkitpy/layout_tests/port/webkit.py: + (WebKitPort): + (WebKitPort.__init__): + (WebKitPort._symbols_string): + (WebKitPort._modules_to_search_for_symbols): + 2012-06-27 Csaba Osztrogonác [Qt] REGRESSION(r121339): It broke the build on the Qt Windows bots diff --git a/Tools/Scripts/webkitpy/layout_tests/port/chromium.py b/Tools/Scripts/webkitpy/layout_tests/port/chromium.py index 5aad94c..08a706d 100755 --- a/Tools/Scripts/webkitpy/layout_tests/port/chromium.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/chromium.py @@ -42,14 +42,16 @@ from webkitpy.common.system import executive from webkitpy.layout_tests.models.test_configuration import TestConfiguration from webkitpy.layout_tests.port.base import Port, VirtualTestSuite from webkitpy.layout_tests.port.driver import DriverOutput -from webkitpy.layout_tests.port.webkit import WebKitDriver +from webkitpy.layout_tests.port.webkit import WebKitPort, WebKitDriver _log = logging.getLogger(__name__) -class ChromiumPort(Port): +class ChromiumPort(WebKitPort): """Abstract base class for Chromium implementations of the Port class.""" + pixel_tests_option_default = True + time_out_ms_option_default = 6 * 1000 ALL_SYSTEMS = ( ('leopard', 'x86'), @@ -106,7 +108,7 @@ class ChromiumPort(Port): return module_path[0:offset] def __init__(self, host, port_name, **kwargs): - Port.__init__(self, host, port_name, **kwargs) + super(ChromiumPort, self).__init__(host, port_name, **kwargs) # All sub-classes override this, but we need an initial value for testing. self._chromium_base_dir_path = None @@ -129,6 +131,11 @@ class ChromiumPort(Port): return False return True + def driver_name(self): + # FIXME: merge this with Port.driver_name, WebKitPort.driver_name + if self.get_option('driver_name'): + return self.get_option('driver_name') + return 'DumpRenderTree' def check_build(self, needs_http): result = True @@ -256,6 +263,20 @@ class ChromiumPort(Port): except AssertionError: return self._build_path(self.get_option('configuration'), 'layout-test-results') + def _driver_class(self): + return ChromiumDriver + + def _missing_symbol_to_skipped_tests(self): + # FIXME: Should WebKitPort have these definitions also? + return { + "ff_mp3_decoder": ["webaudio/codec-tests/mp3"], + "ff_aac_decoder": ["webaudio/codec-tests/aac"], + } + + def skipped_layout_tests(self, test_list): + # FIXME: Merge w/ WebKitPort.skipped_layout_tests() + return set(self._skipped_tests_for_unsupported_features(test_list)) + def setup_test_run(self): # Delete the disk cache if any to ensure a clean test run. dump_render_tree_binary_path = self._path_to_driver() @@ -264,9 +285,6 @@ class ChromiumPort(Port): if self._filesystem.exists(cachedir): self._filesystem.rmtree(cachedir) - def _driver_class(self): - return ChromiumDriver - def start_helper(self): helper_path = self._path_to_helper() if helper_path: @@ -365,6 +383,10 @@ class ChromiumPort(Port): def _build_path(self, *comps): return self._static_build_path(self._filesystem, self.get_option('build_directory'), self.path_from_chromium_base(), self.path_from_webkit_base(), *comps) + def _path_to_image_diff(self): + binary_name = 'ImageDiff' + return self._build_path(self.get_option('configuration'), binary_name) + def _check_driver_build_up_to_date(self, configuration): if configuration in ('Debug', 'Release'): try: @@ -400,10 +422,6 @@ class ChromiumPort(Port): return cygpath(path) return path - def _path_to_image_diff(self): - binary_name = 'ImageDiff' - return self._build_path(self.get_option('configuration'), binary_name) - class ChromiumDriver(WebKitDriver): KILL_TIMEOUT_DEFAULT = 3.0 diff --git a/Tools/Scripts/webkitpy/layout_tests/port/chromium_linux.py b/Tools/Scripts/webkitpy/layout_tests/port/chromium_linux.py index f26bea4..e54078d 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/chromium_linux.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/chromium_linux.py @@ -110,6 +110,9 @@ class ChromiumLinuxPort(chromium.ChromiumPort): port_names = self.FALLBACK_PATHS[self._architecture] return map(self._webkit_baseline_path, port_names) + def _modules_to_search_for_symbols(self): + return [self._build_path(self.get_option('configuration'), 'libffmpegsumo.so')] + def check_build(self, needs_http): result = chromium.ChromiumPort.check_build(self, needs_http) if not result: diff --git a/Tools/Scripts/webkitpy/layout_tests/port/chromium_mac.py b/Tools/Scripts/webkitpy/layout_tests/port/chromium_mac.py index f748cd1..a138e7e 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/chromium_mac.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/chromium_mac.py @@ -88,6 +88,9 @@ class ChromiumMacPort(chromium.ChromiumPort): fallback_paths = self.FALLBACK_PATHS return map(self._webkit_baseline_path, fallback_paths[self._version]) + def _modules_to_search_for_symbols(self): + return [self._build_path(self.get_option('configuration'), 'ffmpegsumo.so')] + def check_build(self, needs_http): result = chromium.ChromiumPort.check_build(self, needs_http) if not result: diff --git a/Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py b/Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py index 492deed..5822669 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py @@ -180,6 +180,13 @@ class ChromiumPortTest(port_testcase.PortTestCase): port_name = 'chromium-mac' port_maker = chromium.ChromiumPort + def test_missing_symbol_to_skipped_tests(self): + # Test that we get the chromium skips and not the webkit default skips + port = self.make_port() + skip_dict = port._missing_symbol_to_skipped_tests() + self.assertTrue('ff_mp3_decoder' in skip_dict) + self.assertFalse('WebGLShader' in skip_dict) + def test_all_test_configurations(self): """Validate the complete set of configurations this port knows about.""" port = self.make_port() diff --git a/Tools/Scripts/webkitpy/layout_tests/port/chromium_win.py b/Tools/Scripts/webkitpy/layout_tests/port/chromium_win.py index ff58842..0731716 100755 --- a/Tools/Scripts/webkitpy/layout_tests/port/chromium_win.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/chromium_win.py @@ -106,6 +106,11 @@ class ChromiumWinPort(chromium.ChromiumPort): port_names = self.FALLBACK_PATHS[self.version()] return map(self._webkit_baseline_path, port_names) + def _modules_to_search_for_symbols(self): + # FIXME: we should return the path to the ffmpeg equivalents to detect if we have the mp3 and aac codecs installed. + # See https://bugs.webkit.org/show_bug.cgi?id=89706. + return [] + def check_build(self, needs_http): result = chromium.ChromiumPort.check_build(self, needs_http) if not result: diff --git a/Tools/Scripts/webkitpy/layout_tests/port/webkit.py b/Tools/Scripts/webkitpy/layout_tests/port/webkit.py index 2a1e6e8..3098c3b 100755 --- a/Tools/Scripts/webkitpy/layout_tests/port/webkit.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/webkit.py @@ -51,15 +51,17 @@ _log = logging.getLogger(__name__) class WebKitPort(Port): + pixel_tests_option_default = False # FIXME: Disable until they are run by default on build.webkit.org. + time_out_ms_option_default = 35 * 1000 + def __init__(self, host, port_name=None, **kwargs): Port.__init__(self, host, port_name=port_name, **kwargs) + self.set_option_default("pixel_tests", self.pixel_tests_option_default) - # FIXME: Disable pixel tests until they are run by default on build.webkit.org. - self.set_option_default("pixel_tests", False) - # WebKit ports expect a 35s timeout, or 350s timeout when running with -g/--guard-malloc. # FIXME: --guard-malloc is only supported on Mac, so this logic should be in mac.py. - default_time_out_seconds = 350 if self.get_option('guard_malloc') else 35 - self.set_option_default("time_out_ms", default_time_out_seconds * 1000) + if self.get_option('guard_malloc'): + self.time_out_ms_option_default = 350 * 1000 + self.set_option_default("time_out_ms", self.time_out_ms_option_default) def driver_name(self): if self.get_option('webkit_test_runner'): @@ -264,15 +266,20 @@ class WebKitPort(Port): def nm_command(self): return 'nm' - def _webcore_symbols_string(self): - webcore_library_path = self._path_to_webcore_library() - if not webcore_library_path: - return None - try: - return self._executive.run_command([self.nm_command(), 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 _modules_to_search_for_symbols(self): + path = self._path_to_webcore_library() + if path: + return [path] + return [] + + def _symbols_string(self): + symbols = '' + for path_to_module in self._modules_to_search_for_symbols(): + try: + symbols += self._executive.run_command([self.nm_command(), path_o_module], error_handler=Executive.ignore_error) + except OSError, e: + _log.warn("Failed to run nm: %s. Can't determine supported features correctly." % e) + return symbols # Ports which use run-time feature detection should define this method and return # a dictionary mapping from Feature Names to skipped directoires. NRWT will @@ -331,10 +338,10 @@ class WebKitPort(Port): # This is a performance optimization to avoid the calling nm. if self._has_test_in_directories(self._missing_symbol_to_skipped_tests().values(), test_list): # 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], []) + # Disable any tests for symbols missing from the executable or libraries. + symbols_string = self._symbols_string() + if 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 symbols_string], []) # Failed to get any runtime or symbol information, don't skip any tests. return [] diff --git a/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py b/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py index f7e9525..ea36fb4 100755 --- a/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py @@ -53,7 +53,7 @@ class TestWebKitPort(WebKitPort): def all_test_configurations(self): return [self.test_configuration()] - def _webcore_symbols_string(self): + def _symbols_string(self): return self.symbols_string def _tests_for_other_platforms(self): -- 2.7.4