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

dependency: upgrade to electron 29 #30224

Closed
wants to merge 66 commits into from
Closed

dependency: upgrade to electron 29 #30224

wants to merge 66 commits into from

Conversation

ryanthemanuel
Copy link
Collaborator

@ryanthemanuel ryanthemanuel commented Sep 11, 2024

CHANGELOG - https://releases.electronjs.org/releases/stable?version=29

Additional details

  • Upgrade to Chromium 122
  • Upgrade to Node 20.9.0+
  • Upgrade to v8 12.0

Related Docker Images PR https://github.com/cypress-io/cypress-docker-images/pull/1052/files

Unfortunately there is a breaking change in Electron 28 that would affect our users. That breaking change has been fixed in Electron 29, but Electron 29 contains a bug in v8 that we've had to raise with Chromium here: issues.chromium.org/issues/345280736 We're awaiting to see what their response is as we need a resolution to continue our upgrade.

Steps to test

How has the user experience changed?

PR Tasks

jennifer-shehane and others added 30 commits February 16, 2024 10:52
@@ -16,7 +16,7 @@
"types": [
"cypress",
"./support",
"./node_modules/@types/node"
"node"
Copy link
Member

Choose a reason for hiding this comment

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

@@ -181,6 +181,7 @@ export class DataContext {
@cached
get cloud () {
return new CloudDataSource({
// @ts-ignore
Copy link
Member

Choose a reason for hiding this comment

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

@@ -26,6 +26,7 @@ describe('network stubbing', { retries: 15 }, function () {

beforeEach(function () {
cy.spy(Cypress.utils, 'warning')
cy.visit('/fixtures/empty.html')
Copy link
Member

Choose a reason for hiding this comment

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

We'll want to remove this. This should have only been necessary in Electron 28 - that's the assumption anyway. See https://github.com/cypress-io/cypress/pull/28959/files#r1603224984

@@ -39,7 +39,7 @@
"ansi_up": "5.0.0",
"ast-types": "0.13.3",
"base64url": "^3.0.1",
"better-sqlite3": "9.2.2",
"better-sqlite3": "10.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

@ryanthemanuel Do you remember the context as to why this was necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I do not unfortunately. We can try reverting?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@AtofStryker do you happen to remember either? I know you were working in here too at times.

Copy link
Member

Choose a reason for hiding this comment

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

I was just wondering - I don't think have confirmation that it's breaking anything - it was just a difference between 29 and 28

Copy link
Member

@jennifer-shehane jennifer-shehane Sep 12, 2024

Choose a reason for hiding this comment

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

This is needed due to this failure when it is left on 9.2.2: https://cloud.nx.app/runs/n23a1Mbn3x

Screenshot 2024-09-12 at 2 55 19 PM

@@ -462,7 +462,7 @@ export class SnapshotGenerator {

// 2. Run the `mksnapshot` binary providing it the path to our snapshot
// script
const args = [this.snapshotScriptPath, '--output_dir', this.snapshotBinDir]
const args = [this.snapshotScriptPath, '--output_dir', this.snapshotBinDir, '--no-use-ic']
Copy link
Member

Choose a reason for hiding this comment

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

See https://issues.chromium.org/issues/345280736#comment12 for context - we should add this inline as a comment.

Copy link

cypress bot commented Sep 11, 2024

cypress    Run #57052

Run Properties:  status check failed Failed #57052  •  git commit f03ad631d8: try something else
Project cypress
Branch Review electron-29
Run status status check failed Failed #57052
Run duration 18m 24s
Commit git commit f03ad631d8: try something else
Committer Ryan Manuel
View all properties for this run ↗︎

Test results
Tests that failed  Failures 2
Tests that were flaky  Flaky 2
Tests that did not run due to a developer annotating a test with .skip  Pending 26
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 768
View all changes introduced in this branch ↗︎
UI Coverage  67.9%
  Untested elements 24  
  Tested elements 55  
Accessibility  96.4%
  Failed rules  0 critical   6 serious   1 moderate   0 minor
  Failed elements 210  

Tests for review

Failed  cypress\e2e\config-files-error-handling.cy.ts • 1 failed test • launchpad-e2e

View Output

Test Artifacts
Launchpad: Error System Tests > shows correct stack trace when config with ts-module error Test Replay Screenshots
Failed  cypress\e2e\runner\retries.experimentalRetries.mochaEvents.cy.ts • 1 failed test • app-e2e

View Output

Test Artifacts
... > does not try to serialize error with err.actual as DOM node Test Replay Screenshots
Flakiness  cypress\e2e\specs.cy.ts • 1 flaky test • app-e2e

View Output

Test Artifacts
... > scaffold starter spec > generates spec with file name that does not contain a known spec extension Test Replay Screenshots
Flakiness  cypress\e2e\subscriptions\createCloudOrgModal-subscription.cy.ts • 1 flaky test • app-e2e

View Output

Test Artifacts
CreateCloudOrgModalSubscription > Runs - Connect Org > opens create Org modal after clicking Connect Project button Test Replay Screenshots

@jennifer-shehane jennifer-shehane changed the title fix(dep): upgrade to electron 29 dependency: upgrade to electron 29 Sep 12, 2024
@@ -185,7 +185,7 @@ export function create (projectRoot, _options: WindowOptions, newBrowserWindow =
})
}

win.webContents.on('crashed', function (...args) {
win.webContents.on('render-process-gone', function (...args) {
Copy link
Member

@jennifer-shehane jennifer-shehane Sep 12, 2024

Choose a reason for hiding this comment

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

@jennifer-shehane
Copy link
Member

Superseded by this PR: #30394

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants