Hotfix/simulators not found #17
Conversation
ehanlon
left a comment
ehanlon
left a comment
There was a problem hiding this comment.
It works! Thanks for the fix!
libraries/xcode.rb
Outdated
| if highest.nil? | ||
| Chef::Application.fatal!("iOS #{major_version} Simulator no longer available from Apple!") | ||
| if available_list.empty? | ||
| Chef::Log.error('iOS simulator list not populated yet, refreshing...') |
There was a problem hiding this comment.
This feels more like a #warning as opposed to an #error, but I'm not totally familiar with implementing the Chef::Log class. What's the difference/reasoning behind error over warning?
There was a problem hiding this comment.
Or even just an info message. Interesting case though: What happens if the list never populates?
There was a problem hiding this comment.
I tried using info and warn levels - but Kitchen wouldn't print them even when I set the log level envionment var. There was a kitchen PR to address this: test-kitchen/test-kitchen#950, but it still doesn't work for me on latest.
There was a problem hiding this comment.
IMO I don't really need to see it in the output. I could always set the log level in the Kitchen output with -l.
brownmike
left a comment
brownmike
left a comment
There was a problem hiding this comment.
I know this is a hotfix so I won't block the merge on my comments. But I'd like to pair on this a bit this week when we get a chance.
libraries/xcode.rb
Outdated
| if highest.nil? | ||
| Chef::Application.fatal!("iOS #{major_version} Simulator no longer available from Apple!") | ||
| if available_list.empty? | ||
| Chef::Log.error('iOS simulator list not populated yet, refreshing...') |
There was a problem hiding this comment.
Or even just an info message. Interesting case though: What happens if the list never populates?
libraries/xcode.rb
Outdated
| Chef::Application.fatal!("iOS #{major_version} Simulator no longer available from Apple!") | ||
| if available_list.empty? | ||
| Chef::Log.error('iOS simulator list not populated yet, refreshing...') | ||
| shell_out('sleep 20') |
There was a problem hiding this comment.
libraries/xcode.rb
Outdated
| highest = available_list.select { |name, vers| requirement.match?(name, vers) }.max | ||
| if highest.nil? | ||
| Chef::Application.fatal!("iOS #{major_version} Simulator no longer available from Apple!") | ||
| if available_list.empty? |
There was a problem hiding this comment.
This method has accumulated quite a few responsibilities:
- Creating a dependency object
- Checking the state of some (global?) list to see if it's populated yet and sleeping (retrying somehow?) if it isn't
- Creating a highest object by selecting the highest available simulator from the aformentioned list
- nil checking on the previous result so we can fail in a loud and obvious way
- and finally joining the highest object with spaces
I think we have a tortured object in here trying to get out.
americanhanko
left a comment
americanhanko
left a comment
There was a problem hiding this comment.
I think there's quite a bit more work to be done here and thinking about how to implement the native xcode-install gem, but this is a great working refactor for now.
This PR fixes an issue with iOS simulator installation in 10.11 and 10.13, where the xcode-install gem does not populate the simulator list immediately after wrapping up an Xcode install.
The method called by the
install_simulatorsaction will refresh the gem's simulator list output until at least one simulator is scraped from Apple.