-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
chore(dependency): Upgrade Electron 28 #28959
base: develop
Are you sure you want to change the base?
Conversation
6 flaky tests on run #55520 ↗︎
Details:
net_stubbing.cy.ts • 1 flaky test • 5x-driver-electron
net_stubbing.cy.ts • 2 flaky tests • 5x-driver-chrome:beta
waiting.cy.js • 1 flaky test • 5x-driver-chrome:beta
net_stubbing.cy.ts • 2 flaky tests • 5x-driver-webkit
|
…nto electron-28
@@ -213,7 +213,7 @@ class SourcemapSupport { | |||
frame = cloneCallSite(frame) | |||
|
|||
frame.getFileName = function getFileName () { | |||
return pos.source || pos.name || null | |||
return pos.source || pos.name || undefined |
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 type check was failing, likely due to upgrading types/node. https://app.circleci.com/pipelines/github/cypress-io/cypress/59945/workflows/36318f1d-bf0e-42e7-b0bc-3fa50a5b29f5/jobs/2493950
yarn.lock
Outdated
integrity sha512-2s6B2CWZM//kPgwnuI0KrYwNjfdByE25zvAaEpq9IH4zcNsarH8Ihu/UuX6XMPEogDAxkuUFeZn60pXNHAqn3A== | ||
version "3.54.0" | ||
resolved "https://registry.yarnpkg.com/node-abi/-/node-abi-3.54.0.tgz#f6386f7548817acac6434c6cba02999c9aebcc69" | ||
integrity sha512-p7eGEiQil0YUV3ItH4/tBb781L5impVmmx2E9FRKF7d18XXzp4PGT2tdYMFY6wQqgxD0IwNZOiSJ0/K0fSi/OA== |
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.
node-abi error was encountered, so needed to bump this. electron/forge#667
@@ -181,6 +181,7 @@ export class DataContext { | |||
@cached | |||
get cloud () { | |||
return new CloudDataSource({ | |||
// @ts-ignore | |||
fetch: (...args) => this.util.fetch(...args), |
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.
To address error below. Open to better ideas here. https://app.circleci.com/pipelines/github/cypress-io/cypress/59948/workflows/efa9b4b3-12ca-4f83-a8d7-d5c9186f28d4/jobs/2493976
@@ -16,7 +16,7 @@ | |||
"types": [ | |||
"cypress", | |||
"./support", | |||
"./node_modules/@types/node" | |||
"node" |
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 comment was marked as resolved.
This comment was marked as resolved.
@MikeMcC399 Removed that reference. |
…indow is never visited, avoiding net::ERR_INVALID_ARGUMENT issues [run ci]
@@ -26,6 +26,7 @@ describe('network stubbing', { retries: 15 }, function () { | |||
|
|||
beforeEach(function () { | |||
cy.spy(Cypress.utils, 'warning') | |||
cy.visit('/fixtures/empty.html') |
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.
with Electron 28, we need to visit a url to establish origin , even though the document.location
is already on the correct origin. A bit strange, but this does make sense for security purposes. I do not think this is a breaking change but more so an oddity with the test.
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.
What is the error when this happens?
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.
Will this be technically breaking? Folks could have tests that could have worked previously without a visit that no longer work?
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.
The error was fairly vague via ERR_INVALID_ARGUMENT
when an xhr request would fire. The only way I could resolve it was to visit and establish the origin before making requests to that origin server. By guess is it was a problem with the domain not being established while attempting to make same origin connections. This could technically be breaking, I wonder for those doing API testing, but I would also think most people doing API testing are doing so from cypress commands through the node API and not browser requests before visiting a page
@@ -16,7 +16,7 @@ const { getNextVersionForBinary } = require('../get-next-version') | |||
job_name: process.env.CIRCLE_JOB, | |||
triggered_workflow_id: process.env.CIRCLE_WORKFLOW_ID, | |||
triggered_job_url: process.env.CIRCLE_BUILD_URL, | |||
branch: process.env.CIRCLE_BRANCH, | |||
branch: 'upgrade-electron-28', |
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.
we will need to remove this post merging (as well as merging the updated binary code)
Additional details
Related Docker Images PR https://github.com/cypress-io/cypress-docker-images/pull/1052/files
Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?