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

chore(deps): Updating electron to v18 + node v16.13.2 #21418

Merged
merged 6 commits into from
May 12, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .node-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
16.5.0
16.13.2
24 changes: 17 additions & 7 deletions circle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ macWorkflowFilters: &mac-workflow-filters
or:
- equal: [ develop, << pipeline.git.branch >> ]
- equal: [ check-results-for-env, << pipeline.git.branch >> ]
- equal: [ tbiethman/chore/electron-18-update, << pipeline.git.branch >> ]
- matches:
pattern: "-release$"
value: << pipeline.git.branch >>
Expand All @@ -49,6 +50,7 @@ windowsWorkflowFilters: &windows-workflow-filters
- equal: [ master, << pipeline.git.branch >> ]
- equal: [ develop, << pipeline.git.branch >> ]
- equal: [ check-results-for-env, << pipeline.git.branch >> ]
- equal: [ tbiethman/chore/electron-18-update, << pipeline.git.branch >> ]
- matches:
pattern: "-release$"
value: << pipeline.git.branch >>
Expand All @@ -60,7 +62,7 @@ executors:
# the Docker image with Cypress dependencies and Chrome browser
cy-doc:
docker:
- image: cypress/browsers:node16.5.0-chrome94-ff93
- image: cypress/browsers:node16.13.2-chrome100-ff98
# by default, we use "small" to save on CI costs. bump on a per-job basis if needed.
resource_class: small
environment:
Expand All @@ -69,7 +71,7 @@ executors:
# Docker image with non-root "node" user
non-root-docker-user:
docker:
- image: cypress/browsers:node16.5.0-chrome94-ff93
- image: cypress/browsers:node16.13.2-chrome100-ff98
user: node
environment:
PLATFORM: linux
Expand Down Expand Up @@ -371,8 +373,10 @@ commands:
npm install yarn -g # ensure yarn is installed with the correct node engine
yarn check-node-version
- run:
name: Check Node
command: yarn check-node-version
name: Check Node
command: |
[ -s "${HOME}/.nvm/nvm.sh" ] && \. "${HOME}/.nvm/nvm.sh"
yarn check-node-version

install-chrome:
description: Install Google Chrome
Expand Down Expand Up @@ -589,7 +593,11 @@ commands:
## by default, assert that at least 1 test ran
default: 0
steps:
- run: yarn verify:mocha:results <<parameters.expectedResultCount>>
- run:
name: 'Verify Mocha Results'
command: |
[ -s "${HOME}/.nvm/nvm.sh" ] && \. "${HOME}/.nvm/nvm.sh"
yarn verify:mocha:results <<parameters.expectedResultCount>>

clone-repo-and-checkout-release-branch:
description: |
Expand Down Expand Up @@ -896,7 +904,9 @@ commands:
yarn binary-build --platform $PLATFORM --version $(node ./scripts/get-next-version.js)
- run:
name: Zip the binary
command: yarn binary-zip --platform $PLATFORM
command: |
apt-get update && apt-get install -y zip
Copy link
Contributor Author

Choose a reason for hiding this comment

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

zip was removed from our base image generator in a recent slimming down effort. We can add it back/generate a new image if there's a concern with pulling it in here.

yarn binary-zip --platform $PLATFORM
- store-npm-logs
- persist_to_workspace:
root: ~/
Expand Down Expand Up @@ -1247,7 +1257,7 @@ jobs:

runner-integration-tests-firefox:
<<: *defaults
resource_class: medium
resource_class: medium+
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The resources were bumped for the Firefox tests due to increased memory consumption. #21319 was logged to investigate further.

parallelism: 2
steps:
- run-runner-integration-tests:
Expand Down
4 changes: 2 additions & 2 deletions cli/test/lib/util_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ describe('util', () => {
})

it('copy NODE_OPTIONS to ORIGINAL_NODE_OPTIONS', () => {
sandbox.stub(process.versions, 'node').value('v16.5.0')
sandbox.stub(process.versions, 'node').value('v16.13.2')
sandbox.stub(process.versions, 'openssl').value('1.0.0')

restoreEnv = mockedEnv({
Expand Down Expand Up @@ -322,7 +322,7 @@ describe('util', () => {

// https://github.com/cypress-io/cypress/issues/18914
it('does not include --openssl-legacy-provider in Node <=16', () => {
sandbox.stub(process.versions, 'node').value('v16.5.0')
sandbox.stub(process.versions, 'node').value('v16.13.2')
sandbox.stub(process.versions, 'openssl').value('1.0.0')

restoreEnv = mockedEnv({})
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@
"yarn-deduplicate": "3.1.0"
},
"engines": {
"node": ">=16.5.0",
"node": ">=16.13.2",
"yarn": ">=1.17.3"
},
"productName": "Cypress",
Expand Down
4 changes: 4 additions & 0 deletions packages/driver/cypress/fixtures/dom.html
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,10 @@
width: 60px;
}

/* normalizing li height across supported browsers */
li {
height: 18px;
}
</style>

DOM Fixture
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -589,13 +589,10 @@ describe('src/cy/commands/actions/trigger', () => {
})

it('passes options.animationDistanceThreshold to cy.ensureElementIsNotAnimating', () => {
const $btn = cy.$$('button:first')

const { fromElWindow } = Cypress.dom.getElementCoordinatesByPosition($btn)

cy.spy(cy, 'ensureElementIsNotAnimating')

cy.get('button:first').trigger('tap', { animationDistanceThreshold: 1000 }).then(() => {
cy.get('button:first').trigger('tap', { animationDistanceThreshold: 1000 }).then(($btn) => {
const { fromElWindow } = Cypress.dom.getElementCoordinatesByPosition($btn)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These tests related to animationDistanceThreshold across trigger/type command tests were failing in a similar way in Firefox.

Firefox reports positions/scroll offsets with something like 13 digits of precision. We do a lot rounding in our coordinate logic that could be impacted by this. These tests also seem to be influenced by how the AUT is being scaled; I can get them to pass/fail in develop by running the tests with the AUT scaled at different sizes.

I updated these tests to grab the reference element window coordinates after the element has been scrolled into position, which makes us less susceptible to variances within Firefox's reported scroll position. This allows them to pass consistently without need to add 1px buffers to our assertions.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we can use a similar technique to remove this 1px hack for Firefox in 10.0-release? https://github.com/cypress-io/cypress/blob/10.0-release/packages/driver/cypress/e2e/commands/actions/type.cy.js#L360-L377

const { args } = cy.ensureElementIsNotAnimating.firstCall

expect(args[1]).to.deep.eq([fromElWindow, fromElWindow])
Expand All @@ -606,13 +603,10 @@ describe('src/cy/commands/actions/trigger', () => {
it('passes config.animationDistanceThreshold to cy.ensureElementIsNotAnimating', () => {
const animationDistanceThreshold = Cypress.config('animationDistanceThreshold')

const $btn = cy.$$('button:first')

const { fromElWindow } = Cypress.dom.getElementCoordinatesByPosition($btn)

cy.spy(cy, 'ensureElementIsNotAnimating')

cy.get('button:first').trigger('mouseover').then(() => {
cy.get('button:first').trigger('mouseover').then(($btn) => {
const { fromElWindow } = Cypress.dom.getElementCoordinatesByPosition($btn)
const { args } = cy.ensureElementIsNotAnimating.firstCall

expect(args[1]).to.deep.eq([fromElWindow, fromElWindow])
Expand Down
14 changes: 4 additions & 10 deletions packages/driver/cypress/integration/commands/actions/type_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -351,13 +351,10 @@ describe('src/cy/commands/actions/type - #type', () => {
})

it('passes options.animationDistanceThreshold to cy.ensureElementIsNotAnimating', () => {
const $txt = cy.$$(':text:first')

const { fromElWindow } = Cypress.dom.getElementCoordinatesByPosition($txt)

cy.spy(cy, 'ensureElementIsNotAnimating')

cy.get(':text:first').type('foo', { animationDistanceThreshold: 1000 }).then(() => {
cy.get(':text:first').type('foo', { animationDistanceThreshold: 1000 }).then(($txt) => {
const { fromElWindow } = Cypress.dom.getElementCoordinatesByPosition($txt)
const { args } = cy.ensureElementIsNotAnimating.firstCall

expect(args[1]).to.deep.eq([fromElWindow, fromElWindow])
Expand All @@ -369,13 +366,10 @@ describe('src/cy/commands/actions/type - #type', () => {
it('passes config.animationDistanceThreshold to cy.ensureElementIsNotAnimating', () => {
const animationDistanceThreshold = Cypress.config('animationDistanceThreshold')

const $txt = cy.$$(':text:first')

const { fromElWindow } = Cypress.dom.getElementCoordinatesByPosition($txt)

cy.spy(cy, 'ensureElementIsNotAnimating')

cy.get(':text:first').type('foo').then(() => {
cy.get(':text:first').type('foo').then(($txt) => {
const { fromElWindow } = Cypress.dom.getElementCoordinatesByPosition($txt)
const { args } = cy.ensureElementIsNotAnimating.firstCall

expect(args[1]).to.deep.eq([fromElWindow, fromElWindow])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,18 @@ context('cy.origin navigation', () => {
cy.get('a[data-cy="dom-link"]').click()

cy.origin('http://foobar.com:3500', () => {
cy.get(':checkbox[name="colors"][value="blue"]').check().should('be.checked')
cy.window().then((localWindow) => {
// Define a custom property on window that we can assert the presence of.
// After reloading, this property should not exist.
// @ts-ignore
localWindow.cy_testCustomValue = true
})

cy.window().should('have.prop', 'cy_testCustomValue', true)

cy.reload()
cy.get(':checkbox[name="colors"][value="blue"]').should('not.be.checked')

cy.window().should('not.have.prop', 'cy_testCustomValue', true)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Firefox now caches input values between reloads, causing this test to fail. Updated to use a window prop that we can be certain will be reset between reloads.

})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@ context('cy.origin spies, stubs, and clock', () => {
cy.get('a[data-cy="cross-origin-secondary-link"]').click()
})

afterEach(() => {
// FIXME: Tests that end with a cy.origin command and enqueue no further cy
// commands may have origin's unload event bleed into subsequent tests
// and prevent stability from being reached, causing those tests to hang.
// We enqueue another cy command after each test to ensure stability
// is reached for the next test. This additional command can be removed with the
// completion of: https://github.com/cypress-io/cypress/issues/21300
cy.then(() => { /* ensuring stability */ })
})

it('spy()', () => {
cy.origin('http://foobar.com:3500', () => {
const foo = { bar () { } }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ context('cy.origin unsupported commands', () => {
cy.get('a[data-cy="cross-origin-secondary-link"]').click()
})

afterEach(() => {
// FIXME: Tests that end with a cy.origin command and enqueue no further cy
// commands may have origin's unload event bleed into subsequent tests
// and prevent stability from being reached, causing those tests to hang.
// We enqueue another cy command after each test to ensure stability
// is reached for the next test. This additional command can be removed with the
// completion of: https://github.com/cypress-io/cypress/issues/21300
cy.then(() => { /* ensuring stability */ })
})

it('cy.route() method is deprecated', (done) => {
cy.on('fail', (err) => {
expect(err.message).to.equal('`cy.route()` has been deprecated and its use is not supported in the `cy.origin()` callback. Consider using `cy.intercept()` (outside of the callback) instead.')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,6 @@
error: 'cy-origin-error',
}

beforeEach(() => {
cy.visit('/fixtures/primary-origin.html')
cy.get('a[data-cy="cross-origin-secondary-link"]').click()
})

afterEach(() => {
// @ts-ignore
window.top.__cySkipValidateConfig = true
Expand Down
10 changes: 10 additions & 0 deletions packages/driver/cypress/integration/e2e/origin/cypress_api.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@ describe('cy.origin Cypress API', () => {
cy.get('a[data-cy="cross-origin-secondary-link"]').click()
})

afterEach(() => {
// FIXME: Tests that end with a cy.origin command and enqueue no further cy
// commands may have origin's unload event bleed into subsequent tests
// and prevent stability from being reached, causing those tests to hang.
// We enqueue another cy command after each test to ensure stability
// is reached for the next test. This additional command can be removed with the
// completion of: https://github.com/cypress-io/cypress/issues/21300
cy.then(() => { /* ensuring stability */ })
})

context('Commands', () => {
it('adds a custom command', () => {
cy.origin('http://foobar.com:3500', () => {
Expand Down
16 changes: 14 additions & 2 deletions packages/driver/cypress/integration/e2e/origin/logging.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,13 @@ describe('cy.origin logging', () => {

expect(originLog.groupStart).to.be.true

done()
// FIXME: Tests that end with a cy.origin command and enqueue no further cy
// commands may have origin's unload event bleed into subsequent tests
// and prevent stability from being reached, causing those tests to hang.
// We execute done after a brief timeout to ensure stability
// is reached for the next test. This timeout can be removed with the
// completion of: https://github.com/cypress-io/cypress/issues/21300
setTimeout(done, 0)
})

cy.visit('/fixtures/primary-origin.html')
Expand All @@ -64,7 +70,13 @@ describe('cy.origin logging', () => {

expect(originLog.groupStart).to.be.true

done()
// FIXME: Tests that end with a cy.origin command and enqueue no further cy
// commands may have origin's unload event bleed into subsequent tests
// and prevent stability from being reached, causing those tests to hang.
// We execute done after a brief timeout to ensure stability
// is reached for the next test. This timeout can be removed with the
// completion of: https://github.com/cypress-io/cypress/issues/21300
setTimeout(done, 0)
})

cy.visit('/fixtures/primary-origin.html')
Expand Down
13 changes: 12 additions & 1 deletion packages/driver/cypress/integration/e2e/origin/origin.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,14 @@
describe('cy.origin', () => {
afterEach(() => {
// FIXME: Tests that end with a cy.origin command and enqueue no further cy
// commands may have origin's unload event bleed into subsequent tests
// and prevent stability from being reached, causing those tests to hang.
// We enqueue another cy command after each test to ensure stability
// is reached for the next test. This additional command can be removed with the
// completion of: https://github.com/cypress-io/cypress/issues/21300
cy.then(() => { /* ensuring stability */ })
})

it('passes viewportWidth/Height state to the secondary origin', () => {
const expectedViewport = [320, 480]

Expand Down Expand Up @@ -237,12 +247,13 @@ describe('cy.origin', () => {
expect(err.message).not.to.include(`The following error originated from your application code, not from Cypress`)
expect(err.codeFrame).to.exist
expect(err.codeFrame!.frame).to.include('cy.origin')

done()
})

const variable = () => {}

cy.origin('http://idp.com:3500', { args: variable }, (variable) => {
cy.origin('http://foobar.com:3500', { args: variable }, (variable) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This spec would hang across all browsers when visiting idp.com; switching to foobar.com like all other specs in this file caused the test to pass (well, fail in the way we expect it too). This is odd, but not sure if it's indicative of a larger problem or the same problem recorded in #21300.

variable()
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ describe('cy.origin - uncaught errors', () => {
cy.get('a[data-cy="errors-link"]').click()
})

afterEach(() => {
// FIXME: Tests that end with a cy.origin command and enqueue no further cy
// commands may have origin's unload event bleed into subsequent tests
// and prevent stability from being reached, causing those tests to hang.
// We enqueue another cy command after each test to ensure stability
// is reached for the next test. This additional command can be removed with the
// completion of: https://github.com/cypress-io/cypress/issues/21300
cy.then(() => { /* ensuring stability */ })
})

describe('sync errors', () => {
it('appropriately reports negative assertions', (done) => {
cy.on('fail', (err) => {
Expand Down
10 changes: 10 additions & 0 deletions packages/driver/cypress/integration/e2e/origin/yield.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@ describe('cy.origin yields', () => {
cy.get('a[data-cy="cross-origin-secondary-link"]').click()
})

afterEach(() => {
// FIXME: Tests that end with a cy.origin command and enqueue no further cy
// commands may have origin's unload event bleed into subsequent tests
// and prevent stability from being reached, causing those tests to hang.
// We enqueue another cy command after each test to ensure stability
// is reached for the next test. This additional command can be removed with the
// completion of: https://github.com/cypress-io/cypress/issues/21300
cy.then(() => { /* ensuring stability */ })
})

it('yields a value', () => {
cy.origin('http://foobar.com:3500', () => {
cy
Expand Down
7 changes: 4 additions & 3 deletions packages/driver/src/cy/commands/actions/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,9 +360,10 @@ export default function (Commands, Cypress, cy, state, config) {
// We don't want to send it twice.
!Cypress.isBrowser('firefox') ||
// After Firefox 98,
// it sends a click event automatically if the element is a <button>
// it does not if the element is an <input>
(!isFirefoxBefore98 && $elements.isInput(event.target))
// it sends a click event automatically if the element is a <button>,
// but it does not if the element is an <input>.
// event.target is null when used with shadow DOM.
flotwig marked this conversation as resolved.
Show resolved Hide resolved
(!isFirefoxBefore98 && event.target && $elements.isInput(event.target))
) &&
// Click event is sent after keyup event with space key.
event.type === 'keyup' && event.code === 'Space' &&
Expand Down
1 change: 1 addition & 0 deletions packages/electron/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ Upgrading `electron` involves more than just bumping this package's `package.jso
- 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` and `browsers` family images matching the Node.js and Chromium versions in Electron.** The `browsers` image will be used for CI and the `base` will be used for any new `included` family images.
- The latest version of Firefox should also be included.
- See [`cypress-docker-images`](https://github.com/cypress-io/cypress-docker-images/).
- [ ] **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
Expand Down
2 changes: 1 addition & 1 deletion packages/electron/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"minimist": "1.2.6"
},
"devDependencies": {
"electron": "15.5.1",
"electron": "18.0.4",
"electron-packager": "15.4.0",
"execa": "4.1.0",
"mocha": "3.5.3"
Expand Down
Loading