Modified: trunk/Tools/ChangeLog (109263 => 109264)
--- trunk/Tools/ChangeLog 2012-02-29 22:35:01 UTC (rev 109263)
+++ trunk/Tools/ChangeLog 2012-02-29 22:40:32 UTC (rev 109264)
@@ -1,3 +1,41 @@
+2012-02-29 Dirk Pranke <dpra...@chromium.org>
+
+ nrwt: support more than two drivers in DriverProxy
+ https://bugs.webkit.org/show_bug.cgi?id=79736
+
+ Reviewed by Adam Barth.
+
+ Now that we can support per-test command lines for
+ Drivers, modify DriverProxy to keep a map of running
+ drivers for each needed command-line; this will allow
+ us to transparently maintain a pool of appropriately
+ configured DRTs without having to constantly start and stop
+ them.
+
+ Note that this potentially raises a garbage collection
+ problem - the number of running DRTs will grow with the
+ number of different sets of command line args. For now
+ this is no worse than the current code - if you're running
+ with pixel tests, you will only need one DRT per worker,
+ and if you aren't, you'll need two (one for text-only tests,
+ and one for reftests).
+
+ An alternative would be to only ever have one running driver,
+ and restart the driver as the command line changes, but this
+ might (?) slow down execution in the text-only case - we
+ should benchmark this because it would be simpler and possibly
+ allow us to eliminate DriverProxy altogether.
+
+ * Scripts/webkitpy/layout_tests/port/driver.py:
+ (DriverProxy.__init__):
+ (DriverProxy):
+ (DriverProxy._make_driver):
+ (DriverProxy.run_test):
+ (DriverProxy.has_crashed):
+ (DriverProxy.stop):
+ (DriverProxy.cmd_line):
+ (DriverProxy._cmd_line_as_key):
+
2012-02-29 Adrienne Walker <e...@google.com>
Unreviewed, add myself as a reviewer
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/driver.py (109263 => 109264)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/driver.py 2012-02-29 22:35:01 UTC (rev 109263)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/driver.py 2012-02-29 22:40:32 UTC (rev 109264)
@@ -172,29 +172,39 @@
single driver."""
def __init__(self, port, worker_number, driver_instance_constructor, pixel_tests, no_timeout):
+ self._port = port
+ self._worker_number = worker_number
+ self._driver_instance_constructor = driver_instance_constructor
+ self._no_timeout = no_timeout
self._pixel_tests = pixel_tests
- self._driver = driver_instance_constructor(port, worker_number, pixel_tests, no_timeout)
- if pixel_tests:
- self._reftest_driver = self._driver
- else:
- self._reftest_driver = driver_instance_constructor(port, worker_number, True, no_timeout)
+ # FIXME: We shouldn't need to create a driver until we actually run a test.
+ self._driver = self._make_driver(pixel_tests)
+ self._running_drivers = {}
+ self._running_drivers[self._cmd_line_as_key(pixel_tests, [])] = self._driver
+
+ def _make_driver(self, pixel_tests):
+ return self._driver_instance_constructor(self._port, self._worker_number, pixel_tests, self._no_timeout)
+
+ # FIXME: this should be a @classmethod (or implemented on Port instead).
def is_http_test(self, test_name):
return self._driver.is_http_test(test_name)
+ # FIXME: this should be a @classmethod (or implemented on Port instead).
def test_to_uri(self, test_name):
return self._driver.test_to_uri(test_name)
+ # FIXME: this should be a @classmethod (or implemented on Port instead).
def uri_to_test(self, uri):
return self._driver.uri_to_test(uri)
def run_test(self, driver_input):
- if driver_input.is_reftest:
- return self._reftest_driver.run_test(driver_input)
- return self._driver.run_test(driver_input)
+ pixel_tests_needed = self._pixel_tests or driver_input.is_reftest
+ cmd_line_key = self._cmd_line_as_key(pixel_tests_needed, [])
+ if not cmd_line_key in self._running_drivers:
+ self._running_drivers[cmd_line_key] = self._make_driver(pixel_tests_needed)
- def has_crashed(self):
- return self._driver.has_crashed() or self._reftest_driver.has_crashed()
+ return self._running_drivers[cmd_line_key].run_test(driver_input)
def start(self):
# FIXME: Callers shouldn't normally call this, since this routine
@@ -206,12 +216,16 @@
# into run_test() directly.
self._driver.start(self._pixel_tests, [])
+ def has_crashed(self):
+ return any(driver.has_crashed() for driver in self._running_drivers.values())
+
def stop(self):
- self._driver.stop()
- self._reftest_driver.stop()
+ for driver in self._running_drivers.values():
+ driver.stop()
- def cmd_line(self, pixel_tests, per_test_args):
- cmd_line = self._driver.cmd_line(pixel_tests, per_test_args)
- if self._driver != self._reftest_driver:
- cmd_line += ['; '] + self._reftest_driver.cmd_line(pixel_tests, per_test_args)
- return cmd_line
+ # FIXME: this should be a @classmethod (or implemented on Port instead).
+ def cmd_line(self, pixel_tests=None, per_test_args=None):
+ return self._driver.cmd_line(pixel_tests or self._pixel_tests, per_test_args or [])
+
+ def _cmd_line_as_key(self, pixel_tests, per_test_args):
+ return ' '.join(self.cmd_line(pixel_tests, per_test_args))