[webkit-reviews] review denied: [Bug 185315] webkitpy: Check for com.apple.datamigrator before declaring simulators booted : [Attachment 339577] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has denied Jonathan Bedard
<jbedard at apple.com>'s request for review:
Bug 185315: webkitpy: Check for com.apple.datamigrator before declaring
simulators booted
https://bugs.webkit.org/show_bug.cgi?id=185315

Attachment 339577: Patch

https://bugs.webkit.org/attachment.cgi?id=339577&action=review




--- Comment #4 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 339577
  --> https://bugs.webkit.org/attachment.cgi?id=339577
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=339577&action=review

> Tools/ChangeLog:3
> +	   webkitpy: Check for com.apple.datamigrator before declaring
simulators booted

We should update webkitdirs::runIOSWebKitAppInSimulator()
<https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitdirs.pm?rev=231302#L
2595> and any other tools/scripts that need to know when the simulator is ready
to be used.

> Tools/Scripts/webkitpy/common/system/executive_mock.py:181
> +	   self._running_pids = {'test-webkitpy': os.getpid()}

Is it necessary to call os.getpid()? Can we just hardcode a dummy canonical pid
and avoid the cost of calling os.getpid()?

> Tools/Scripts/webkitpy/xcode/simulated_device.py:368
> +	   SimulatedDeviceManager.wait_until_data_migration_is_done(host,
deadline - time.time())

deadline - time.time() can be negative. What does that mean?

> Tools/Scripts/webkitpy/xcode/simulated_device.py:370
>	   return SimulatedDeviceManager.INITIALIZED_DEVICES

I know you did not touch this code in this patch, but this function as written
assumes that all devices in SimulatedDeviceManager.INITIALIZED_DEVICES have
been booted by the time we get to this line even though
SimulatedDeviceManager._wait_until_device_in_state() may have timed out.

> Tools/Scripts/webkitpy/xcode/simulated_device.py:424
> +	   deadline = time.time() + timeout

This function assumes timeout > 0.

> Tools/Scripts/webkitpy/xcode/simulated_device.py:427
> +	       time.sleep(1)

The placement of this sleep() means that this function will wait 1 second
regardless of the value of timeout.