cli
cli copied to clipboard
fix: update logic to check for available ios devices for `log-ios` to work
Summary:
Hey, just ran into this while working on a new feature.
When pulling the devices via xcrun simctl list devices --json
now, I don't actually get the availability
key, I just get the isAvailable
key. They are both already listed as optional keys in the Device
type in the repo. My guess is that the availability
key was on an older version of simctl
.
Anyway, if I run the log-ios
as-is, without this change, it always fails to find an available iOS device because device.availability
simply never exists. Changing it to also check for device.isAvailable
works.
Test Plan:
environment: macOS 12.5 (latest stable) xcrun version 61 / xcode 13.4.1 (latest stable)
- Have an iOS simulator open and running
- Run the
log-ios
command as is in the currentmain
cli branch. (Result: FAILS witherror: No active iOS device found
) - Run the
log-ios
command with this PR's code. (Result: Successfully finds the active simulator and starts a log session)
Two things that we might want to follow-up on:
- update tests (the
findMatchingSimulator.test.ts
still just passesdevice.availability
mocks, so it should probably have some mocks withdevice.isAvailable
) - make a shared variable for availability so we don't repeat ourselves in the future and can update the logic in just one place?
(please lmk if these are needed before merging or anything else)
It looks like we have some overlapping implementations, and this is properly resolved in run-ios, but it haven't propagated to log-ios: https://github.com/react-native-community/cli/blob/b3812eac55dd979bb82b078f347fd45f69dc4392/packages/cli-platform-ios/src/commands/runIOS/findMatchingSimulator.ts#L70-L73 Would you be so kind and refactor this so more code is being shared, to avoid such discrepancies in the future?
Would you be so kind and refactor this so more code is being shared, to avoid such discrepancies in the future?
Yes, 💯 . I wanted to propose this, but didn't want to risk rocking the boat too much. Happy to hop on it!
Would you be so kind and refactor this so more code is being shared, to avoid such discrepancies in the future?
Yes, 💯 . I wanted to propose this, but didn't want to risk rocking the boat too much. Happy to hop on it!
Go for it! 💪
Would you be so kind and refactor this so more code is being shared, to avoid such discrepancies in the future?
Yes, 💯 . I wanted to propose this, but didn't want to risk rocking the boat too much. Happy to hop on it!
Go for it! 💪
Will do!
@esthor is there anything we can help you with to push this through?
I'm adding it to my TODO in a week 🙏
There hasn't been any activity on this pull request in the past 3 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 7 days.
My bad, I thought this was handled in other changes, but it's not. I'll find some time to try and wrap up this PR.
Hey @esthor, recently @adamTrz fixed error with getting devices and simulators in CLI https://github.com/react-native-community/cli/pull/1823 and basically there was a change from xcrun xctrace list devices
to xcrun xcdevice list --json
, and this new command outputs the beautiful JSON with all required informations together with avaliable
. We're thinking about moving to this new command also in log-ios
. Because it's safer (we're not doing some crazy regex, and after that tons of difficult logic instead we're just getting information from JSON).