Bug 141718 - [iOS] run-webkit-tests should check that simctl can boot and shutdown simulator device before running tests
Summary: [iOS] run-webkit-tests should check that simctl can boot and shutdown simulat...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: iPhone / iPad iOS 8.1
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords:
Depends on: 141711
Blocks: 141815
  Show dependency treegraph
 
Reported: 2015-02-17 12:12 PST by Daniel Bates
Modified: 2015-08-10 14:11 PDT (History)
4 users (show)

See Also:


Attachments
Patch (4.10 KB, patch)
2015-02-17 12:21 PST, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2015-02-17 12:12:31 PST
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.
Comment 1 Daniel Bates 2015-02-17 12:21:13 PST
Created attachment 246759 [details]
Patch
Comment 2 Alex Christensen 2015-02-17 12:55:06 PST
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  <dabates@apple.com>

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.
Comment 3 Daniel Bates 2015-02-17 14:18:48 PST
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.
Comment 4 Daniel Bates 2015-02-17 14:27:38 PST
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
Comment 5 Daniel Bates 2015-02-17 14:30:15 PST
Comment on attachment 246759 [details]
Patch

Clearing flags on attachment: 246759

Committed r180239: <http://trac.webkit.org/changeset/180239>
Comment 6 Daniel Bates 2015-02-17 14:30:19 PST
All reviewed patches have been landed.  Closing bug.