Use simctl instead of LayoutTestRelay

https://bugs.webkit.org/show_bug.cgi?id=165927

Reviewed by Daniel Bates.

Part 1

LayoutTestRelay uses SPI, since recent versions of the iOS SDK allow for installing apps on
simulators through simctl (iOS 10 and later), use this functionality instead.

* Scripts/webkitpy/port/base.py:
(Port.__init__): Added _test_runner_process_constructor.
* Scripts/webkitpy/port/darwin.py:
(DarwinPort.app_identifier_from_bundle): Added function to extract bundle ID from plist.
* Scripts/webkitpy/port/driver.py:
(Driver._start): Pass worker_number to server_process so we can look up the correct simulator device to use.
(IOSSimulatorDriver): Deleted.
* Scripts/webkitpy/port/driver_unittest.py:
(DriverTest.test_stop_cleans_up_properly): Set _test_runner_process_constructor for testing.
(DriverTest.test_two_starts_cleans_up_properly): Ditto.
(DriverTest.test_start_actually_starts): Ditto.
* Scripts/webkitpy/port/ios.py:
(IOSSimulatorPort): Remove relay_name.
(IOSSimulatorPort.__init__): Set _test_runner_process_constructor to SimulatorProcess for IOSSimulatorPort.
(IOSSimulatorPort._create_simulators): Formatting change.
(IOSSimulatorPort.relay_path): Deleted.
(IOSSimulatorPort._check_relay): Deleted.
(IOSSimulatorPort._check_port_build): Deleted. Use base class implementation
(IOSSimulatorPort._build_relay): Deleted.
(IOSSimulatorPort._build_driver): Deleted. Use base class implementation
(IOSSimulatorPort._driver_class): Deleted. Use base class implementation
* Scripts/webkitpy/port/ios_unittest.py:
(iosTest.test_32bit): Update test.
(iosTest.test_64bit): Update test.
* Scripts/webkitpy/port/server_process.py:
(ServerProcess.__init__): Added argument worker_number. This class does not make use of it. We will make use of this argument in SimulatorProcess to lookup the associated simulator device.
(ServerProcess._set_file_nonblocking): Added to share common code.
* Scripts/webkitpy/port/server_process_mock.py:
(MockServerProcess.__init__): Added argument worker_number.
* Scripts/webkitpy/port/simulator_process.py: Added.
(SimulatorProcess): Added.
(SimulatorProcess.Popen): Added.
(SimulatorProcess.Popen.__init__): Added. Initialize Popen structure with stdin, stdout, stderr and pid.
(SimulatorProcess.Popen.poll): Added. Check if the process is running.
(SimulatorProcess.Popen.wait): Added. Wait for process to close.
(SimulatorProcess.__init__): Added. Install app to device specified through port and worker_number.
(SimulatorProcess._reset): Added. Unlink fifos.
(SimulatorProcess._start): Added. Launch app on simulator, link fifos.
(SimulatorProcess._kill): Added. Shutdown app on simulator.
* Scripts/webkitpy/xcode/simulator.py:
(Device.__init__): Accept host to run install/launch/terminate.
(Device.install_app): Install app to target Device.
(Device.launch_app): Launch app on target Device.
(Device.terminate_app): Shutdown app on target Device.
(Simulator._parse_devices): Pass host to Device.


Canonical link: https://commits.webkit.org/184614@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@211370 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog

@@ -1,3 +1,61 @@
+2017-01-30  Jonathan Bedard  <jbedard@apple.com>
+
+        Use simctl instead of LayoutTestRelay
+        https://bugs.webkit.org/show_bug.cgi?id=165927
+
+        Reviewed by Daniel Bates.
+
+        Part 1
+
+        LayoutTestRelay uses SPI, since recent versions of the iOS SDK allow for installing apps on
+        simulators through simctl (iOS 10 and later), use this functionality instead.
+
+        * Scripts/webkitpy/port/base.py:
+        (Port.__init__): Added _test_runner_process_constructor.
+        * Scripts/webkitpy/port/darwin.py:
+        (DarwinPort.app_identifier_from_bundle): Added function to extract bundle ID from plist.
+        * Scripts/webkitpy/port/driver.py:
+        (Driver._start): Pass worker_number to server_process so we can look up the correct simulator device to use.
+        (IOSSimulatorDriver): Deleted.
+        * Scripts/webkitpy/port/driver_unittest.py:
+        (DriverTest.test_stop_cleans_up_properly): Set _test_runner_process_constructor for testing.
+        (DriverTest.test_two_starts_cleans_up_properly): Ditto.
+        (DriverTest.test_start_actually_starts): Ditto.
+        * Scripts/webkitpy/port/ios.py:
+        (IOSSimulatorPort): Remove relay_name.
+        (IOSSimulatorPort.__init__): Set _test_runner_process_constructor to SimulatorProcess for IOSSimulatorPort.
+        (IOSSimulatorPort._create_simulators): Formatting change.
+        (IOSSimulatorPort.relay_path): Deleted.
+        (IOSSimulatorPort._check_relay): Deleted.
+        (IOSSimulatorPort._check_port_build): Deleted. Use base class implementation
+        (IOSSimulatorPort._build_relay): Deleted.
+        (IOSSimulatorPort._build_driver): Deleted. Use base class implementation
+        (IOSSimulatorPort._driver_class): Deleted. Use base class implementation
+        * Scripts/webkitpy/port/ios_unittest.py:
+        (iosTest.test_32bit): Update test.
+        (iosTest.test_64bit): Update test.
+        * Scripts/webkitpy/port/server_process.py:
+        (ServerProcess.__init__): Added argument worker_number. This class does not make use of it. We will make use of this argument in SimulatorProcess to lookup the associated simulator device.
+        (ServerProcess._set_file_nonblocking): Added to share common code.
+        * Scripts/webkitpy/port/server_process_mock.py:
+        (MockServerProcess.__init__): Added argument worker_number.
+        * Scripts/webkitpy/port/simulator_process.py: Added.
+        (SimulatorProcess): Added.
+        (SimulatorProcess.Popen): Added.
+        (SimulatorProcess.Popen.__init__): Added. Initialize Popen structure with stdin, stdout, stderr and pid.
+        (SimulatorProcess.Popen.poll): Added. Check if the process is running.
+        (SimulatorProcess.Popen.wait): Added. Wait for process to close.
+        (SimulatorProcess.__init__): Added. Install app to device specified through port and worker_number.
+        (SimulatorProcess._reset): Added. Unlink fifos.
+        (SimulatorProcess._start): Added. Launch app on simulator, link fifos.
+        (SimulatorProcess._kill): Added. Shutdown app on simulator.
+        * Scripts/webkitpy/xcode/simulator.py:
+        (Device.__init__): Accept host to run install/launch/terminate.
+        (Device.install_app): Install app to target Device.
+        (Device.launch_app): Launch app on target Device.
+        (Device.terminate_app): Shutdown app on target Device.
+        (Simulator._parse_devices): Pass host to Device.
+
 2017-01-30  Carlos Alberto Lopez Perez  <clopez@igalia.com>
 
         [GTK] pixman fails to compile on Raspberry Pi (GCC crash)

Tools/Scripts/webkitpy/port/base.py

@@ -131,6 +131,7 @@ def __init__(self, host, port_name, options=None, **kwargs):
         self._web_platform_test_server = None
         self._image_differ = None
         self._server_process_constructor = server_process.ServerProcess  # overridable for testing
+        self._test_runner_process_constructor = server_process.ServerProcess
 
         if not hasattr(options, 'configuration') or not options.configuration:
             self.set_option_default('configuration', self.default_configuration())

Tools/Scripts/webkitpy/port/darwin.py

@@ -174,3 +174,11 @@ def xcrun_find(self, command, fallback=None):
         except ScriptError:
             _log.warn("xcrun failed; falling back to '%s'." % fallback)
             return fallback
+
+    def app_identifier_from_bundle(self, app_bundle):
+        plist_path = self._filesystem.join(app_bundle, 'Info.plist')
+        if not self._filesystem.exists(plist_path):
+            plist_path = self._filesystem.join(app_bundle, 'Contents', 'Info.plist')
+        if not self._filesystem.exists(plist_path):
+            return None
+        return self._executive.run_command(['/usr/libexec/PlistBuddy', '-c', 'Print CFBundleIdentifier', plist_path]).rstrip()

Tools/Scripts/webkitpy/port/driver.py

@@ -138,7 +138,7 @@ def __init__(self, port, worker_number, pixel_tests, no_timeout=False):
         self._driver_user_directory_suffix = None
         self._driver_user_cache_directory = None
 
-        # WebKitTestRunner/LayoutTestRelay can report back subprocess crashes by printing
+        # WebKitTestRunner can report back subprocess crashes by printing
         # "#CRASHED - PROCESSNAME".  Since those can happen at any time and ServerProcess
         # won't be aware of them (since the actual tool didn't crash, just a subprocess)
         # we record the crashed subprocess name here.
@@ -327,7 +327,6 @@ def _append_environment_variable_path(self, environment, variable, path):
 
     def _setup_environ_for_driver(self, environment):
         build_root_path = str(self._port._build_path())
-        # FIXME: DYLD_* variables should be Mac-only. Even iOS Simulator doesn't need them, as LayoutTestRelay is a host binary.
         self._append_environment_variable_path(environment, 'DYLD_LIBRARY_PATH', build_root_path)
         self._append_environment_variable_path(environment, '__XPC_DYLD_LIBRARY_PATH', build_root_path)
         self._append_environment_variable_path(environment, 'DYLD_FRAMEWORK_PATH', build_root_path)
@@ -369,7 +368,7 @@ def _start(self, pixel_tests, per_test_args):
         environment = self._setup_environ_for_test()
         self._crashed_process_name = None
         self._crashed_pid = None
-        self._server_process = self._port._server_process_constructor(self._port, self._server_name, self.cmd_line(pixel_tests, per_test_args), environment)
+        self._server_process = self._port._test_runner_process_constructor(self._port, self._server_name, self.cmd_line(pixel_tests, per_test_args), environment, worker_number=self._worker_number)
         self._server_process.start()
 
     def _run_post_start_tasks(self):
@@ -611,27 +610,6 @@ def check_driver(port):
         return True
 
 
-# FIXME: this should be abstracted out via the Port subclass somehow.
-class IOSSimulatorDriver(Driver):
-    def cmd_line(self, pixel_tests, per_test_args):
-        cmd = super(IOSSimulatorDriver, self).cmd_line(pixel_tests, per_test_args)
-        relay_tool = self._port.relay_path
-        dump_tool = cmd[0]
-        dump_tool_args = cmd[1:]
-        product_dir = self._port._build_path()
-        relay_args = [
-            '-developerDir', self._port.developer_dir,
-            '-udid', self._port.device_id_for_worker_number(self._worker_number),
-            '-productDir', product_dir,
-            '-app', dump_tool,
-        ]
-        return [relay_tool] + relay_args + ['--'] + dump_tool_args
-
-    def _setup_environ_for_driver(self, environment):
-        environment['DEVELOPER_DIR'] = self._port.developer_dir
-        return super(IOSSimulatorDriver, self)._setup_environ_for_driver(environment)
-
-
 class ContentBlock(object):
     def __init__(self):
         self.content_type = None

Tools/Scripts/webkitpy/port/driver_unittest.py

@@ -264,7 +264,7 @@ def test_creating_a_port_does_not_write_to_the_filesystem(self):
 
     def test_stop_cleans_up_properly(self):
         port = TestWebKitPort()
-        port._server_process_constructor = MockServerProcess
+        port._test_runner_process_constructor = MockServerProcess
         driver = Driver(port, 0, pixel_tests=True)
         driver.start(True, [])
         last_tmpdir = port._filesystem.last_tmpdir
@@ -274,7 +274,7 @@ def test_stop_cleans_up_properly(self):
 
     def test_two_starts_cleans_up_properly(self):
         port = TestWebKitPort()
-        port._server_process_constructor = MockServerProcess
+        port._test_runner_process_constructor = MockServerProcess
         driver = Driver(port, 0, pixel_tests=True)
         driver.start(True, [])
         last_tmpdir = port._filesystem.last_tmpdir
@@ -283,7 +283,7 @@ def test_two_starts_cleans_up_properly(self):
 
     def test_start_actually_starts(self):
         port = TestWebKitPort()
-        port._server_process_constructor = MockServerProcess
+        port._test_runner_process_constructor = MockServerProcess
         driver = Driver(port, 0, pixel_tests=True)
         driver.start(True, [])
         self.assertTrue(driver._server_process.started)

Tools/Scripts/webkitpy/port/ios.py

@@ -34,6 +34,7 @@
 from webkitpy.port import config as port_config
 from webkitpy.port import driver, image_diff
 from webkitpy.port.darwin import DarwinPort
+from webkitpy.port.simulator_process import SimulatorProcess
 from webkitpy.xcode.simulator import Simulator, Runtime, DeviceType
 from webkitpy.common.system.crashlogs import CrashLogs
 
@@ -78,7 +79,6 @@ class IOSSimulatorPort(DarwinPort):
     SDK = 'iphonesimulator'
 
     SIMULATOR_BUNDLE_ID = 'com.apple.iphonesimulator'
-    relay_name = 'LayoutTestRelay'
     SIMULATOR_DIRECTORY = "/tmp/WebKitTestingSimulators/"
     LSREGISTER_PATH = "/System/Library/Frameworks/CoreServices.framework/Frameworks/LaunchServices.framework/Versions/Current/Support/lsregister"
     PROCESS_COUNT_ESTIMATE_PER_SIMULATOR_INSTANCE = 100
@@ -97,6 +97,7 @@ class IOSSimulatorPort(DarwinPort):
 
     def __init__(self, host, port_name, **kwargs):
         DarwinPort.__init__(self, host, port_name, **kwargs)
+        self._test_runner_process_constructor = SimulatorProcess
 
         optional_device_class = self.get_option('device_class')
         self._printing_cmd_line = False
@@ -148,16 +149,6 @@ def simulator_device_type(self):
             device_type = DeviceType.from_name(device_name)
         return device_type
 
-    @property
-    @memoized
-    def relay_path(self):
-        if self._root_was_set:
-            path = self._filesystem.abspath(self.get_option('root'))
-        else:
-            mac_config = port_config.Config(self._executive, self._filesystem, 'mac')
-            path = mac_config.build_directory(self.get_option('configuration'))
-        return self._filesystem.join(path, self.relay_name)
-
     @memoized
     def child_processes(self):
         return int(self.get_option('child_processes'))
@@ -182,19 +173,6 @@ def default_child_processes(self):
 
         return min(maximum_simulator_count_on_this_system, best_child_process_count_for_cpu)
 
-    def _check_relay(self):
-        if not self._filesystem.exists(self.relay_path):
-            _log.error("%s was not found at %s" % (self.relay_name, self.relay_path))
-            return False
-        return True
-
-    def _check_port_build(self):
-        if not self._root_was_set and self.get_option('build') and not self._build_relay():
-            return False
-        if not self._check_relay():
-            return False
-        return True
-
     def _get_crash_log(self, name, pid, stdout, stderr, newer_than, time_fn=time.time, sleep_fn=time.sleep, wait_for_log=True):
         time_fn = time_fn or time.time
         sleep_fn = sleep_fn or time.sleep
@@ -215,7 +193,7 @@ def _get_crash_log(self, name, pid, stdout, stderr, newer_than, time_fn=time.tim
             return self._get_crash_log(crashed_subprocess_name_and_pid[0], crashed_subprocess_name_and_pid[1], stdout,
                 '\n'.join(stderr_lines), newer_than, time_fn, sleep_fn, wait_for_log)
 
-        # LayoutTestRelay crashed
+        # App crashed
         _log.debug('looking for crash log for %s:%s' % (name, str(pid)))
         crash_log = ''
         crash_logs = CrashLogs(self.host)
@@ -233,25 +211,6 @@ def _get_crash_log(self, name, pid, stdout, stderr, newer_than, time_fn=time.tim
             return stderr, None
         return stderr, crash_log
 
-    def _build_relay(self):
-        environment = self.host.copy_current_environment()
-        environment.disable_gcc_smartquotes()
-        env = environment.to_dictionary()
-
-        try:
-            # FIXME: We should be passing _arguments_for_configuration(), which respects build configuration and port,
-            # instead of hardcoding --ios-simulator.
-            self._run_script("build-layouttestrelay", args=["--ios-simulator"], env=env)
-        except ScriptError, e:
-            _log.error(e.message_with_output(output_limit=None))
-            return False
-        return True
-
-    def _build_driver(self):
-        built_tool = super(IOSSimulatorPort, self)._build_driver()
-        built_relay = self._build_relay()
-        return built_tool and built_relay
-
     def _build_driver_flags(self):
         archs = ['ARCHS=i386'] if self.architecture() == 'x86' else []
         sdk = ['--sdk', 'iphonesimulator']
@@ -264,9 +223,6 @@ def _generate_all_test_configurations(self):
                 configurations.append(TestConfiguration(version=self._version, architecture=architecture, build_type=build_type))
         return configurations
 
-    def _driver_class(self):
-        return driver.IOSSimulatorDriver
-
     def default_baseline_search_path(self):
         if self.get_option('webkit_test_runner'):
             fallback_names = [self._wk2_port_name()] + [self.port_name] + ['wk2']
@@ -280,9 +236,9 @@ def _set_device_class(self, device_class):
 
     def _create_simulators(self):
         if (self.default_child_processes() < self.child_processes()):
-                _log.warn("You have specified very high value({0}) for --child-processes".format(self.child_processes()))
-                _log.warn("maximum child-processes which can be supported on this system are: {0}".format(self.default_child_processes()))
-                _log.warn("This is very likely to fail.")
+            _log.warn('You have specified very high value({0}) for --child-processes'.format(self.child_processes()))
+            _log.warn('maximum child-processes which can be supported on this system are: {0}'.format(self.default_child_processes()))
+            _log.warn('This is very likely to fail.')
 
         if self._using_dedicated_simulators():
             self._createSimulatorApps()
@@ -413,7 +369,7 @@ def _remove_device(self, number):
     def _testing_device(self, number):
         return Simulator.device_number(number)
 
-    # This is only exposed so that IOSSimulatorDriver can use it.
+    # FIXME: This is only exposed so that SimulatorProcess can use it.
     def device_id_for_worker_number(self, number):
         if self._printing_cmd_line:
             return '<dummy id>'

Tools/Scripts/webkitpy/port/ios_unittest.py

@@ -70,7 +70,7 @@ def run_script(script, args=None, env=None):
         port._run_script = run_script
         self.assertEqual(port.architecture(), 'x86')
         port._build_driver()
-        self.assertEqual(self.args, ['--ios-simulator'])
+        self.assertEqual(self.args, ['ARCHS=i386', '--sdk', 'iphonesimulator'])
 
     def test_64bit(self):
         # Apple Mac port is 64-bit by default
@@ -82,7 +82,7 @@ def run_script(script, args=None, env=None):
 
         port._run_script = run_script
         port._build_driver()
-        self.assertEqual(self.args, ['--ios-simulator'])
+        self.assertEqual(self.args, ['--sdk', 'iphonesimulator'])
 
     def test_sdk_name(self):
         port = self.make_port()

Tools/Scripts/webkitpy/port/server_process.py

@@ -1,3 +1,4 @@
+# Copyright (C) 2017 Apple Inc. All rights reserved.
 # Copyright (C) 2010 Google Inc. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
@@ -59,7 +60,7 @@ class ServerProcess(object):
     indefinitely. The class also handles transparently restarting processes
     as necessary to keep issuing commands."""
 
-    def __init__(self, port_obj, name, cmd, env=None, universal_newlines=False, treat_no_data_as_crash=False):
+    def __init__(self, port_obj, name, cmd, env=None, universal_newlines=False, treat_no_data_as_crash=False, worker_number=None):
         self._port = port_obj
         self._name = name  # Should be the command name (e.g. DumpRenderTree, ImageDiff)
         self._cmd = cmd
@@ -103,6 +104,11 @@ def _reset(self):
     def process_name(self):
         return self._name
 
+    @staticmethod
+    def _set_file_nonblocking(file):
+        flags = fcntl.fcntl(file.fileno(), fcntl.F_GETFL)
+        fcntl.fcntl(file.fileno(), fcntl.F_SETFL, flags | os.O_NONBLOCK)
+
     def _start(self):
         if self._proc:
             raise ValueError("%s already running" % self._name)
@@ -117,13 +123,9 @@ def _start(self):
             universal_newlines=self._universal_newlines)
         self._pid = self._proc.pid
         self._port.find_system_pid(self.name(), self._pid)
-        fd = self._proc.stdout.fileno()
         if not self._use_win32_apis:
-            fl = fcntl.fcntl(fd, fcntl.F_GETFL)
-            fcntl.fcntl(fd, fcntl.F_SETFL, fl | os.O_NONBLOCK)
-            fd = self._proc.stderr.fileno()
-            fl = fcntl.fcntl(fd, fcntl.F_GETFL)
-            fcntl.fcntl(fd, fcntl.F_SETFL, fl | os.O_NONBLOCK)
+            self._set_file_nonblocking(self._proc.stdout)
+            self._set_file_nonblocking(self._proc.stderr)
 
     def _handle_possible_interrupt(self):
         """This routine checks to see if the process crashed or exited

Tools/Scripts/webkitpy/port/server_process_mock.py

@@ -28,7 +28,7 @@
 
 
 class MockServerProcess(object):
-    def __init__(self, port_obj=None, name=None, cmd=None, env=None, universal_newlines=False, lines=None, crashed=False):
+    def __init__(self, port_obj=None, name=None, cmd=None, env=None, universal_newlines=False, lines=None, crashed=False, worker_number=None):
         self.timed_out = False
         self.lines = lines or []
         self.crashed = crashed