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

build: allow unified to run cypress on Apple Silicon (arm64) #19067

Merged
merged 8 commits into from
Dec 7, 2021

Conversation

elevatebart
Copy link
Contributor

connected to #14923

This PR updates electron-packager for arm64 compatibility.
Defaults have cnaged a little so a rename was necessary.

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Nov 23, 2021

Thanks for taking the time to open a PR!

@elevatebart elevatebart changed the title build: allow to run cypress on apple cilicon build: allow unified to run cypress on Apple Silicon (arm64) Nov 23, 2021
@cypress
Copy link

cypress bot commented Nov 23, 2021



Test summary

18146 1 202 0Flakiness 2


Run details

Project cypress
Status Failed
Commit bf7a46c
Started Dec 6, 2021 4:17 PM
Ended Dec 6, 2021 4:29 PM
Duration 11:31 💡
OS Linux Debian - 10.10
Browser Multiple

View run in Cypress Dashboard ➡️


Failures

cypress/integration/commands/net_stubbing_spec.ts Failed
1 network stubbing > intercepting request > should delay the same amount on every response

Flakiness

cypress/integration/commands/net_stubbing_spec.ts Flakiness
1 network stubbing > intercepting response > can delay and throttle a static response
2 network stubbing > waiting and aliasing > can timeout waiting on a single request using "alias.request"

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

@mapsandapps
Copy link
Contributor

Working for me! It'd be nice if we could merge this.

@elevatebart
Copy link
Contributor Author

@flotwig @chrisbreiding you guys are the experts in all things electron.
Do you think this change can break the binary?

@elevatebart elevatebart marked this pull request as ready for review December 3, 2021 18:05
@elevatebart elevatebart requested a review from a team as a code owner December 3, 2021 18:05
@emilyrohrbough
Copy link
Member

@elevatebart What is your plan to test this to ensure binary behaves as expected? Were you able to verify the linux, mac & windows binaries with this change?

@elevatebart
Copy link
Contributor Author

@emilyrohrbough I ran this on Mac M1 & Intel and it works.
I cannot sign a binary myself. I don't know that I can test anything prod like without signing the final binary.
I would hope that we merge this, test it and revert the commit if things are broken.
The change is simple enough that it will not impact much.

Additionally, the windows build for 10.0-release has not compiled for at least a few weeks.
I don't have a linux machine, but I expect tests in CI to be run. with a linux machine

@flotwig
Copy link
Contributor

flotwig commented Dec 3, 2021

I'm not totally sure what advantage this brings. I see in 15.1.0 electron-packager added darwin-arm64 as an arch/platform combo. But this change doesn't modify any of the build artifacts, right? So is there any advantage to making this change?

@elevatebart are you planning on making the rest of the changes needed for darwin-arm64 support (updating the download server, CLI, build process, CI setup?) afaik we are still blocked on https://circleci.canny.io/cloud-feature-requests/p/support-new-m1-arm-based-macs

Do you think this change can break the binary?

No, I don't think so, but it's worth manually testing the built binaries (Windows, Mac, Linux) any time the packaging is updated.

@elevatebart
Copy link
Contributor Author

@flotwig this change only impact development.
For the rest of the work, I plan to wait for your PR to be delivered. Then I can iterate to make this branch work with the changes.

For information, until recently we could run yarn dev on a arm64 machine without trouble using Rosetta.
Now it has become impossible, even using Rosetta because yarn dev packages for the current environment.
This small change fixes the dev process.

flotwig
flotwig previously approved these changes Dec 3, 2021
Copy link
Contributor

@flotwig flotwig left a comment

Choose a reason for hiding this comment

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

Seems fine to upgrade. @elevatebart do you happen to have a link to the issue that changed app.js -> index.js? I'm not seeing anything in electron-packager's changelog.

For the rest of the work, I plan to wait for your PR to be delivered. Then I can iterate to make this branch work with the changes.

Gotcha, currently waiting on CircleCI for that one.

@elevatebart
Copy link
Contributor Author

@flotwig I did not find this issue, I read the error message and fixed the error as I saw it.
I can try and dig it up if you really care but it will take me a while.

@flotwig
Copy link
Contributor

flotwig commented Dec 6, 2021

I can try and dig it up if you really care but it will take me a while.

All good. I did push a commit just to verify that the packaged app still works on all platforms, but as long as that passes, LGTM. 9308af8

This reverts commit 9308af8.
@flotwig
Copy link
Contributor

flotwig commented Dec 6, 2021

Well, considering that 10.0-release's CI is just as busted as this branch, I guess there was no point in
9308af8 🤠

@elevatebart elevatebart merged commit 656f769 into 10.0-release Dec 7, 2021
@elevatebart elevatebart deleted the dev/allow-devs-arm64 branch December 7, 2021 14:47
baversjo added a commit to smrt/cypress that referenced this pull request Jan 22, 2022
baversjo added a commit to smrt/cypress that referenced this pull request Jan 29, 2022
tgriesser added a commit that referenced this pull request Feb 4, 2022
* develop:
  chore: fix cypress npm package artifact upload path (#20023)
  chore(driver): move cy.within logic into it's own file (#20036)
  chore: update automerge workflows (#19982)
  fix(selectFile): use target window's File/DataTransfer classes (#20003)
  chore: Update Chrome (stable) to 98.0.4758.80 and Chrome (beta) to 98.0.4758.80 (#19995)
  fix: Adjust ffmpeg CLI args for performance (#19983)
  build: allow unified to run cypress on Apple Silicon (arm64) (backport #19067 to 9.x) (#19968)
brian-mann pushed a commit that referenced this pull request Feb 5, 2022
* develop:
  chore: fix cypress npm package artifact upload path (#20023)
  chore(driver): move cy.within logic into it's own file (#20036)
  chore: update automerge workflows (#19982)
  fix(selectFile): use target window's File/DataTransfer classes (#20003)
  chore: Update Chrome (stable) to 98.0.4758.80 and Chrome (beta) to 98.0.4758.80 (#19995)
  fix: Adjust ffmpeg CLI args for performance (#19983)
  build: allow unified to run cypress on Apple Silicon (arm64) (backport #19067 to 9.x) (#19968)
tgriesser added a commit that referenced this pull request Feb 7, 2022
* 10.0-release:
  chore: make error more informative about migration to Cypress 10 (#20007)
  fix: remove shelljs in example build script, correct file renames
  fix: last step in the ct setup goes back to start  (#20030)
  fix: typescript default plugin file (#20046)
  chore: fix cypress npm package artifact upload path (#20023)
  chore(driver): move cy.within logic into it's own file (#20036)
  chore: update automerge workflows (#19982)
  fix(selectFile): use target window's File/DataTransfer classes (#20003)
  chore: Update Chrome (stable) to 98.0.4758.80 and Chrome (beta) to 98.0.4758.80 (#19995)
  fix: Adjust ffmpeg CLI args for performance (#19983)
  build: allow unified to run cypress on Apple Silicon (arm64) (backport #19067 to 9.x) (#19968)
  release 9.4.1 [skip ci]
  fix: trigger 9.4.1 build
  release 9.4.0 [skip ci]
brian-mann added a commit that referenced this pull request Feb 11, 2022
* chore: rename errors.js -> errors.ts

* refactor: type safety on errors

* refactor: add err_template for consistent error formatting

* fix a few system tests

* fix tests; update snapshots

* Fix types

* normalize snapshot - remove chalk ansi colors

* more unit test fixes

* more system test fixes

* circleci build

* backtick always in stdout, fix error formatting and failing snapshots

* refactor: create @packages/errors

* fix import

* fix import

* fixing build / tests

* remove extraneous file

* move warnIfExplicitCiBuildId

* fix build / tests

* Fix

* error, type fixes, documentation, standardize child process error serialization

* fix import

* build errors on install

* wrote specs generating visual images of all errors

* remove unused dep

* sanitize stack traces

* add image diffing

- if base images don't exist, create them
- if base images don't match and local, overwrite them, if in CI throw
- if base images are stale and local, delete them, if in CI throw

* remove Courier New + MesloLGS NF font

* type fixes, remove Bluebird, general cleanup

* TS Cleanup

* skip typecheck on tests for now

* yarn.lock

* fix @types/chai version

* fix yarn.lock

* Different version of mocha types so it isnt patched

* errors spec snapshot

* CI fix

* fixes

* store snapshot images in circle artifacts

* dont change artifact destination prefix

* use Courier Prime

* antialias the text

* decrease pixelmatch threshold, fail in CI only when changed pixels > 100

* increase timeout

* overflow: hidden, remove new Promise, add debug logging

Co-Authored-By: Tim Griesser <tgriesser@gmail.com>

* run unit tests w/ concurrency=1

* unique window per file

* disable app hardware acceleration + use in process gpu + single process

* do not do image diffing

- conditionally convert html to images
- store html snapshots
- do not store images in git

* store snapshot html

* Merge branch 'tgriesser/chore/refactor-errors' of https://github.com/cypress-io/cypress into tgriesser/chore/refactor-errors

* remove concurrency

* fix assertion

* fixing ci

* Link in readme

* pass the browsers to listItems

* fix: build @packages/errors in CI, defer import to prevent errors locally

* Merge branch 'develop' into tgriesser/chore/refactor-errors

* develop:
  chore: fix cypress npm package artifact upload path (#20023)
  chore(driver): move cy.within logic into it's own file (#20036)
  chore: update automerge workflows (#19982)
  fix(selectFile): use target window's File/DataTransfer classes (#20003)
  chore: Update Chrome (stable) to 98.0.4758.80 and Chrome (beta) to 98.0.4758.80 (#19995)
  fix: Adjust ffmpeg CLI args for performance (#19983)
  build: allow unified to run cypress on Apple Silicon (arm64) (backport #19067 to 9.x) (#19968)

* fix run-if-ci.sh

* remove dead code

* Mark the .html files as generated in gitattributes

* fix running single error case, slice out more of the brittle stack

* remove additional brittle stack line

* firefox web security error

* nest inside of describe

* reformat and redesign errors

* more error cleanup and standardization

* additional formatting of errors, code cleanup, refactoring

* update ansi colors to match terminal colors

* cleanup remaining loose ends, update several errors, compact excess formatters

* fix types

* additional formatting, remove TODO's, ensure no [object Object] in output

* add test for 412 server response on invalid schema

* update unknown dashboard error on creating run

* use fs.access instead of fs.stat for perf

* added PLUGINS_FILE_NOT_FOUND error

- separated out from PLUGINS_FILE_ERROR
- add system tests for both cases
- update snapshots
- remove stack trace from PLUGINS_FILE_NOT_FOUND fka PLUGINS_FILE_ERROR

* add plugins system test around plugins synchronously throwing on require

* remove forcing process.cwd() to be packages/server, update affected code

- this was a long needed hangover from very old code that was doing unnecessary things due to respawning electron from node and handling various entrypoints into booting cypress
- this also makes the root yarn dev and dev-debug work correctly because options.cwd is set correctly now

* perf: lazy load chalk

* remove excessive line since the file exists but is invalid

* fix types

* add system test when plugins function throws a synchronous error

* create new PLUGINS_INVALID_EVENT_ERROR, wire it up correctly for error template use

- properly pass error instance from child to ensure proper user stack frames
- move error display code into the right place

* only show a single stack trace, either originalError or internal cypressError

* push error html snapshots

* fix tests, types

* fix test

* fix failing tests

* fix tests

* fixes lots of broken tests

* more test fixes

* fixes more tests

* fix type checking

* wip: consistent handling of interpolated values

* feat: fixing up errors, added simple error comparison tool

* wrapping up error formatting

* Fixes for unit tests

* fix PLUGINS_VALIDATION_ERROR

* fix fs.readdir bug, show rows even if there's only markdown formatting [SKIP CI]

* when in base-list, show full width of errors

* Fix type errors

* added searching and filtering for files based on error name

* fix: system tests

* updated NO_SPECS_FOUND error to properly join searched folder + pattern

- join patterns off of process.cwd, not projectRoot
- highlight original specPattern in yellow, baseDir in blue
- add tests

* fixes failing tests

* fix test

* preserve original spec pattern, display relative to projectRoot for terminal banner

* make the nodeVersion path display in gray, not white

* fix tests, pass right variables

* fix chrome:canary snapshots

* update snapshots

* update snapshot

* strip newlines caused by "Still waiting to connect to {browser}..."

* don't remove the snapshotHtmlFolder in CI, add additional verification snapshots match to error keys symmetrically

* update snapshot

* update snapshot

* update snapshot

* update snapshot

* update snapshot

* update snapshot

* update gitignore

* fix snapshot

* update snapshot html

* update logic for parsing the resolve pattern matching, add tests

* update snapshots

* update snapshot

* update snapshot

* update snapshot

* fix failing test

* fix: error_message_spec

* fix snapshot

* run each variant through an it(...) so multiple failures are received

* add newlines to multiline formatters, add fmt.stringify, allow format overrides

* stringify invalid return values from plugins

* move config validation errors into packages/errors, properly highlight and stringify values

* add component testing yarn commands

* fix the arrow not showing on details

* fix typescript error

* fixed lots of poorly written tests that weren't actually testing anything. created settings validation error when given a string validation result.

* fixes tests

* fixes tests, adds new error template for validating within an array list (for browser validation)

* remove dupe line

* fix copy for consistency, update snapshots

* remove redundant errors, standardize formatting and phrasing

* more formatting

* remove excess snapshots

* prune out excessive error snapshot html files when not in CI

* add missing tests, add case for when config validation fails without a fileType

* fixes test

* update snapshot

* update snapshot

* update snapshot

* sort uniqErrors + errorKeys prior to assertion - assert one by one

* add system test for binding to an event with the wrong handler

* fixes tests

* set more descriptive errors when setting invalid plugin events, or during plugin event validation

* remove duplicate PLUGINS_EVENT_ERROR, collapse into existing error

* use the same multiline formatting as @packages/errors

* standardize verbiage and highlighting for consistency

* fix incorrect error path

* fixes tests, standardized and condensed more language

* Update packages/errors/src/errors.ts

Co-authored-by: Ryan Manuel <ryanm@cypress.io>

* Update guides/error-handling.md

Co-authored-by: Ryan Manuel <ryanm@cypress.io>

* Update guides/error-handling.md

Co-authored-by: Ryan Manuel <ryanm@cypress.io>

* added some final todo's

* fix types

Co-authored-by: Tim Griesser <tgriesser10@gmail.com>
Co-authored-by: Tim Griesser <tgriesser@gmail.com>
Co-authored-by: Ryan Manuel <ryanm@cypress.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants