fix(cli-platform-apple): properly receive simulators #2251
Conversation
szymonrybczak
changed the title
|
Idk why units tests are failing, passing locally: |
| } | ||
| }); | ||
|
|
||
| return merged.filter(({isAvailable}) => isAvailable === true); |
There was a problem hiding this comment.
I don't understand: why do we need to run both xcdevice and simctl commands? Shouldn't they provide the same output?
There was a problem hiding this comment.
So xcdevice provides physical devices, but simctl only simulators. Actually I checked, and simctl is giving informations about simulator's availability, I don't why we had wrong assumptions earlier. I'll update the comment with current state of things.
packages/cli-platform-apple/src/tools/__tests__/listDevices.test.ts
Outdated
Show resolved
Hide resolved
| const xcdeviceOutput = execa.sync('xcrun', ['xcdevice', 'list']).stdout; | ||
| const parsedXcdeviceOutput = parseXcdeviceList(xcdeviceOutput, sdkNames); | ||
|
|
||
| const simctlOutput = JSON.parse( | ||
| execa.sync('xcrun', ['simctl', 'list', '--json', 'devices']).stdout, | ||
| ); |
There was a problem hiding this comment.
can you measure whether running these 2 commands in parallel is faster or not?
There was a problem hiding this comment.
So, I gave it a shot, and used performance.now() to measure execution time in parallel and in sequential run, here are the result:
| Parallel run | Sequential run |
|---|---|
| 5601.046166986227 | 5644.935749977827 |
| 5649.5136660039425 | 5632.401457995176 |
| 5634.3378329873085 | 5684.918291002512 |
| 5606.314500004053 | 5615.477707982063 |
| 5674.328749984503 | 5632.90391600132 |
| AVG: 5633.108183 | AVG: 5642.127425 |
(Results in milliseconds.)
The difference is really small, so I think we can leave it as it is 👍 Btw these two command are adding 5 seconds to the whole execution time of the command 🙈
|
Could this be backported to 12.x so it can ship with RN 0.73.3? |
Summary:
Fixes regression introduced in #2239, where usage of
getSimulatorswas replaced withlistDevices:all simulators were marked as booted.
In this Pull Request I also removed usage
getSimulatorsand unified downloading simulators across CLI into one place (we were doing this in two separate function, and then we were duplicating.filter, and.mapover the codebase).Test Plan:
Checklist