Bug 174724 - Fail gracefully when xcrun fails in IOSSimulatorPort constructor
Summary: Fail gracefully when xcrun fails in IOSSimulatorPort constructor
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-21 14:13 PDT by Jonathan Bedard
Modified: 2017-07-21 15:20 PDT (History)
6 users (show)

See Also:


Attachments
Patch (1.70 KB, patch)
2017-07-21 14:18 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch for landing (2.01 KB, patch)
2017-07-21 14:42 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2017-07-21 14:13:45 PDT
There is no reason to let an error encountered when trying to detect a running simulator prevent test expectation linting or style checking.  We should ignore such an exception until setting up iOS simulators for testing.
Comment 1 Jonathan Bedard 2017-07-21 14:18:46 PDT
Created attachment 316120 [details]
Patch
Comment 2 Jonathan Bedard 2017-07-21 14:20:06 PDT
<rdar://problem/33446618> tracks this internally.
Comment 3 Aakash Jain 2017-07-21 14:25:57 PDT
Comment on attachment 316120 [details]
Patch

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

> Tools/Scripts/webkitpy/port/ios_simulator.py:82
> +                IOSSimulatorPort._CURRENT_DEVICE = Device(Simulator(host).current_device())

what's the exception? 

If Simulator(host).current_device() is None, then it might be better to check for it being non None, instead of try except.
Comment 4 Jonathan Bedard 2017-07-21 14:29:12 PDT
(In reply to Aakash Jain from comment #3)
> Comment on attachment 316120 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=316120&action=review
> 
> > Tools/Scripts/webkitpy/port/ios_simulator.py:82
> > +                IOSSimulatorPort._CURRENT_DEVICE = Device(Simulator(host).current_device())
> 
> what's the exception? 
> 
> If Simulator(host).current_device() is None, then it might be better to
> check for it being non None, instead of try except.

We had an engineer with a unique personal configuration hit this, 'xcrun simctl' commands would fail, so Simulator(host) would throw a webkitpy.common.system.executive.ScriptError.

If he were running tests on the Simulator, I'd say we let the exception be thrown, but this code is triggered during style checking and test expectation linting.
Comment 5 Aakash Jain 2017-07-21 14:34:29 PDT
Comment on attachment 316120 [details]
Patch

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

> Tools/Scripts/webkitpy/port/ios_simulator.py:83
> +            except:

ok. we should use "except ScriptError:"
Comment 6 Jonathan Bedard 2017-07-21 14:42:35 PDT
Created attachment 316125 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2017-07-21 15:20:41 PDT
Comment on attachment 316125 [details]
Patch for landing

Clearing flags on attachment: 316125

Committed r219747: <http://trac.webkit.org/changeset/219747>
Comment 8 WebKit Commit Bot 2017-07-21 15:20:42 PDT
All reviewed patches have been landed.  Closing bug.