Title: [133200] trunk/Tools
Revision
133200
Author
pe...@chromium.org
Date
2012-11-01 11:40:29 -0700 (Thu, 01 Nov 2012)

Log Message

[Chromium-Android] Apache doesn't properly clean up ipc semaphores after a layout test run
https://bugs.webkit.org/show_bug.cgi?id=100950

Reviewed by Dirk Pranke.

When a test run would fail to complete due to an exception in one of
the workers, the HTTP server wouldn't get a chance to gracefully shut
down. This caused too much IPC semaphores to be left on the server,
causing Apache to fail to start in subsequent runs.

By unifying the Android-specific code with other ports, we no longer
fail to call the ChromiumPort/Base setup_test_run() and clean_up_test_run()
methods either. Furthermore, the number_of_servers argument for starting
the HTTP server is now available as well.

Because not all tests require an HTTP server, it's not guaranteed that
it will be started. Android depends on this, so add a new method to Port
and override it for Android: requires_http_server().

* Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:
(LayoutTestRunner.run_tests):
* Scripts/webkitpy/layout_tests/controllers/manager.py:
(Manager._run_tests):
* Scripts/webkitpy/layout_tests/port/base.py:
(Port.to.requires_http_server):
* Scripts/webkitpy/layout_tests/port/base_unittest.py:
(PortTest.test_dont_require_http_server):
* Scripts/webkitpy/layout_tests/port/chromium_android.py:
(ChromiumAndroidPort.requires_http_server):
(ChromiumAndroidPort.start_http_server):
* Scripts/webkitpy/layout_tests/port/chromium_android_unittest.py:
(ChromiumAndroidPortTest.test_must_require_http_server):

Modified Paths

Diff

Modified: trunk/Tools/ChangeLog (133199 => 133200)


--- trunk/Tools/ChangeLog	2012-11-01 18:37:23 UTC (rev 133199)
+++ trunk/Tools/ChangeLog	2012-11-01 18:40:29 UTC (rev 133200)
@@ -1,3 +1,38 @@
+2012-11-01  Peter Beverloo  <pe...@chromium.org>
+
+        [Chromium-Android] Apache doesn't properly clean up ipc semaphores after a layout test run
+        https://bugs.webkit.org/show_bug.cgi?id=100950
+
+        Reviewed by Dirk Pranke.
+
+        When a test run would fail to complete due to an exception in one of
+        the workers, the HTTP server wouldn't get a chance to gracefully shut
+        down. This caused too much IPC semaphores to be left on the server,
+        causing Apache to fail to start in subsequent runs.
+
+        By unifying the Android-specific code with other ports, we no longer
+        fail to call the ChromiumPort/Base setup_test_run() and clean_up_test_run()
+        methods either. Furthermore, the number_of_servers argument for starting
+        the HTTP server is now available as well.
+
+        Because not all tests require an HTTP server, it's not guaranteed that
+        it will be started. Android depends on this, so add a new method to Port
+        and override it for Android: requires_http_server().
+
+        * Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:
+        (LayoutTestRunner.run_tests):
+        * Scripts/webkitpy/layout_tests/controllers/manager.py:
+        (Manager._run_tests):
+        * Scripts/webkitpy/layout_tests/port/base.py:
+        (Port.to.requires_http_server):
+        * Scripts/webkitpy/layout_tests/port/base_unittest.py:
+        (PortTest.test_dont_require_http_server):
+        * Scripts/webkitpy/layout_tests/port/chromium_android.py:
+        (ChromiumAndroidPort.requires_http_server):
+        (ChromiumAndroidPort.start_http_server):
+        * Scripts/webkitpy/layout_tests/port/chromium_android_unittest.py:
+        (ChromiumAndroidPortTest.test_must_require_http_server):
+
 2012-11-01  Adam Roben  <aro...@webkit.org>
 
         Crash beneath WKRelease after failed load in MiniBrowser

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py (133199 => 133200)


--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py	2012-11-01 18:37:23 UTC (rev 133199)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py	2012-11-01 18:40:29 UTC (rev 133200)
@@ -126,7 +126,7 @@
 
         all_shards = locked_shards + unlocked_shards
         self._remaining_locked_shards = locked_shards
-        if locked_shards and self._options.http:
+        if self._port.requires_http_server() or (locked_shards and self._options.http):
             self.start_servers_with_lock(2 * min(num_workers, len(locked_shards)))
 
         num_workers = min(num_workers, len(all_shards))
@@ -252,7 +252,7 @@
         index = find(list_name, self._remaining_locked_shards)
         if index >= 0:
             self._remaining_locked_shards.pop(index)
-            if not self._remaining_locked_shards:
+            if not self._remaining_locked_shards and not self._port.requires_http_server():
                 self.stop_servers_with_lock()
 
     def _handle_finished_test(self, worker_name, result, elapsed_time, log_messages=[]):

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py (133199 => 133200)


--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py	2012-11-01 18:37:23 UTC (rev 133199)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py	2012-11-01 18:40:29 UTC (rev 133200)
@@ -459,7 +459,7 @@
 
     def _run_tests(self, tests, result_summary, num_workers):
         test_inputs = [self._test_input_for_file(test) for test in tests]
-        needs_http = any(self._is_http_test(test) for test in tests)
+        needs_http = self._port.requires_http_server() or any(self._is_http_test(test) for test in tests)
         needs_websockets = any(self._is_websocket_test(test) for test in tests)
         return self._runner.run_tests(test_inputs, self._expectations, result_summary, num_workers, needs_http, needs_websockets, self._retrying)
 

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py (133199 => 133200)


--- trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py	2012-11-01 18:37:23 UTC (rev 133199)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py	2012-11-01 18:40:29 UTC (rev 133200)
@@ -899,6 +899,11 @@
         method."""
         pass
 
+    def requires_http_server(self):
+        """Does the port require an HTTP server for running tests? This could
+        be the case when the tests aren't run on the host platform."""
+        return False
+
     def start_http_server(self, additional_dirs=None, number_of_servers=None):
         """Start a web server. Raise an error if it can't start or is already running.
 

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py (133199 => 133200)


--- trunk/Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py	2012-11-01 18:37:23 UTC (rev 133199)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py	2012-11-01 18:40:29 UTC (rev 133200)
@@ -459,5 +459,9 @@
         port = self.make_port(options=optparse.Values({'build_directory': '/my-build-directory/'}))
         self.assertEqual(port._build_path(), '/my-build-directory/Release')
 
+    def test_dont_require_http_server(self):
+        port = self.make_port()
+        self.assertEqual(port.requires_http_server(), False)
+
 if __name__ == '__main__':
     unittest.main()

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py (133199 => 133200)


--- trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py	2012-11-01 18:37:23 UTC (rev 133199)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py	2012-11-01 18:40:29 UTC (rev 133200)
@@ -232,21 +232,17 @@
         android_expectations_file = self.path_from_webkit_base('LayoutTests', 'platform', 'chromium-android', 'TestExpectations')
         return super(ChromiumAndroidPort, self).expectations_files() + [android_expectations_file]
 
+    def requires_http_server(self):
+        """Chromium Android runs tests on devices, and uses the HTTP server to
+        serve the actual layout tests to DumpRenderTree."""
+        return True
+
     def start_http_server(self, additional_dirs=None, number_of_servers=0):
-        # The http server runs during the whole testing period, so ignore this call.
-        pass
+        if not additional_dirs:
+            additional_dirs = {}
+        additional_dirs[TEST_PATH_PREFIX] = self.layout_tests_dir()
+        super(ChromiumAndroidPort, self).start_http_server(additional_dirs, number_of_servers)
 
-    def stop_http_server(self):
-        # Same as start_http_server().
-        pass
-
-    def setup_test_run(self):
-        # Start the HTTP server so that the device can access the test cases.
-        super(ChromiumAndroidPort, self).start_http_server(additional_dirs={TEST_PATH_PREFIX: self.layout_tests_dir()})
-
-    def clean_up_test_run(self):
-        super(ChromiumAndroidPort, self).stop_http_server()
-
     def create_driver(self, worker_number, no_timeout=False):
         # We don't want the default DriverProxy which is not compatible with our driver.
         # See comments in ChromiumAndroidDriver.start().

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_android_unittest.py (133199 => 133200)


--- trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_android_unittest.py	2012-11-01 18:37:23 UTC (rev 133199)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_android_unittest.py	2012-11-01 18:40:29 UTC (rev 133200)
@@ -165,7 +165,11 @@
         self.assertEquals(self.mock_run_command._mock_devices[1], port._get_device_serial(1))
         self.assertRaises(AssertionError, port._get_device_serial, 2)
 
+    def test_must_require_http_server(self):
+        port = self.make_port()
+        self.assertEquals(port.requires_http_server(), True)
 
+
 class ChromiumAndroidDriverTest(unittest.TestCase):
     def setUp(self):
         self.mock_run_command = MockRunCommand()
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to