-
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
Electron upgrade #5849
Electron upgrade #5849
Conversation
Thanks for the contribution! Below are some guidelines Cypress uses when doing PR reviews.
PR Review ChecklistIf any of the following requirements can't be met, leave a comment in the review selecting 'Request changes', otherwise 'Approve'. User Experience
Functionality
Maintainability
Quality
Internal
|
This reverts commit db2d521.
Test summaryRun details
View run in 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 |
This reverts commit 57fe764.
ba328aa
to
45d3bd5
Compare
executors: | ||
# the Docker image with Cypress dependencies and Chrome browser | ||
cy-doc: | ||
docker: | ||
- image: cypress/browsers:node12.0.0-chrome73 | ||
- image: cypress/browsers:node12.8.1-chrome78-ff70 |
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.
Is this PR updating the Node version? Aren't we using 12.0.0, don't we want to test against that?
Also, if this is a convenience thing - probably highlights why our docker images combinations are problematic for our own users. :P
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 PR updates the bundled Node version to 12.8.1 (Electron 5 -> 7). The node
version we use in CI needs to match this, since the unit + integrations tests (of Electron code) all run in node
in CI, not electron
. We could use electron-mocha
to eliminate this requirement, but currently that's the way it is.
User facing changelog
Additional details
automation
code and related tests #5850 getting merged - most of the diff is thisHow has the user experience changed?
PR Tasks