141718 2015-02-17 12:12:31 -0800 [iOS] run-webkit-tests should check that simctl can boot and shutdown simulator device before running tests 2015-08-10 14:11:08 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) iPhone / iPad iOS 8.1 RESOLVED FIXED P2 Normal --- 141711 141815 1 dbates dbates achristensen ap ddkilzer jake.nielsen.webkit oldest_to_newest 1069911 0 dbates 2015-02-17 12:12:31 -0800 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. 1069914 1 246759 dbates 2015-02-17 12:21:13 -0800 Created attachment 246759 Patch 1069920 2 246759 achristensen 2015-02-17 12:55:06 -0800 Comment on attachment 246759 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. 1069939 3 246759 dbates 2015-02-17 14:18:48 -0800 Comment on attachment 246759 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. 1069942 4 246759 dbates 2015-02-17 14:27:38 -0800 Comment on attachment 246759 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 1069944 5 246759 dbates 2015-02-17 14:30:15 -0800 Comment on attachment 246759 Patch Clearing flags on attachment: 246759 Committed r180239: <http://trac.webkit.org/changeset/180239> 1069945 6 dbates 2015-02-17 14:30:19 -0800 All reviewed patches have been landed. Closing bug. 246759 2015-02-17 12:21:13 -0800 2015-02-17 14:30:15 -0800 Patch bug-141718-20150217122107.patch text/plain 4194 dbates U3VidmVyc2lvbiBSZXZpc2lvbjogMTgwMTQ2CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggMmIyYWQ3ZTcxY2VlMWFkMTZjMTQ4M2NhYmZkNTI1ZjRl NTgwODc5MC4uYmY4NDQ2MTcyNDBiZjRiZGYzMTZlZDY0MjBiOGJlNTE4Mjg2NzZmYyAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDI2 IEBACiAyMDE1LTAyLTE3ICBEYW5pZWwgQmF0ZXMgIDxkYWJhdGVzQGFwcGxlLmNvbT4KIAorICAg ICAgICBbaU9TXSBydW4td2Via2l0LXRlc3RzIHNob3VsZCBjaGVjayB0aGF0IHNpbWN0bCBjYW4g Ym9vdCBhbmQgc2h1dGRvd24gc2ltdWxhdG9yIGRldmljZSBiZWZvcmUgcnVubmluZyB0ZXN0cwor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTQxNzE4CisK KyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgV2Ugc2hvdWxk IG9ubHkgcnVuIGxheW91dCB0ZXN0cyBpZiBzaW1jdGwgY2FuIHN1Y2Nlc3NmdWxseSBib290IGFu ZCBzaHV0ZG93biB0aGUgdGVzdGluZyBkZXZpY2UuCisKKyAgICAgICAgKiBTY3JpcHRzL3dlYmtp dHB5L3BvcnQvaW9zLnB5OgorICAgICAgICAoSU9TU2ltdWxhdG9yUG9ydC5jaGVja19zeXNfZGVw cyk6IE1vZGlmaWVkIHRvIGNhbGwgU2ltdWxhdG9yLmNoZWNrX3NpbXVsYXRvcl9kZXZpY2VfYW5k X2VyYXNlX2lmX25lZWRlZCgpLgorICAgICAgICBMb2cgYSBlcnJvciBhbmQgcmV0dXJuIEZhbHNl IGlmIHdlIGFyZSB1bmFibGUgdG8gYm9vdCB0aGUgc2ltdWxhdG9yIGRldmljZSBzbyB0aGF0IHRo ZSBjYWxsZXIgY2FuIHRha2UKKyAgICAgICAgYXBwcm9wcmlhdGUgYWN0aW9uLCBzYXkgZXhpdCgz KSBiZWZvcmUgcnVubmluZyBhbnkgbGF5b3V0IHRlc3RzLgorICAgICAgICAqIFNjcmlwdHMvd2Vi a2l0cHkveGNvZGUvc2ltdWxhdG9yLnB5OgorICAgICAgICAoU2ltdWxhdG9yLl9ib290X2FuZF9z aHV0ZG93bl9zaW11bGF0b3JfZGV2aWNlKTogQWRkZWQuIEJvb3QgYW5kIHRoZW4gc2h1dCBkb3du IHRoZSBzaW11bGF0b3IgZGV2aWNlCisgICAgICAgIHdpdGggdGhlIHNwZWNpZmllZCBVRElELgor ICAgICAgICAoU2ltdWxhdG9yLmNoZWNrX3NpbXVsYXRvcl9kZXZpY2VfYW5kX2VyYXNlX2lmX25l ZWRlZCk6IEFkZGVkLiBDaGVja3MgdGhhdCBzaW11bGF0b3IgZGV2aWNlCisgICAgICAgIHdpdGgg dGhlIHNwZWNpZmllZCBVRElEIGNhbiBzdWNjZXNzZnVsbHkgYm9vdCBhbmQgc2h1dCBkb3duLiBX ZSBtYWtlIGF0IG1vc3QgdHdvIGF0dGVtcHRzIHRvCisgICAgICAgIGJvb3QgYW5kIHNodXQgZG93 biB0aGUgZGV2aWNlLCBlcmFzaW5nIHRoZSBkZXZpY2UgYmV0d2VlbiB0cmllcyBzbyBhcyB0byBy ZXN0b3JlIHRoZSBkZXZpY2UKKyAgICAgICAgdG8gYSBrbm93biBnb29kIHN0YXRlLgorCisyMDE1 LTAyLTE3ICBEYW5pZWwgQmF0ZXMgIDxkYWJhdGVzQGFwcGxlLmNvbT4KKwogICAgICAgICBTaW11 bGF0b3Iud2FpdF91bnRpbF9kZXZpY2VfaXNfaW5fc3RhdGUoKSBzaG91bGQgbm90IHdhaXQgaW5k ZWZpbml0ZWx5CiAgICAgICAgIE5lZWQgdGhlIGJ1ZyBVUkwgKE9PUFMhKS4KIApkaWZmIC0tZ2l0 IGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9wb3J0L2lvcy5weSBiL1Rvb2xzL1NjcmlwdHMvd2Vi a2l0cHkvcG9ydC9pb3MucHkKaW5kZXggNGYzOTUzZmYzNTU2NDYzZjA1OWFjMWMzNWQ3MzllMGIw N2U2MWU2My4uZmM1MmFlODAyYjhlMDk5Nzc1NGRjYjEzYjFkNDAzY2Q2ZjA2MTAwNSAxMDA2NDQK LS0tIGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9wb3J0L2lvcy5weQorKysgYi9Ub29scy9TY3Jp cHRzL3dlYmtpdHB5L3BvcnQvaW9zLnB5CkBAIC0yMzEsNiArMjMxLDEwIEBAIGNsYXNzIElPU1Np bXVsYXRvclBvcnQoUG9ydCk6CiAgICAgICAgIGlmIG5vdCBzZWxmLnNpbXVsYXRvcl9ydW50aW1l LmF2YWlsYWJsZToKICAgICAgICAgICAgIF9sb2cuZXJyb3IoJ1RoZSBpT1MgU2ltdWxhdG9yIHJ1 bnRpbWUgd2l0aCBpZGVudGlmaWVyICJ7MH0iIGNhbm5vdCBiZSB1c2VkIGJlY2F1c2UgaXQgaXMg dW5hdmFpbGFibGUuJy5mb3JtYXQoc2VsZi5zaW11bGF0b3JfcnVudGltZS5pZGVudGlmaWVyKSkK ICAgICAgICAgICAgIHJldHVybiBGYWxzZQorICAgICAgICB0ZXN0aW5nX2RldmljZSA9IHNlbGYu dGVzdGluZ19kZXZpY2UgICMgTWF5IGNyZWF0ZSBhIG5ldyBzaW11bGF0b3IgZGV2aWNlCisgICAg ICAgIGlmIG5vdCBTaW11bGF0b3IuY2hlY2tfc2ltdWxhdG9yX2RldmljZV9hbmRfZXJhc2VfaWZf bmVlZGVkKHNlbGYuaG9zdCwgdGVzdGluZ19kZXZpY2UudWRpZCk6CisgICAgICAgICAgICBfbG9n LmVycm9yKCdVbmFibGUgdG8gYm9vdCB0aGUgc2ltdWxhdG9yIGRldmljZSB3aXRoIFVESUQgezB9 LicuZm9ybWF0KHRlc3RpbmdfZGV2aWNlLnVkaWQpKQorICAgICAgICAgICAgcmV0dXJuIEZhbHNl CiAgICAgICAgIHJldHVybiBzdXBlcihJT1NTaW11bGF0b3JQb3J0LCBzZWxmKS5jaGVja19zeXNf ZGVwcyhuZWVkc19odHRwKQogCiAgICAgZGVmIGNoZWNrX2Zvcl9sZWFrcyhzZWxmLCBwcm9jZXNz X25hbWUsIHByb2Nlc3NfcGlkKToKZGlmZiAtLWdpdCBhL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkv eGNvZGUvc2ltdWxhdG9yLnB5IGIvVG9vbHMvU2NyaXB0cy93ZWJraXRweS94Y29kZS9zaW11bGF0 b3IucHkKaW5kZXggZjM3NDUwMTVlMWVlZWVhOTY2MTMyMmQwYzc5MGJjMDNiMzY3MTdiYS4uM2Yw MTJiNmZkNWM0NTRmYWJlYjk5ZDZiODlkOTFmMzNhZmYwNTQ5MiAxMDA2NDQKLS0tIGEvVG9vbHMv U2NyaXB0cy93ZWJraXRweS94Y29kZS9zaW11bGF0b3IucHkKKysrIGIvVG9vbHMvU2NyaXB0cy93 ZWJraXRweS94Y29kZS9zaW11bGF0b3IucHkKQEAgLTI4MCw2ICsyODAsMjQgQEAgY2xhc3MgU2lt dWxhdG9yKG9iamVjdCk6CiAgICAgZGVmIGRldmljZV9kaXJlY3RvcnkodWRpZCk6CiAgICAgICAg IHJldHVybiBvcy5wYXRoLnJlYWxwYXRoKG9zLnBhdGguZXhwYW5kdXNlcihvcy5wYXRoLmpvaW4o J34vTGlicmFyeS9EZXZlbG9wZXIvQ29yZVNpbXVsYXRvci9EZXZpY2VzJywgdWRpZCkpKQogCisg ICAgQHN0YXRpY21ldGhvZAorICAgIGRlZiBfYm9vdF9hbmRfc2h1dGRvd25fc2ltdWxhdG9yX2Rl dmljZShob3N0LCB1ZGlkKToKKyAgICAgICAgZXhpdF9jb2RlID0gaG9zdC5leGVjdXRpdmUucnVu X2NvbW1hbmQoWyd4Y3J1bicsICdzaW1jdGwnLCAnYm9vdCcsIHVkaWRdLCByZXR1cm5fZXhpdF9j b2RlPVRydWUpCisgICAgICAgIGlmIGV4aXRfY29kZToKKyAgICAgICAgICAgIHJldHVybiBleGl0 X2NvZGUKKyAgICAgICAgZXhpdF9jb2RlID0gaG9zdC5leGVjdXRpdmUucnVuX2NvbW1hbmQoWyd4 Y3J1bicsICdzaW1jdGwnLCAnc2h1dGRvd24nLCB1ZGlkXSwgcmV0dXJuX2V4aXRfY29kZT1UcnVl KQorICAgICAgICByZXR1cm4gZXhpdF9jb2RlCisKKyAgICBAc3RhdGljbWV0aG9kCisgICAgZGVm IGNoZWNrX3NpbXVsYXRvcl9kZXZpY2VfYW5kX2VyYXNlX2lmX25lZWRlZChob3N0LCB1ZGlkKToK KyAgICAgICAgZXhpdF9jb2RlID0gU2ltdWxhdG9yLl9ib290X2FuZF9zaHV0ZG93bl9zaW11bGF0 b3JfZGV2aWNlKGhvc3QsIHVkaWQpCisgICAgICAgIGlmIG5vdCBleGl0X2NvZGU6CisgICAgICAg ICAgICByZXR1cm4gVHJ1ZSAgIyBDYW4gYm9vdCBkZXZpY2UKKyAgICAgICAgIyBUcnkgZXJhc2lu ZyB0aGUgc2ltdWxhdG9yIGRldmljZSB0byByZXN0b3JlIGl0IHRvIGEga25vd24gZ29vZCBzdGF0 ZS4KKyAgICAgICAgaWYgbm90IGhvc3QuZXhlY3V0aXZlLnJ1bl9jb21tYW5kKFsneGNydW4nLCAn c2ltY3RsJywgJ2VyYXNlJywgdWRpZF0sIHJldHVybl9leGl0X2NvZGU9VHJ1ZSk6CisgICAgICAg ICAgICByZXR1cm4gU2ltdWxhdG9yLl9ib290X2FuZF9zaHV0ZG93bl9zaW11bGF0b3JfZGV2aWNl KGhvc3QsIHVkaWQpID09IDAgICMgQ2FuIGJvb3QgZGV2aWNlCisgICAgICAgIHJldHVybiBGYWxz ZSAgIyBDYW5ub3QgYm9vdCBvciBlcmFzZSBkZXZpY2UKKwogICAgIGRlZiByZWZyZXNoKHNlbGYp OgogICAgICAgICAiIiIKICAgICAgICAgUmVmcmVzaCBydW50aW1lIGFuZCBkZXZpY2UgdHlwZSBp bmZvcm1hdGlvbiBmcm9tIGBgc2ltY3RsIGxpc3RgYC4K