Skip to content
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

fix(launcher): set ARCHPREFERENCE on macOS to prefer arm64 #25014

Merged
merged 27 commits into from
Dec 22, 2022

Conversation

flotwig
Copy link
Contributor

@flotwig flotwig commented Dec 6, 2022

  • Closes

User facing changelog

Fixed an issue where browsers distributed as universal binaries (Chrome, Firefox) on M1 Macs could be launched in the wrong architecture, resulting in poor performance in-browser.

Additional details

  • Browsers launched via processes that are grandchildren of an Intel process will be launched as Intel processes, if they are universal.
  • So if you launch arm64 Cypress via amd64 Node.js, macOS will launch Chrome/FF as their amd64 versions (slow)
  • We can use arch and ARCHPREFERENCE to work around this: https://stackoverflow.com/a/74709144/3474615
  • This PR also splits the browsers list out of browsers.ts and into a knownBrowsers object in known-browsers.ts, to avoid an issue where it is imported in the app's supportFile, which now causes an error due to execa being incompatible with the browser: https://cypressio.slack.com/archives/C01TQQCQLJY/p1671475149587209

Steps to test

Validate that it launches Arm browsers on M1 with x64 Node:

  1. Set up x64 Node.js on an M1 Mac.
  2. Using x64 Node.js, install this branch.
  3. Launch Cypress and validate it is launched as "Apple" in Activity Monitor.
  4. Launch a browser (Chrome, FF) from Cypress and validate it is launched as "Apple" in Activity Monitor, not "Intel".

Validate that x64 Macs can still launch browsers:

  1. Launch Cypress on an x64 Mac and launch a browser packaged as universal (Chrome, FF), validate it launches.

How has the user experience changed?

PR Tasks

  • Have tests been added/updated?
  • [na] Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • [na] Has a PR for user-facing changes been opened in cypress-documentation?
  • [na] Have API changes been updated in the type definitions?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Dec 6, 2022

Thanks for taking the time to open a PR!

@cypress
Copy link

cypress bot commented Dec 6, 2022



Test summary

26392 0 1179 0Flakiness 34


Run details

Project cypress
Status Passed
Commit 3d1badc
Started Dec 22, 2022 8:02 PM
Ended Dec 22, 2022 8:21 PM
Duration 19:07 💡
OS Linux Debian -
Browser Multiple

View run in Cypress Dashboard ➡️


Flakiness

create-from-component.cy.ts Flakiness
1 ... > runs generated spec
2 ... > runs generated spec
commands/net_stubbing.cy.ts Flakiness
1 network stubbing > intercepting request > can delay and throttle a StaticResponse
2 network stubbing > intercepting request > can delay and throttle a StaticResponse
3 ... > with `times` > only uses each handler N times
This comment includes only the first 5 flaky tests. See all 34 flaky tests in the Cypress Dashboard.

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@nagash77 nagash77 requested review from ryanthemanuel and removed request for ryanthemanuel December 7, 2022 14:24
@flotwig flotwig marked this pull request as ready for review December 20, 2022 16:56
@lmiller1990
Copy link
Contributor

Looks like it's working. Here's a video.

I did see cypress config manager as 'intel' - is that correct? The actual app and browser both launched as Apple.

vid.mov

Copy link
Contributor

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

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

Code looks good, I also posted a video of my testing.

Copy link
Collaborator

@ryanthemanuel ryanthemanuel left a comment

Choose a reason for hiding this comment

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

Looks good!

@flotwig
Copy link
Contributor Author

flotwig commented Dec 21, 2022

I did see cypress config manager as 'intel' - is that correct? The actual app and browser both launched as Apple.

@lmiller1990 thanks for testing. This would be expected, since we are using the system Node to launch the config process. Node.js isn't packaged as a universal app so we have to use whatever arch of Node Cy was launched with.

@flotwig flotwig merged commit 4d6034f into develop Dec 22, 2022
@flotwig flotwig deleted the macOS-launch-arm-browser branch December 22, 2022 20:38
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jan 3, 2023

Released in 12.3.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v12.3.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Jan 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants