Bug 141718 – [iOS] run-webkit-tests should check that simctl can boot and shutdown simulator device before running tests

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Daniel Bates]

We should ensure that simctl can boot and shutdown the testing device before running layout tests to avoid unnecessarily spending time running tests with a simulator device or iOS Simulator that is in a bad state.

[Daniel Bates]

Created attachment 246759 [details]
Patch

[Alex Christensen]

Comment on attachment 246759 [details]
Patch

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

Looks good.  Minor change suggestions and questions:

> Tools/ChangeLog:1
>  2015-02-17  Daniel Bates  <[e-mail]>

This patch probably does not apply because the changelog has the name at the bottom.

> Tools/Scripts/webkitpy/xcode/simulator.py:287
> +            return exit_code

Are there ever cases where we would need to clean up anything before returning here, or would a non-zero exit code always mean that it did not start anything?

> Tools/Scripts/webkitpy/xcode/simulator.py:298
> +            return Simulator._boot_and_shutdown_simulator_device(host, udid) == 0  # Can boot device

Before you used not exit_code, here you use == 0. I think this should be consistent.

[Daniel Bates]

Comment on attachment 246759 [details]
Patch

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

>> Tools/Scripts/webkitpy/xcode/simulator.py:287
>> +            return exit_code
> 
> Are there ever cases where we would need to clean up anything before returning here, or would a non-zero exit code always mean that it did not start anything?

A non-zero exit code means that we did not boot the simulator device.

>> Tools/Scripts/webkitpy/xcode/simulator.py:298
>> +            return Simulator._boot_and_shutdown_simulator_device(host, udid) == 0  # Can boot device
> 
> Before you used not exit_code, here you use == 0. I think this should be consistent.

Will use not directive instead of equality testing.

[Daniel Bates]

Comment on attachment 246759 [details]
Patch

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

>>> Tools/Scripts/webkitpy/xcode/simulator.py:298
>>> +            return Simulator._boot_and_shutdown_simulator_device(host, udid) == 0  # Can boot device
>> 
>> Before you used not exit_code, here you use == 0. I think this should be consistent.
> 
> Will use not directive instead of equality testing.

Actually, will land as-is. Notice that this function returns a boolean result and Simulator._boot_and_shutdown_simulator_device() returns an integer. We could write this line as:

return not bool(Simulator._boot_and_shutdown_simulator_device(host, udid))

But this seems less clear than:

return Simulator._boot_and_shutdown_simulator_device(host, udid) == 0

[Daniel Bates]

Comment on attachment 246759 [details]
Patch

Clearing flags on attachment: 246759

Committed r180239: <http://trac.webkit.org/changeset/180239>

[Daniel Bates]

All reviewed patches have been landed.  Closing bug.