Skip to content

Commit

Permalink
feat: Fix detached DOM errors for all Cypress commands (#24417)
Browse files Browse the repository at this point in the history
* First stab at removing old .get() implementation

* Fix TS and a couple of tests

* Fix tests and TS

* Fix case-sensitivity for .contains()

* Stop TS complaining

* Rework cy-contains jquery expression

* Add comments, make ts happy

* Fix one test, review feedback

* Review updates

* Fix additional tests

* Fix accidental deletion of vital code

* One more try at getting logs right

* Fix race condition in cross-origin .contains

* Add commented out test to ensure .within() works properly with selectors

* Fix for sessions + query subject chaining

* Fix mixing .within() shadow DOM and .contains() in same chainer

* One more attempt at .within + .contains

* Fix rebase commits

* Update many commands to be queries; improve log message around invalid subjects

* Update connectors, location, focused and window commands to queries

* Return noop to a command and not a query (to avoid implicit assertions)

* More test fixes

* Fix test failures

* Fix for weird-ass frontend-component test

* Error message improvements

* Fix for broken system test

* Update withinSubject to use subject chain

* Test clarifications

* Unbreak cypress-testing-library via withinState backwards compatibility

* Typo in last commit

* Improvement for assertion following failed traversal

* WIP adding query support to

* More work on actionability + detached dom

* Fix TS, rename _addQuery to addQuery

* Another try to fix types

* Fix lint

* Fix for bad merge

* Fixes for a couple more tests

* Increase timeout 50ms -> 100ms on certain tests failing in CI

* Switch to new branch of cypress-testing-library

* Update lockfile

* Fix yarn.lock with latest version of forked testing-library

* More test fixes

* Fix TS again

* Increase test assertion timeout so it passes on slow browsers (webkit)

* Apply suggestions from code review

Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>
Co-authored-by: Zach Bloomquist <git@chary.us>

* More review changes

* Fix selectFile tests based on updated error message

* Improve types and type comments for Commands.add

* Undo change to Commands.add types

* Update yarn lockfiles again

* Remove overwriteQuery from Cy12; .focused() now respects passed in timeout

* Update cli/types/cypress.d.ts

Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com>

* Restore .uncheck() tests

Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>
Co-authored-by: Zach Bloomquist <git@chary.us>
Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com>
  • Loading branch information
4 people authored Nov 9, 2022
1 parent 2e35f2a commit 45953f7
Show file tree
Hide file tree
Showing 65 changed files with 776 additions and 563 deletions.
3 changes: 1 addition & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ mainBuildFilters: &mainBuildFilters
branches:
only:
- develop
- fix-ci-deps
- issue-23843_electron_21_upgrade
- issue-7306

# usually we don't build Mac app - it takes a long time
# but sometimes we want to really confirm we are doing the right thing
Expand Down
71 changes: 68 additions & 3 deletions cli/types/cypress.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ declare namespace Cypress {
interface CommandFnWithOriginalFnAndSubject<T extends keyof Chainable, S> {
(this: Mocha.Context, originalFn: CommandOriginalFnWithSubject<T, S>, prevSubject: S, ...args: Parameters<ChainableMethods[T]>): ReturnType<ChainableMethods[T]> | void
}
interface QueryFn<T extends keyof ChainableMethods> {
(...args: Parameters<ChainableMethods[T]>): (subject: any) => any
}
interface ObjectLike {
[key: string]: any
}
Expand Down Expand Up @@ -462,30 +465,92 @@ declare namespace Cypress {
*/
log(options: Partial<LogConfig>): Log

/**
* @see https://on.cypress.io/api/commands
*/
Commands: {
/**
* Add a custom command
* @see https://on.cypress.io/api/commands
*/
add<T extends keyof Chainable>(name: T, fn: CommandFn<T>): void

/**
* Add a custom parent command
* @see https://on.cypress.io/api/commands#Parent-Commands
*/
add<T extends keyof Chainable>(name: T, options: CommandOptions & {prevSubject: false}, fn: CommandFn<T>): void

/**
* Add a custom child command
* @see https://on.cypress.io/api/commands#Child-Commands
*/
add<T extends keyof Chainable, S = any>(name: T, options: CommandOptions & {prevSubject: true}, fn: CommandFnWithSubject<T, S>): void

/**
* Add a custom child or dual command
* @see https://on.cypress.io/api/commands#Validations
*/
add<T extends keyof Chainable, S extends PrevSubject>(
name: T, options: CommandOptions & { prevSubject: S | ['optional'] }, fn: CommandFnWithSubject<T, PrevSubjectMap[S]>,
): void

/**
* Add a custom command that allows multiple types as the prevSubject
* @see https://on.cypress.io/api/commands#Validations#Allow-Multiple-Types
*/
add<T extends keyof Chainable, S extends PrevSubject>(
name: T, options: CommandOptions & { prevSubject: S[] }, fn: CommandFnWithSubject<T, PrevSubjectMap<void>[S]>,
): void

/**
* Add one or more custom commands
* @see https://on.cypress.io/api/commands
*/
addAll<T extends keyof Chainable>(fns: CommandFns): void

/**
* Add one or more custom parent commands
* @see https://on.cypress.io/api/commands#Parent-Commands
*/
addAll<T extends keyof Chainable>(options: CommandOptions & {prevSubject: false}, fns: CommandFns): void

/**
* Add one or more custom child commands
* @see https://on.cypress.io/api/commands#Child-Commands
*/
addAll<T extends keyof Chainable, S = any>(options: CommandOptions & { prevSubject: true }, fns: CommandFnsWithSubject<S>): void

/**
* Add one or more custom commands that validate their prevSubject
* @see https://on.cypress.io/api/commands#Validations
*/
addAll<T extends keyof Chainable, S extends PrevSubject>(
options: CommandOptions & { prevSubject: S | ['optional'] }, fns: CommandFnsWithSubject<PrevSubjectMap[S]>,
): void

/**
* Add one or more custom commands that allow multiple types as their prevSubject
* @see https://on.cypress.io/api/commands#Allow-Multiple-Types
*/
addAll<T extends keyof Chainable, S extends PrevSubject>(
options: CommandOptions & { prevSubject: S[] }, fns: CommandFnsWithSubject<PrevSubjectMap<void>[S]>,
): void

/**
* Overwrite an existing Cypress command with a new implementation
* @see https://on.cypress.io/api/commands#Overwrite-Existing-Commands
*/
overwrite<T extends keyof Chainable>(name: T, fn: CommandFnWithOriginalFn<T>): void

/**
* Overwrite an existing Cypress command with a new implementation
* @see https://on.cypress.io/api/commands#Overwrite-Existing-Commands
*/
overwrite<T extends keyof Chainable, S extends PrevSubject>(name: T, fn: CommandFnWithOriginalFnAndSubject<T, PrevSubjectMap[S]>): void

/**
* Add a custom query
* @see https://on.cypress.io/api/commands#Queries
*/
addQuery<T extends keyof Chainable>(name: T, fn: QueryFn<T>): void
}

/**
Expand Down
3 changes: 1 addition & 2 deletions packages/app/cypress/e2e/specs_list_e2e.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,7 @@ describe('App: Spec List (E2E)', () => {
cy.findByText('No specs matched your search:').should('not.be.visible')
})

// TODO: fix flaky test https://github.com/cypress-io/cypress/issues/23305
it.skip('saves the filter when navigating to a spec and back', function () {
it('saves the filter when navigating to a spec and back', function () {
const targetSpecFile = 'accounts_list.spec.js'

clearSearchAndType(targetSpecFile)
Expand Down
10 changes: 5 additions & 5 deletions packages/app/cypress/e2e/top-nav.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ describe('App Top Nav Workflows', () => {
})

it('hides dropdown when version in header is clicked', () => {
cy.findByTestId('cypress-update-popover').findByRole('button', { expanded: false }).as('topNavVersionButton').click()
cy.findByTestId('cypress-update-popover').findAllByRole('button').first().as('topNavVersionButton').click()

cy.get('@topNavVersionButton').should('have.attr', 'aria-expanded', 'true')

Expand Down Expand Up @@ -541,7 +541,7 @@ describe('App Top Nav Workflows', () => {
cy.findByRole('button', { name: 'Log in' }).click()
})

cy.findByRole('dialog', { name: 'Log in to Cypress' }).within(() => {
cy.findByRole('dialog').within(() => {
cy.findByRole('button', { name: 'Log in' }).click()

cy.contains('http://127.0.0.1:0000/redirect-to-auth').should('be.visible')
Expand Down Expand Up @@ -573,7 +573,7 @@ describe('App Top Nav Workflows', () => {
cy.findByRole('button', { name: 'Log in' }).click()
})

cy.findByRole('dialog', { name: 'Log in to Cypress' }).within(() => {
cy.findByRole('dialog').within(() => {
cy.findByRole('button', { name: 'Log in' }).click()

cy.contains(loginText.titleFailed).should('be.visible')
Expand Down Expand Up @@ -623,7 +623,7 @@ describe('App Top Nav Workflows', () => {
cy.findByRole('button', { name: 'Log in' }).as('loginButton').click()
})

cy.findByRole('dialog', { name: 'Log in to Cypress' }).within(() => {
cy.findByRole('dialog').within(() => {
cy.findByRole('button', { name: 'Log in' }).click()

cy.contains(loginText.titleFailed).should('be.visible')
Expand Down Expand Up @@ -660,7 +660,7 @@ describe('App Top Nav Workflows', () => {
cy.findByRole('button', { name: 'Log in' }).as('loginButton').click()
})

cy.findByRole('dialog', { name: 'Log in to Cypress' }).within(() => {
cy.findByRole('dialog').within(() => {
cy.findByRole('button', { name: 'Log in' }).click()
cy.contains(loginText.titleFailed).should('be.visible')
cy.contains(loginText.bodyError).should('be.visible')
Expand Down
2 changes: 1 addition & 1 deletion packages/app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"@intlify/vite-plugin-vue-i18n": "2.4.0",
"@packages/frontend-shared": "0.0.0-development",
"@percy/cypress": "^3.1.0",
"@testing-library/cypress": "8.0.0",
"@testing-library/cypress": "BlueWinds/cypress-testing-library#119054b5963b0d2e064b13c5cc6fc9db32c8b7b5",
"@types/faker": "5.5.8",
"@urql/core": "2.4.4",
"@urql/vue": "0.6.2",
Expand Down
25 changes: 23 additions & 2 deletions packages/driver/cypress/e2e/commands/actions/check.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,27 @@ describe('src/cy/commands/actions/check', () => {
cy.get(checkbox).check()
})

it('requeries if the DOM rerenders during actionability', () => {
cy.$$('[name=colors]').first().prop('disabled', true)

const listener = _.after(3, () => {
cy.$$('[name=colors]').first().prop('disabled', false)

const parent = cy.$$('[name=colors]').parent()

parent.replaceWith(parent[0].outerHTML)
cy.off('command:retry', listener)
})

cy.on('command:retry', listener)

cy.get('[name=colors]').check().then(($inputs) => {
$inputs.each((i, el) => {
expect($(el)).to.be.checked
})
})
})

// readonly should only be limited to inputs, not checkboxes
it('can check readonly checkboxes', () => {
cy.get('#readonly-checkbox').check().then(($checkbox) => {
Expand Down Expand Up @@ -437,7 +458,7 @@ describe('src/cy/commands/actions/check', () => {

cy.on('fail', (err) => {
expect(checked).to.eq(1)
expect(err.message).to.include('`cy.check()` failed because this element')
expect(err.message).to.include('`cy.check()` failed because the page updated')

done()
})
Expand Down Expand Up @@ -1079,7 +1100,7 @@ describe('src/cy/commands/actions/check', () => {

cy.on('fail', (err) => {
expect(unchecked).to.eq(1)
expect(err.message).to.include('`cy.uncheck()` failed because this element')
expect(err.message).to.include('`cy.uncheck()` failed because the page updated')

done()
})
Expand Down
22 changes: 21 additions & 1 deletion packages/driver/cypress/e2e/commands/actions/clear.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,26 @@ describe('src/cy/commands/actions/type - #clear', () => {
})
})

it('requeries if the DOM rerenders during actionability', () => {
const clicked = cy.stub()
const retried = cy.stub()

const textarea = cy.$$('#comments').val('foo bar').prop('disabled', true)

cy.on('command:retry', _.after(3, () => {
if (!retried.callCount) {
textarea.replaceWith(textarea[0].outerHTML)
cy.$$('#comments').prop('disabled', false).on('click', clicked)
retried()
}
}))

cy.get('#comments').clear().then(() => {
expect(clicked).to.be.calledOnce
expect(retried).to.be.called
})
})

it('can force clear even when being covered by another element', () => {
const $input = $('<input />')
.attr('id', 'input-covered-in-span')
Expand Down Expand Up @@ -275,7 +295,7 @@ describe('src/cy/commands/actions/type - #clear', () => {

cy.on('fail', (err) => {
expect(cleared).to.be.calledOnce
expect(err.message).to.include('`cy.clear()` failed because this element')
expect(err.message).to.include('`cy.clear()` failed because the page updated')

done()
})
Expand Down
Loading

0 comments on commit 45953f7

Please sign in to comment.