Skip to content

fix(cli-platform-apple): properly receive simulators - #2251

Merged
thymikee merged 6 commits into
mainfrom
fix/simulators
Jan 15, 2024
Merged

thymikee merged 6 commits into
mainfrom
fix/simulators

Conversation

@szymonrybczak

Copy link
Copy Markdown
Collaborator

Summary:

Fixes regression introduced in #2239, where usage of getSimulators was replaced with listDevices:
CleanShot 2024-01-08 at 21 36 15
all simulators were marked as booted.

In this Pull Request I also removed usage getSimulators and unified downloading simulators across CLI into one place (we were doing this in two separate function, and then we were duplicating .filter, and .map over the codebase).

Test Plan:

  1. Clone the repository and do all the required steps from the Contributing guide
  2. Run this command:
node /path/to/react-native-cli/packages/cli/build/bin.js run-ios 
  1. Run this command:
node /path/to/react-native-cli/packages/cli/build/bin.js log-ios

Checklist

  • Documentation is up to date to reflect these changes.
  • Follows commit message convention described in CONTRIBUTING.md

@szymonrybczak szymonrybczak changed the title fix: properly receive simulators fix(cli-platform-apple): properly receive simulators Jan 8, 2024
@github-actions github-actions Bot added the bugfix label Jan 8, 2024
@szymonrybczak

Copy link
Copy Markdown
Collaborator Author

Idk why units tests are failing, passing locally:
CleanShot 2024-01-08 at 22 09 25

}
});

return merged.filter(({isAvailable}) => isAvailable === true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand: why do we need to run both xcdevice and simctl commands? Shouldn't they provide the same output?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread packages/cli-platform-apple/src/tools/__tests__/listDevices.test.ts Outdated
Comment on lines +58 to +63
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,
);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you measure whether running these 2 commands in parallel is faster or not?

@szymonrybczak szymonrybczak Jan 14, 2024 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🙈

@thymikee
thymikee merged commit dec46c3 into main Jan 15, 2024
@thymikee
thymikee deleted the fix/simulators branch January 15, 2024 09:03
@swrobel

swrobel commented Jan 16, 2024

Copy link
Copy Markdown
Contributor

Could this be backported to 12.x so it can ship with RN 0.73.3?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants