simctl launch has a known issue where it will sometimes report a successful install even though the install failed. We need to workaround this issue until it is fixed.
Created attachment 300241 [details] Complete workaround
Comment on attachment 300241 [details] Complete workaround This workaround will recover from any recoverable bug in simctl install, but will take much longer to install an app in the event that any simctl install bug is encountered.
Created attachment 300242 [details] Partial workaround
Comment on attachment 300242 [details] Partial workaround This workaround will work if simctl launch never places the app in the target simulator's directory when it fails without warning. We have no evidence that this is how simctl launch fails. This patch, however, will take less time in the event that simctl launch does fail.
Comment on attachment 300241 [details] Complete workaround View in context: https://bugs.webkit.org/attachment.cgi?id=300241&action=review > Tools/Scripts/webkitpy/xcode/simulator.py:266 > def install_app(self, app_path): A comment pointing to the appropriate bug needs to be added
Comment on attachment 300242 [details] Partial workaround Bugs related to the issue we are working around indicate that an app may exist on simulator but still not entirely be installed. This workaround will still be susceptible to a failure of simctl install
Created attachment 300261 [details] Patch
Comment on attachment 300261 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=300261&action=review I am not happy that we have to use launching of a simulator app as a way to validate whether the install was successful. I wish there was a more direct way to test that installation succeeded. Even better, I wish that "simctl install" only returned a zero status (for success) after successful installing the app. > Tools/Scripts/webkitpy/xcode/simulator.py:267 > + # FIXME: This is a workaround for <rdar://problem/30273973>, Racey failure of simctl launch. The bug title for <rdar://problem/30273973> mentions "simctl launch". Is this the correct bug for this install_app workaround? > Tools/Scripts/webkitpy/xcode/simulator.py:270 > + break I would just return from here instead of breaking out of the loop. > Tools/Scripts/webkitpy/xcode/simulator.py:277 > + bundle_id = self._host.executive.run_command([ > + '/usr/libexec/PlistBuddy', > + '-c', > + 'Print CFBundleIdentifier', > + self._host.filesystem.join(app_path, 'Info.plist'), > + ]).rstrip() This duplicates the logic of the function you added in the patch for bug #165927 (https://trac.webkit.org/changeset/211370), DarwinPort.app_identifier_from_bundle(). > Tools/Scripts/webkitpy/xcode/simulator.py:278 > + pid = self.launch_app(bundle_id, [], attempts=1) I take it you find it variable name pid to be helpful in documenting that launch_app returns a process identifier? I would have inlined the right-hand side of this expression into the line below and avoid the need for the local variable pid. > Tools/Scripts/webkitpy/xcode/simulator.py:294 > + # FIXME: This is a workaround for <rdar://problem/30172453> Very Minor: Missing a period at the end of this sentence.
Comment on attachment 300261 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=300261&action=review >> Tools/Scripts/webkitpy/xcode/simulator.py:267 >> + # FIXME: This is a workaround for <rdar://problem/30273973>, Racey failure of simctl launch. > > The bug title for <rdar://problem/30273973> mentions "simctl launch". Is this the correct bug for this install_app workaround? Right radar, wrong title. Fixed. >> Tools/Scripts/webkitpy/xcode/simulator.py:277 >> + ]).rstrip() > > This duplicates the logic of the function you added in the patch for bug #165927 (https://trac.webkit.org/changeset/211370), DarwinPort.app_identifier_from_bundle(). Yes, it does. I thought this was the best solution, rather than refactoring the entire device class just to pass in a port rather than an executive. Hopefully, this fix is relatively short-lived.
Created attachment 300266 [details] Patch
Comment on attachment 300266 [details] Patch Rejecting attachment 300266 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 300266, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Dan Bates found in /Volumes/Data/EWS/WebKit/Tools/ChangeLog does not appear to be a valid reviewer according to contributors.json. /Volumes/Data/EWS/WebKit/Tools/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://webkit-queues.webkit.org/results/2981688
Created attachment 300274 [details] Patch for landing
Comment on attachment 300274 [details] Patch for landing Clearing flags on attachment: 300274 Committed r211457: <http://trac.webkit.org/changeset/211457>
All reviewed patches have been landed. Closing bug.