Attachment 356753 Details for Bug 192470 – Patch

[patch] Patch

bug-192470-20181206141156.patch (text/plain), 9.79 KB, created by Jonathan Bedard on 2018-12-06 14:11:57 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Bedard

Created: 2018-12-06 14:11:57 PST

Size: 9.79 KB

patch

obsolete

>Index: Tools/ChangeLog
>===================================================================
>--- Tools/ChangeLog	(revision 238939)
>+++ Tools/ChangeLog	(working copy)
>@@ -1,3 +1,36 @@
>+2018-12-06  Jonathan Bedard  <jbedard@apple.com>
>+
>+        REGRESSION: run-webkit-tests may fail when using booted simulators
>+        https://bugs.webkit.org/show_bug.cgi?id=192470
>+        <rdar://problem/46532001>
>+
>+        Reviewed by Lucas Forschler.
>+
>+        * Scripts/webkitpy/layout_tests/controllers/manager.py:
>+        (Manager.run): Logging uses the number of child processes to print out information about how efficiently tests
>+        were sharded after the fact. This number is the most meaningful if it is the maximum number of child processes used.
>+        * Scripts/webkitpy/port/base.py:
>+        (Port.max_child_processes): By default, Ports do not support running on any specific kind of device.
>+        * Scripts/webkitpy/port/device_port.py:
>+        (DevicePort.default_child_processes): Rather than using the currently initialized devices as a proxy for how many
>+        child processes are being used, check the device manager every time. Regardless of which devices are attached or
>+        available, iOS cannot boot watchOS devices and vice-versa. dedicated_simulators is not a known argument to
>+        device_count_for_type, use use_booted_simulator instead.
>+        (DevicePort.max_child_processes): Simulators can boot more devices than what is specified by device_count_for_type,
>+        but, if no devices are available, then max_child_processes should return 0 even for simulators.
>+        * Scripts/webkitpy/port/ios_device_unittest.py:
>+        (IOSDeviceTest):
>+        (IOSDeviceTest.test_max_child_processes):
>+        * Scripts/webkitpy/port/ios_simulator_unittest.py:
>+        (IOSSimulatorTest):
>+        (IOSSimulatorTest.test_max_child_processes):
>+        * Scripts/webkitpy/port/port_testcase.py:
>+        (PortTestCase):
>+        (PortTestCase.test_max_child_processes):
>+        * Scripts/webkitpy/port/watch_simulator_unittest.py:
>+        (WatchSimulatorTest):
>+        (WatchSimulatorTest.test_max_child_processes):
>+
> 2018-12-06  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [iOS] WKWebView should match UITextView behavior when editing text with an RTL keyboard
>Index: Tools/Scripts/webkitpy/layout_tests/controllers/manager.py
>===================================================================
>--- Tools/Scripts/webkitpy/layout_tests/controllers/manager.py	(revision 238936)
>+++ Tools/Scripts/webkitpy/layout_tests/controllers/manager.py	(working copy)
>@@ -245,6 +245,7 @@ class Manager(object):
>         retry_results = None
>         enabled_pixel_tests_in_retry = False
> 
>+        max_child_processes_for_run = 1
>         child_processes_option_value = self._options.child_processes
> 
>         while device_type_order:
>@@ -260,6 +261,8 @@ class Manager(object):
>                 _log.info('')
>                 continue
> 
>+            max_child_processes_for_run = max(self._options.child_processes, max_child_processes_for_run)
>+
>             # This loop looks for any less-specific device types which match the current device type
>             index = 0
>             while index < len(device_type_order):
>@@ -281,6 +284,9 @@ class Manager(object):
>             retry_results = retry_results.merge(temp_retry_results) if retry_results else temp_retry_results
>             enabled_pixel_tests_in_retry |= temp_enabled_pixel_tests_in_retry
> 
>+        # Used for final logging, max_child_processes_for_run is most relevant here.
>+        self._options.child_processes = max_child_processes_for_run
>+
>         self._runner.stop_servers()
> 
>         end_time = time.time()
>Index: Tools/Scripts/webkitpy/port/base.py
>===================================================================
>--- Tools/Scripts/webkitpy/port/base.py	(revision 238936)
>+++ Tools/Scripts/webkitpy/port/base.py	(working copy)
>@@ -181,8 +181,10 @@ class Port(object):
>         """Return the number of DumpRenderTree instances to use for this port."""
>         return self._executive.cpu_count()
> 
>-    def max_child_processes(self, **kwargs):
>+    def max_child_processes(self, device_type=None):
>         """Forbid the user from specifying more than this number of child processes"""
>+        if device_type:
>+            return 0
>         return float('inf')
> 
>     def worker_startup_delay_secs(self):
>Index: Tools/Scripts/webkitpy/port/device_port.py
>===================================================================
>--- Tools/Scripts/webkitpy/port/device_port.py	(revision 238936)
>+++ Tools/Scripts/webkitpy/port/device_port.py	(working copy)
>@@ -115,18 +115,25 @@ class DevicePort(DarwinPort):
>     def default_child_processes(self, device_type=None):
>         if not self.DEVICE_MANAGER:
>             raise RuntimeError(self.NO_DEVICE_MANAGER)
>-        if self.DEVICE_MANAGER.INITIALIZED_DEVICES:
>-            return len(self.DEVICE_MANAGER.INITIALIZED_DEVICES)
>+
>+        # FIXME Checking software variant is important for simulators, otherwise an iOS port could boot a watchOS simulator.
>+        # Really, the DEFAULT_DEVICE_TYPE for simulators should be a general instead of specific type, then this code would
>+        # explicitly compare against device_type
>+        device_type = self._device_type_with_version(device_type)
>+        if device_type.software_variant and self.DEFAULT_DEVICE_TYPE.software_variant != device_type.software_variant:
>+            return 0
>+
>         return self.DEVICE_MANAGER.device_count_for_type(
>             self._device_type_with_version(device_type),
>             host=self.host,
>-            dedicated_simulators=not self.get_option('dedicated_simulators', False),
>+            use_booted_simulator=not self.get_option('dedicated_simulators', False),
>         )
> 
>     def max_child_processes(self, device_type=None):
>-        if self.DEVICE_MANAGER == SimulatedDeviceManager:
>-            return super(DevicePort, self).max_child_processes(device_type=device_type)
>-        return self.default_child_processes(device_type=device_type)
>+        result = self.default_child_processes(device_type=device_type)
>+        if result and self.DEVICE_MANAGER == SimulatedDeviceManager:
>+            return super(DevicePort, self).max_child_processes(device_type=None)
>+        return result
> 
>     def setup_test_run(self, device_type=None):
>         if not self.DEVICE_MANAGER:
>Index: Tools/Scripts/webkitpy/port/ios_device_unittest.py
>===================================================================
>--- Tools/Scripts/webkitpy/port/ios_device_unittest.py	(revision 238936)
>+++ Tools/Scripts/webkitpy/port/ios_device_unittest.py	(working copy)
>@@ -112,3 +112,6 @@ class IOSDeviceTest(ios_testcase.IOSTest
>         self.assertEqual(search_path[5], '/mock-checkout/LayoutTests/platform/ios-wk2')
>         self.assertEqual(search_path[6], '/mock-checkout/LayoutTests/platform/ios')
>         self.assertEqual(search_path[7], '/mock-checkout/LayoutTests/platform/wk2')
>+
>+    def test_max_child_processes(self):
>+        pass
>Index: Tools/Scripts/webkitpy/port/ios_simulator_unittest.py
>===================================================================
>--- Tools/Scripts/webkitpy/port/ios_simulator_unittest.py	(revision 238936)
>+++ Tools/Scripts/webkitpy/port/ios_simulator_unittest.py	(working copy)
>@@ -27,6 +27,7 @@ from webkitpy.common.system.outputcaptur
> from webkitpy.common.version import Version
> from webkitpy.tool.mocktool import MockOptions
> from webkitpy.common.system.executive_mock import MockExecutive2, ScriptError
>+from webkitpy.xcode.device_type import DeviceType
> 
> 
> class IOSSimulatorTest(ios_testcase.IOSTest):
>@@ -115,3 +116,7 @@ class IOSSimulatorTest(ios_testcase.IOST
>         self.assertEqual(search_path[5], '/mock-checkout/LayoutTests/platform/ios-wk2')
>         self.assertEqual(search_path[6], '/mock-checkout/LayoutTests/platform/ios')
>         self.assertEqual(search_path[7], '/mock-checkout/LayoutTests/platform/wk2')
>+
>+    def test_max_child_processes(self):
>+        port = self.make_port()
>+        self.assertEqual(port.max_child_processes(DeviceType.from_string('Apple Watch')), 0)
>Index: Tools/Scripts/webkitpy/port/port_testcase.py
>===================================================================
>--- Tools/Scripts/webkitpy/port/port_testcase.py	(revision 238936)
>+++ Tools/Scripts/webkitpy/port/port_testcase.py	(working copy)
>@@ -684,3 +684,8 @@ MOCK output of child process
>     def test_additional_platform_directory(self):
>         port = self.make_port(options=MockOptions(additional_platform_directory=['/tmp/foo']))
>         self.assertEqual(port.baseline_search_path()[0], '/tmp/foo')
>+
>+    def test_max_child_processes(self):
>+        port = self.make_port()
>+        self.assertEqual(port.max_child_processes(True), 0)
>+        self.assertEqual(port.max_child_processes(), float('inf'))
>Index: Tools/Scripts/webkitpy/port/watch_simulator_unittest.py
>===================================================================
>--- Tools/Scripts/webkitpy/port/watch_simulator_unittest.py	(revision 238936)
>+++ Tools/Scripts/webkitpy/port/watch_simulator_unittest.py	(working copy)
>@@ -26,6 +26,7 @@ from webkitpy.common.version import Vers
> from webkitpy.port.watch_simulator import WatchSimulatorPort
> from webkitpy.port import watch_testcase
> from webkitpy.tool.mocktool import MockOptions
>+from webkitpy.xcode.device_type import DeviceType
> 
> 
> class WatchSimulatorTest(watch_testcase.WatchTest):
>@@ -74,3 +75,7 @@ class WatchSimulatorTest(watch_testcase.
>         port._executive = MockExecutive2(run_command_fn=throwing_run_command)
>         expected_stdout = "['xcrun', '--sdk', 'watchsimulator', '-find', 'test']\n"
>         OutputCapture().assert_outputs(self, port.xcrun_find, args=['test', 'falling'], expected_stdout=expected_stdout)
>+
>+    def test_max_child_processes(self):
>+        port = self.make_port()
>+        self.assertEqual(port.max_child_processes(DeviceType.from_string('iPhone')), 0)

Actions: View | Formatted Diff | Diff

Attachments on bug 192470: 356745 | 356753