-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Cacie/chore/upgrade electron 27 #28544
Changes from 43 commits
8a6c4ab
a89659c
139487b
f3229ed
3a453b0
032a165
185cf86
45a220e
c1fcdb1
509e499
7912db2
f875b0a
b1c1d52
58b1349
b32a1e2
3bb82dd
a0bd74d
1066e37
acaaea6
b76511d
eae7513
a6ac000
e7e48ad
f843948
6e64ceb
9a3af58
a63fde1
3961271
4e0d91e
7d80f77
f4fb6c4
8dadce3
016923e
0d05c41
4a4ae23
8895708
e091dfb
bfee9dd
665e81c
0c0ff5a
a882362
b55e8e0
6475305
35e10a3
4dbac8e
2049685
98bf7b2
42c115b
8a99704
3a02fec
a9ece8a
ac7091d
c1209c6
a7a5a6b
4849187
382dbc5
fab6898
aa09aa8
a4673ca
9dd9ab1
ab49bc1
f48e6f0
fabb6a1
bb2a882
219e4f6
7468022
d257809
5816027
e3eb46c
71c1bf3
8d8a547
21977f8
7c6646f
1f8bce9
307e709
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
# Bump this version to force CI to re-create the cache from scratch. | ||
|
||
11-20-23 | ||
12-15-23 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
18.15.0 | ||
18.17.1 |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -36,7 +36,7 @@ Upgrading `electron` involves more than just bumping this package's `package.jso | |||
- the major version number of Node.js changes, since users rely on the bundled Node.js to load plugins and `.js` fixtures, or | ||||
- there are changes to Electron that require new shared libraries to be installed on Linux, breaking existing CI setups, or | ||||
- there is some other change that would break existing usage of Cypress (for example, a Web API feature being removed/added to the bundled Chromium) | ||||
- [ ] **Create and publish Docker `base-internal` and `browsers-internal` family images matching the Node.js and Chromium versions in Electron.** These images live inside the [`cypress-docker-images`](https://github.com/cypress-io/cypress-docker-images/) repository. The `browsers-internal` image will be used inside our CI pipelines. The `base-internal` image will be used by the `browsers-internal` image and possibly other system images (described below). For general use of Cypress in Docker, we encourage the use of the [Cypress Docker Factory](https://github.com/cypress-io/cypress-docker-images#cypressfactory). This works great for using Cypress as an end user, but doesn't fully suit the needs for developing Cypress, as we require: | ||||
- [ ] **Create and publish Docker `base-internal` and `browsers-internal` family images matching the Node.js and Chromium versions in Electron.** These images live inside the [`cypress-docker-images`](https://github.com/cypress-io/cypress-docker-images/) repository. The `browsers-internal` image will be used inside our CI pipelines. The `base-internal` image will be used by the `browsers-internal` image and possibly other system images (described below). The Chromium version can be determined from the [DEPS](https://github.com/electron/electron/blob/main/DEPS) file in Electron's repository on the correct tag. The Node version can be determined from the [Electron Releases page](https://www.electronjs.org/docs/latest/tutorial/electron-timelines). For general use of Cypress in Docker, we encourage the use of the [Cypress Docker Factory](https://github.com/cypress-io/cypress-docker-images#cypressfactory). This works great for using Cypress as an end user, but doesn't fully suit the needs for developing Cypress, as we require: | ||||
- The installation of packages, such as `curl`, `xauth`, and `build-essential`/`make` needed for our [`circleci`](../../.circleci/config.yml) jobs/pipelines. | ||||
- Specific images targeted to test Cypress on various node versions and distributions of linux, such as different versions of `ubuntu`. | ||||
|
||||
|
@@ -45,19 +45,44 @@ Upgrading `electron` involves more than just bumping this package's `package.jso | |||
- The Ubuntu images in [base-internal](https://github.com/cypress-io/cypress-docker-images/tree/master/base-internal) are updated to be used in the [system binary tests](../../system-tests/test-binary) if any of the following are true for the images used inside the system binary tests: | ||||
- The last two major [Ubuntu LTS Releases](https://ubuntu.com/about/release-cycle) are out-of-date. | ||||
- The [NodeJS](https://nodejs.org/en) version is not the active LTS. | ||||
- [ ] **Update `workflows.yml`** | ||||
- [ ] Ensure it references the new `base-internal` and `browsers-internal` Docker images | ||||
- [ ] Ensure the new Electron version is used as a build target in the `Build better-sqlite3 for CentOS 7` step | ||||
- [ ] | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
It looks like this line should be removed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, Mike! Still making sure all of the rest of things are working as expected, so this file will likely change some more before it's marked ready for review. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. I'll hold back until the PR is marked for review. |
||||
|
||||
- [ ] **Ensure that a matching Node.js version is enforced in the monorepo for local development and CI.** When Electron is upgraded, oftentimes, the bundled Node.js version that comes with Electron is updated as well. Because all unit and integration tests run in normal Node.js (not Electron's Node.js), it's important for this Node.js version to be synced with the monorepo. There are a few places where this needs to be done: | ||||
- [ ] [`/.node-version`](../../.node-version) - used by `nvm` and other Node version managers | ||||
- [ ] `@types/node` used throughout the monorepo to determine compatible node types. The major version of this package must reflect the node version set in [`/.node-version`](../../.node-version). | ||||
- [ ] [github workflows](../../.github) - used for repository templates, vulnerability detection, and V8 snapshots. If the node version for Snyk needs to be updated, then the required pull request check into `develop` must also be updated. A repository administrator will need to accomplish this. | ||||
- [ ] [`/package.json`](../../package.json) - update `engines` | ||||
- [ ] [`/scripts/run-docker-local.sh`](../../scripts/run-docker-local.sh) - update Docker image to the new matching `browsers` image | ||||
- [ ] [`docker-compose.yml`](../../docker-compose.yml) - update Docker image to the new matching `browsers` image | ||||
- [ ] [`/system-tests/test-binary/*`](../../system-tests/test-binary) - update binary system tests to use the newly published Ubuntu and Node images mentioned above, if applicable | ||||
- [ ] [`/.circleci/config.yml`](../../.circleci/config.yml) | ||||
- Update the Docker `image`s to the new matching `browsers` image. | ||||
- Update the `xcode` version to one with the same major Node.js version bundled. There is usually not an exact match, this is ok as long as the major version number as the same. | ||||
- [ ] Do a global search for the old Node.js version to identify any new areas that may need updating/unification, and update those locations (and this document!) | ||||
- [ ] Do a global search for the old Node.js version to identify any new areas that may need updating/unification, and update those locations (and this document!) | ||||
|
||||
- [ ] For **binary publishing**, make sure the `electron` version that we updated in [`/package.json`](../../package.json) matches the `electron` version inside the [publish binary project](https://github.com/cypress-io/cypress-publish-binary/blob/main/package.json). This is to make sure add-on tests inside the publish-binary repository work locally, but are not required to install the correct version of `electron` in CI when publishing the binary. Ensure the electron target in this project's .circleci configuration is updated as well. Set the target branch on that project as a `branch` property on the request body in [../../scripts/binary/trigger-publish-binary-pipeline.js](../../scripts/binary/trigger-publish-binary-pipeline.js) script, so that you can test in CI. Remove this before merging, and ensure that branch is merged as well. | ||||
|
||||
|
||||
- [ ] **Manually smoke test `cypress open`.** Upgrading Electron can break the `desktop-gui` in unexpected ways. Since testing in this area is weak, double-check that things like launching `cypress open`, signing into Cypress Cloud, and launching Electron tests still work. | ||||
- [] **Manually smoke test `cypress run` in record mode** Upgrading Electron can cause `better-sqlite3` to SIGSEGV the Electron process. | ||||
cacieprins marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
- [ ] **Fix failing tests.** Usually, these are due to breaking changes in either Node.js or Electron. Check the changelogs of both to find relevant changes. | ||||
- [ ] For **binary publishing**, make sure the `electron` version that we updated in [`/package.json`](../../package.json) matches the `electron` version inside the [publish binary project](https://github.com/cypress-io/cypress-publish-binary/blob/main/package.json). This is to make sure add-on tests inside the publish-binary repository work locally, but are not required to install the correct version of `electron` in CI when publishing the binary. | ||||
- [ ] If needed, update the **[V8 Snapshot Cache](https://github.com/cypress-io/cypress/actions/workflows/update_v8_snapshot_cache.yml)** by running the workflow. Make sure to use the branch that contains the electron updates to populate the `'workflow from'` and `'branch to update'` arguments. Select `'Generate from scratch'` and `'commit directly to branch'`. This will usually take 6-8 hours to complete and is best to not be actively developing on the branch when this workflow runs. | ||||
|
||||
- [ ] If needed, update the **[V8 Snapshot Cache](https://github.com/cypress-io/cypress/actions/workflows/update_v8_snapshot_cache.yml)** by running the GitHub workflow. Make sure to use the branch that contains the electron updates to populate the `'workflow from'` and `'branch to update'` arguments. Select `'Generate from scratch'` and `'commit directly to branch'`. This will usually take 6-8 hours to complete and is best to not be actively developing on the branch when this workflow runs. | ||||
|
||||
|
||||
### Common Upgrade Issues | ||||
|
||||
#### Integrity Check Failures | ||||
|
||||
*Solution*: Update the string representation of `fs.readFileSync` in [scripts/binary/binary-integrity-check-source.js](../../scripts/binary/binary-integrity-check-source.js) to match the string generated by the new version of electron. Create a throw-away script and simply `console.log(fs.readFileSync.toString())`, and execute it with *Electron* rather than *Node*. | ||||
|
||||
#### ResizeObserver errors in Component Test | ||||
|
||||
*Solution*: This error is benign. From time to time, the error message we match against in order to swallow the error changes. Update the necessary support files with the new error message. | ||||
|
||||
#### Electron crashes immediately after initializing the Protocol database | ||||
|
||||
*Solution*: ??? | ||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be non-passive to bump for yarn projects
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wait -- my bad -- this is our for our development version. This is fine. I mistook this for cli/package.json.