-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add standalone flag for logic tests #5
Conversation
Under Xcode 11 Beta 5 this is required
I'm not sure when |
test_runner/logic_test_util.py
Outdated
'xcrun', 'simctl', 'spawn', sim_id, | ||
|
||
command = ['xcrun', 'simctl', 'spawn'] | ||
if xcode_info_util.GetXcodeVersionNumber() >= 1100: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it need this version check? i.e. would it behave same way on previous xcodes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe not, and I'm not sure what version the --standalone
flag was introduced. If we try to run with --standalone
on a version before the flag was supported, then it might cause an error.
Let me know if you'd like to just remove the version check and I'll revert 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the slack conversation it sounds like you don't need this check because this existed before too
test_runner/logic_test_util.py
Outdated
|
||
command = ['xcrun', 'simctl', 'spawn'] | ||
if xcode_info_util.GetXcodeVersionNumber() >= 1100: | ||
command.extend('--standalone') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
command.extend('--standalone') | |
command.append('--standalone') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will update if we decided to leave the version check
This reverts commit f3d8aae.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing that.
Under Xcode 11 Beta 5 this is required