Diff
Modified: trunk/Tools/ChangeLog (90825 => 90826)
--- trunk/Tools/ChangeLog 2011-07-12 17:51:16 UTC (rev 90825)
+++ trunk/Tools/ChangeLog 2011-07-12 17:52:54 UTC (rev 90826)
@@ -1,3 +1,48 @@
+2011-07-12 Eric Seidel <[email protected]>
+
+ NRWT should open test results page with Safari trunk, not the system provided one on Mac
+ https://bugs.webkit.org/show_bug.cgi?id=64346
+
+ Reviewed by Adam Barth.
+
+ To fix this I implemented Port.show_results_html_file in Mac, Gtk and Qt ports with
+ implementations (mostly) matching those found in old-run-webkit-tests.
+ There are still some minor differences for Qt which Qt hackers may wish to tweak.
+
+ I had to add a WebKitPort._port_flag_for_scripts method (similar to flag() in common.config.ports.py)
+ for the Qt/Gtk ports which always require a flag passed to scripts.
+
+ While trying to test this, I found that FactoryTest.test_chromium_gpu_linux
+ was using a real filesystem (due to assert_platform) which due to
+ global variables in config.py was causing set-webkit-configuration to have
+ an affect on unit test results! So I fixed this by making FactoryTest.assert_port
+ pass a mock file system whenever calling factory.get.
+
+ Unfortunately TestPort was depending on always being passed a None filesystem
+ and asserting filesystem._tests (only true for unit_test_filesystem()).
+ So I just removed the FactoryTest.test_test and FactoryTest.test_dryrun tests
+ deciding that they were pretty much useless anyway. If others feel strongly
+ I'm happy to fix this in a different way.
+
+ * Scripts/webkitpy/common/system/executive.py:
+ - default arguments in python are screwy. They use a single shared
+ instance, so it's better to use argument=None and then argument = argument or Default()
+ if you have any chance of mutating (or returning) the default argument.
+ * Scripts/webkitpy/layout_tests/port/config.py:
+ - This code is wrong. We don't need to use a global variable here (as far as I can tell).
+ I'm not fixing it in this patch, but I've marked it with a FIXME and we can convert to
+ storing the results of the read on the Config object (which should only be created once during normal operation).
+ Unit tests shouldn't be hitting the disk anyway. It's possible Config should move off of Port and onto Tool/Host directly.
+ * Scripts/webkitpy/layout_tests/port/factory.py:
+ * Scripts/webkitpy/layout_tests/port/factory_unittest.py:
+ * Scripts/webkitpy/layout_tests/port/gtk.py:
+ * Scripts/webkitpy/layout_tests/port/gtk_unittest.py: Added.
+ * Scripts/webkitpy/layout_tests/port/mac.py:
+ * Scripts/webkitpy/layout_tests/port/mac_unittest.py:
+ * Scripts/webkitpy/layout_tests/port/qt.py:
+ * Scripts/webkitpy/layout_tests/port/qt_unittest.py:
+ * Scripts/webkitpy/layout_tests/port/webkit.py:
+
2011-07-12 Adam Roben <[email protected]>
Teach TestFailures to recognize when run-webkit-tests gets killed by buildbot
Modified: trunk/Tools/Scripts/webkitpy/common/system/executive.py (90825 => 90826)
--- trunk/Tools/Scripts/webkitpy/common/system/executive.py 2011-07-12 17:51:16 UTC (rev 90825)
+++ trunk/Tools/Scripts/webkitpy/common/system/executive.py 2011-07-12 17:52:54 UTC (rev 90826)
@@ -183,7 +183,8 @@
return 2
@staticmethod
- def interpreter_for_script(script_path, fs=FileSystem()):
+ def interpreter_for_script(script_path, fs=None):
+ fs = fs or FileSystem()
lines = fs.read_text_file(script_path).splitlines()
if not len(lines):
return None
@@ -199,7 +200,8 @@
return None
@staticmethod
- def shell_command_for_script(script_path, fs=FileSystem()):
+ def shell_command_for_script(script_path, fs=None):
+ fs = fs or FileSystem()
# Win32 does not support shebang. We need to detect the interpreter ourself.
if sys.platform == 'win32':
interpreter = Executive.interpreter_for_script(script_path, fs)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/config.py (90825 => 90826)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/config.py 2011-07-12 17:51:16 UTC (rev 90825)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/config.py 2011-07-12 17:52:54 UTC (rev 90826)
@@ -131,6 +131,8 @@
#
# FIXME: See the comment at the top of the file regarding unit tests
# and our use of global mutable static variables.
+ # FIXME: We should just @memoize this method and then this will only
+ # be read once per object lifetime (which should be sufficiently fast).
global _have_determined_configuration, _configuration
if not _have_determined_configuration:
contents = self._read_configuration()
@@ -146,8 +148,7 @@
def _read_configuration(self):
try:
- configuration_path = self._filesystem.join(self.build_directory(None),
- "Configuration")
+ configuration_path = self._filesystem.join(self.build_directory(None), "Configuration")
if not self._filesystem.exists(configuration_path):
return None
except (OSError, executive.ScriptError):
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/factory.py (90825 => 90826)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/factory.py 2011-07-12 17:51:16 UTC (rev 90825)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/factory.py 2011-07-12 17:52:54 UTC (rev 90826)
@@ -75,8 +75,7 @@
port_to_use = 'mac'
if port_to_use is None:
- raise NotImplementedError('unknown port; sys.platform = "%s"' %
- sys.platform)
+ raise NotImplementedError('unknown port; sys.platform = "%s"' % sys.platform)
if port_to_use.startswith('test'):
import test
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/factory_unittest.py (90825 => 90826)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/factory_unittest.py 2011-07-12 17:51:16 UTC (rev 90825)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/factory_unittest.py 2011-07-12 17:52:54 UTC (rev 90826)
@@ -29,7 +29,8 @@
import sys
import unittest
-from webkitpy.tool import mocktool
+from webkitpy.common.system.filesystem_mock import MockFileSystem
+from webkitpy.tool.mocktool import MockOptions, MockUser, MockExecutive
import chromium_gpu
import chromium_linux
@@ -56,9 +57,8 @@
def setUp(self):
self.real_sys_platform = sys.platform
- self.webkit_options = mocktool.MockOptions(pixel_tests=False)
- self.chromium_options = mocktool.MockOptions(pixel_tests=False,
- chromium=True)
+ self.webkit_options = MockOptions(pixel_tests=False)
+ self.chromium_options = MockOptions(pixel_tests=False, chromium=True)
def tearDown(self):
sys.platform = self.real_sys_platform
@@ -71,7 +71,8 @@
expected_port: class of expected port object.
port_obj: optional port object
"""
- port_obj = port_obj or factory.get(port_name=port_name)
+
+ port_obj = port_obj or factory.get(port_name=port_name, filesystem=MockFileSystem(), user=MockUser(), executive=MockExecutive())
self.assertTrue(isinstance(port_obj, expected_port))
def assert_platform_port(self, platform, options, expected_port):
@@ -85,17 +86,9 @@
"""
orig_platform = sys.platform
sys.platform = platform
- self.assertTrue(isinstance(factory.get(options=options),
- expected_port))
+ self.assertTrue(isinstance(factory.get(options=options, filesystem=MockFileSystem(), user=MockUser(), executive=MockExecutive()), expected_port))
sys.platform = orig_platform
- def test_test(self):
- self.assert_port("test", test.TestPort)
-
- def test_dryrun(self):
- self.assert_port("dryrun-test", dryrun.DryRunPort)
- self.assert_port("dryrun-mac", dryrun.DryRunPort)
-
def test_mac(self):
self.assert_port("mac", mac.MacPort)
self.assert_platform_port("darwin", None, mac.MacPort)
@@ -111,14 +104,10 @@
def test_google_chrome(self):
# The actual Chrome class names aren't available so we test that the
# objects we get are at least subclasses of the Chromium versions.
- self.assert_port("google-chrome-linux32",
- chromium_linux.ChromiumLinuxPort)
- self.assert_port("google-chrome-linux64",
- chromium_linux.ChromiumLinuxPort)
- self.assert_port("google-chrome-win",
- chromium_win.ChromiumWinPort)
- self.assert_port("google-chrome-mac",
- chromium_mac.ChromiumMacPort)
+ self.assert_port("google-chrome-linux32", chromium_linux.ChromiumLinuxPort)
+ self.assert_port("google-chrome-linux64", chromium_linux.ChromiumLinuxPort)
+ self.assert_port("google-chrome-win", chromium_win.ChromiumWinPort)
+ self.assert_port("google-chrome-mac", chromium_mac.ChromiumMacPort)
def test_gtk(self):
self.assert_port("gtk", gtk.GtkPort)
@@ -157,8 +146,7 @@
def test_unknown_specified(self):
# Test what happens when you specify an unknown port.
orig_platform = sys.platform
- self.assertRaises(NotImplementedError, factory.get,
- port_name='unknown')
+ self.assertRaises(NotImplementedError, factory.get, port_name='unknown')
def test_unknown_default(self):
# Test what happens when you're running on an unknown platform.
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/gtk.py (90825 => 90826)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/gtk.py 2011-07-12 17:51:16 UTC (rev 90825)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/gtk.py 2011-07-12 17:52:54 UTC (rev 90826)
@@ -59,6 +59,9 @@
class GtkPort(webkit.WebKitPort):
port_name = "gtk"
+ def _port_flag_for_scripts(self):
+ return "--gtk"
+
def create_driver(self, worker_number):
return GtkDriver(self, worker_number)
@@ -105,3 +108,13 @@
def _runtime_feature_list(self):
return None
+
+ # FIXME: We should find a way to share this implmentation with Gtk,
+ # or teach run-launcher how to call run-safari and move this down to WebKitPort.
+ def show_results_html_file(self, results_filename):
+ run_launcher_args = ["file://%s" % results_filename]
+ if self.get_option('webkit_test_runner'):
+ run_launcher_args.append('--webkit-test-runner')
+ # FIXME: old-run-webkit-tests also added ["-graphicssystem", "raster", "-style", "windows"]
+ # FIXME: old-run-webkit-tests converted results_filename path for cygwin.
+ self._run_script("run-launcher", run_launcher_args)
Added: trunk/Tools/Scripts/webkitpy/layout_tests/port/gtk_unittest.py (0 => 90826)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/gtk_unittest.py (rev 0)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/gtk_unittest.py 2011-07-12 17:52:54 UTC (rev 90826)
@@ -0,0 +1,45 @@
+# Copyright (C) 2011 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+# * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+# * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+import unittest
+
+from webkitpy.common.system.outputcapture import OutputCapture
+from webkitpy.layout_tests.port.gtk import GtkPort
+from webkitpy.layout_tests.port import port_testcase
+from webkitpy.tool.mocktool import MockExecutive
+
+
+class GtkPortTest(port_testcase.PortTestCase):
+ def port_maker(self, platform):
+ return GtkPort
+
+ def test_show_results_html_file(self):
+ port = self.make_port()
+ port._executive = MockExecutive(should_log=True)
+ expected_stderr = "MOCK run_command: ['Tools/Scripts/run-launcher', '--release', '--gtk', 'file://test.html']\n"
+ OutputCapture().assert_outputs(self, port.show_results_html_file, ["test.html"], expected_stderr=expected_stderr)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/mac.py (90825 => 90826)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/mac.py 2011-07-12 17:51:16 UTC (rev 90825)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/mac.py 2011-07-12 17:52:54 UTC (rev 90826)
@@ -115,3 +115,6 @@
def _path_to_webcore_library(self):
return self._build_path('WebCore.framework/Versions/A/WebCore')
+
+ def show_results_html_file(self, results_filename):
+ self._run_script('run-safari', ['-NSOpen', results_filename])
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/mac_unittest.py (90825 => 90826)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/mac_unittest.py 2011-07-12 17:51:16 UTC (rev 90825)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/mac_unittest.py 2011-07-12 17:52:54 UTC (rev 90826)
@@ -33,11 +33,13 @@
from webkitpy.layout_tests.port.mac import MacPort
from webkitpy.layout_tests.port import port_testcase
from webkitpy.common.system.filesystem_mock import MockFileSystem
+from webkitpy.common.system.outputcapture import OutputCapture
from webkitpy.tool.mocktool import MockOptions, MockUser, MockExecutive
class MacTest(port_testcase.PortTestCase):
def port_maker(self, platform):
+ # FIXME: This platform check should no longer be necessary and should be removed as soon as possible.
if platform != 'darwin':
return None
return MacPort
@@ -141,6 +143,13 @@
self._assert_search_path(['mac-wk2', 'mac-leopard', 'mac-snowleopard', 'mac'], 'leopard', use_webkit2=True)
self._assert_search_path(['mac-wk2', 'mac-snowleopard', 'mac'], 'snowleopard', use_webkit2=True)
+ def test_show_results_html_file(self):
+ port = MacPort(filesystem=MockFileSystem(), user=MockUser(), executive=MockExecutive())
+ # Delay setting a should_log executive to avoid logging from MacPort.__init__.
+ port._executive = MockExecutive(should_log=True)
+ expected_stderr = "MOCK run_command: ['Tools/Scripts/run-safari', '--release', '-NSOpen', 'test.html']\n"
+ OutputCapture().assert_outputs(self, port.show_results_html_file, ["test.html"], expected_stderr=expected_stderr)
+
if __name__ == '__main__':
port_testcase.main()
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/qt.py (90825 => 90826)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/qt.py 2011-07-12 17:51:16 UTC (rev 90825)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/qt.py 2011-07-12 17:52:54 UTC (rev 90826)
@@ -42,6 +42,9 @@
class QtPort(WebKitPort):
port_name = "qt"
+ def _port_flag_for_scripts(self):
+ return "--qt"
+
def _operating_system_for_platform(self, platform):
if platform.startswith('linux'):
return "linux"
@@ -83,3 +86,11 @@
env = WebKitPort.setup_environ_for_server(self)
env['QTWEBKIT_PLUGIN_PATH'] = self._build_path('lib/plugins')
return env
+
+ # FIXME: We should find a way to share this implmentation with Gtk,
+ # or teach run-launcher how to call run-safari and move this down to WebKitPort.
+ def show_results_html_file(self, results_filename):
+ run_launcher_args = ["file://%s" % results_filename]
+ if self.get_option('webkit_test_runner'):
+ run_launcher_args.append('--webkit-test-runner')
+ self._run_script("run-launcher", run_launcher_args)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/qt_unittest.py (90825 => 90826)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/qt_unittest.py 2011-07-12 17:51:16 UTC (rev 90825)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/qt_unittest.py 2011-07-12 17:52:54 UTC (rev 90826)
@@ -29,11 +29,16 @@
import unittest
from webkitpy.common.system.filesystem_mock import MockFileSystem
+from webkitpy.common.system.outputcapture import OutputCapture
from webkitpy.layout_tests.port.qt import QtPort
+from webkitpy.layout_tests.port import port_testcase
from webkitpy.tool.mocktool import MockOptions, MockUser, MockExecutive
-class QtPortTest(unittest.TestCase):
+class QtPortTest(port_testcase.PortTestCase):
+ def port_maker(self, platform):
+ return QtPort
+
def _assert_search_path(self, search_paths, sys_platform, use_webkit2=False):
# FIXME: Port constructors should not "parse" the port name, but
# rather be passed components (directly or via setters). Once
@@ -56,3 +61,9 @@
self._assert_search_path(['qt-wk2', 'qt-mac', 'qt'], 'darwin', use_webkit2=True)
self._assert_search_path(['qt-wk2', 'qt-win', 'qt'], 'cygwin', use_webkit2=True)
self._assert_search_path(['qt-wk2', 'qt-linux', 'qt'], 'linux2', use_webkit2=True)
+
+ def test_show_results_html_file(self):
+ port = self.make_port()
+ port._executive = MockExecutive(should_log=True)
+ expected_stderr = "MOCK run_command: ['Tools/Scripts/run-launcher', '--release', '--qt', 'file://test.html']\n"
+ OutputCapture().assert_outputs(self, port.show_results_html_file, ["test.html"], expected_stderr=expected_stderr)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py (90825 => 90826)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py 2011-07-12 17:51:16 UTC (rev 90825)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py 2011-07-12 17:52:54 UTC (rev 90826)
@@ -93,15 +93,33 @@
return "build-webkittestrunner"
return "build-dumprendertree"
+ def _port_flag_for_scripts(self):
+ # This is overrriden by ports which need a flag passed to scripts to distinguish the use of that port.
+ # For example --qt on linux, since a user might have both Gtk and Qt libraries installed.
+ # FIXME: Chromium should override this once ChromiumPort is a WebKitPort.
+ return None
+
+ # This is modeled after webkitdirs.pm argumentsForConfiguration() from old-run-webkit-tests
+ def _arguments_for_configuration(self):
+ config_args = []
+ config_args.append(self._config.flag_for_configuration(self.get_option('configuration')))
+ # FIXME: We may need to add support for passing --32-bit like old-run-webkit-tests had.
+ port_flag = self._port_flag_for_scripts()
+ if port_flag:
+ config_args.append(port_flag)
+ return config_args
+
+ def _run_script(self, script_name, args=None, include_configuration_arguments=True):
+ run_script_command = [self._config.script_path(script_name)]
+ if include_configuration_arguments:
+ run_script_command.extend(self._arguments_for_configuration())
+ if args:
+ run_script_command.extend(args)
+ return self._executive.run_command(run_script_command, cwd=self._config.webkit_base_dir()) # It's unclear if setting cwd is necessary for all callers.
+
def _build_driver(self):
- configuration = self.get_option('configuration')
try:
- # FIXME: We should probably have a run_script helper which automatically adds the configuration flags.
- self._executive.run_command([
- self._config.script_path(self._driver_build_script_name()),
- self._config.flag_for_configuration(configuration)],
- # FIXME: It's unclear if this cwd= is necessary. Again a helper should do this for us.
- cwd=self._config.webkit_base_dir())
+ self._run_script(self._driver_build_script_name())
except ScriptError:
_log.error("Failed to build %s" % self.driver_name())
return False
