Summary: | [iOS] run-webkit-tests should check that simctl can boot and shutdown simulator device before running tests | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||
Component: | Tools / Tests | Assignee: | Daniel Bates <dbates> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | achristensen, ap, ddkilzer, jake.nielsen.webkit | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | iPhone / iPad | ||||||
OS: | iOS 8.1 | ||||||
Bug Depends on: | 141711 | ||||||
Bug Blocks: | 141815 | ||||||
Attachments: |
|
Description
Daniel Bates
2015-02-17 12:12:31 PST
Created attachment 246759 [details]
Patch
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 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 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 on attachment 246759 [details] Patch Clearing flags on attachment: 246759 Committed r180239: <http://trac.webkit.org/changeset/180239> All reviewed patches have been landed. Closing bug. |