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

feat: Fix detached DOM errors for all Cypress commands #24417

Merged
merged 61 commits into from
Nov 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
3c74ac8
First stab at removing old .get() implementation
BlueWinds Sep 12, 2022
ed9765b
Fix TS and a couple of tests
BlueWinds Sep 12, 2022
6e408b3
Fix tests and TS
BlueWinds Sep 27, 2022
c27dae2
Fix case-sensitivity for .contains()
BlueWinds Sep 29, 2022
4bfdec8
Stop TS complaining
BlueWinds Sep 29, 2022
a881c11
Rework cy-contains jquery expression
BlueWinds Sep 29, 2022
4dc9134
Add comments, make ts happy
BlueWinds Sep 29, 2022
7a62f79
Fix one test, review feedback
BlueWinds Oct 3, 2022
a58e46b
Review updates
BlueWinds Oct 3, 2022
f62277a
Fix additional tests
BlueWinds Oct 3, 2022
ed852c2
Fix accidental deletion of vital code
BlueWinds Oct 3, 2022
e481858
One more try at getting logs right
BlueWinds Oct 3, 2022
5b73353
Fix race condition in cross-origin .contains
BlueWinds Oct 3, 2022
b1b7b38
Add commented out test to ensure .within() works properly with selectors
BlueWinds Oct 3, 2022
94f3d3f
Fix for sessions + query subject chaining
BlueWinds Oct 4, 2022
7fc0859
Fix mixing .within() shadow DOM and .contains() in same chainer
BlueWinds Oct 4, 2022
4b511da
One more attempt at .within + .contains
BlueWinds Oct 4, 2022
0eecaed
Fix rebase commits
BlueWinds Oct 5, 2022
aab9565
Update many commands to be queries; improve log message around invali…
BlueWinds Oct 5, 2022
6e69496
Update connectors, location, focused and window commands to queries
BlueWinds Oct 6, 2022
6a43c68
Return noop to a command and not a query (to avoid implicit assertions)
BlueWinds Oct 6, 2022
570b349
More test fixes
BlueWinds Oct 11, 2022
6776613
Merge branch 'issue-7306' into issue-7306-addQuery-traversal
BlueWinds Oct 11, 2022
a080940
Fix test failures
BlueWinds Oct 11, 2022
34fd50e
Fix for weird-ass frontend-component test
BlueWinds Oct 11, 2022
f8f89cd
Error message improvements
BlueWinds Oct 11, 2022
6a4a168
Merge branch 'issue-7306' into issue-7306-addQuery-traversal
BlueWinds Oct 12, 2022
4b1a0f6
Fix for broken system test
BlueWinds Oct 12, 2022
10bde23
Update withinSubject to use subject chain
BlueWinds Oct 12, 2022
14b5e23
Test clarifications
BlueWinds Oct 12, 2022
a675694
Unbreak cypress-testing-library via withinState backwards compatibility
BlueWinds Oct 12, 2022
e1e7eca
Typo in last commit
BlueWinds Oct 12, 2022
652fd06
Improvement for assertion following failed traversal
BlueWinds Oct 17, 2022
7cb9377
Merge branch 'issue-7306' into issue-7306-addQuery-traversal
BlueWinds Oct 17, 2022
7fe6442
Merge branch 'issue-7306' into issue-7306-addQuery-actionability
BlueWinds Oct 25, 2022
e982e05
WIP adding query support to
BlueWinds Oct 25, 2022
66262d4
More work on actionability + detached dom
BlueWinds Oct 25, 2022
7a29d46
Fix TS, rename _addQuery to addQuery
BlueWinds Oct 26, 2022
a051a28
Merge remote-tracking branch 'origin/issue-7306' into issue-7306-addQ…
BlueWinds Oct 26, 2022
9224571
Another try to fix types
BlueWinds Oct 26, 2022
ea6e11e
Fix lint
BlueWinds Oct 26, 2022
6a492c2
Fix for bad merge
BlueWinds Oct 27, 2022
3cb0f56
Fixes for a couple more tests
BlueWinds Oct 27, 2022
dfd440c
Increase timeout 50ms -> 100ms on certain tests failing in CI
BlueWinds Oct 27, 2022
e775659
Switch to new branch of cypress-testing-library
BlueWinds Oct 27, 2022
e8e5809
Update lockfile
BlueWinds Oct 31, 2022
62acc80
Fix yarn.lock with latest version of forked testing-library
BlueWinds Oct 31, 2022
b90ef7a
More test fixes
BlueWinds Oct 31, 2022
d037686
Fix TS again
BlueWinds Oct 31, 2022
2c5f801
Increase test assertion timeout so it passes on slow browsers (webkit)
BlueWinds Oct 31, 2022
d7526d8
Merge branch 'issue-7306' into issue-7306-addQuery-actionability
BlueWinds Oct 31, 2022
0bf6daa
Apply suggestions from code review
Nov 3, 2022
bf40c37
More review changes
BlueWinds Nov 3, 2022
4c56a53
Fix selectFile tests based on updated error message
BlueWinds Nov 3, 2022
ecb5aa9
Improve types and type comments for Commands.add
BlueWinds Nov 3, 2022
757953b
Undo change to Commands.add types
BlueWinds Nov 3, 2022
bf4dff7
Update yarn lockfiles again
BlueWinds Nov 7, 2022
ab3b3ed
Remove overwriteQuery from Cy12; .focused() now respects passed in ti…
BlueWinds Nov 8, 2022
17fe330
Update cli/types/cypress.d.ts
Nov 9, 2022
22b10f4
Restore .uncheck() tests
BlueWinds Nov 9, 2022
ed990f8
Merge branch 'issue-7306-addQuery-actionability' of github.com:cypres…
BlueWinds Nov 9, 2022
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
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
BlueWinds marked this conversation as resolved.
Show resolved Hide resolved

# 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

Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, kinda surprised that there aren't type comments for any of these.

Copy link
Member

Choose a reason for hiding this comment

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

Links to docs at least would be nice!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call - some comments here can definitely help make sense of the mess of TS definitions.

I'm adding links to a couple of anchors that don't exist yet:

  • #Queries
  • #Overwrite-Existing-Queries

I'll be creating these in the docs PR, which isn't ready yet.

/**
* 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 () {
BlueWinds marked this conversation as resolved.
Show resolved Hide resolved
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()
BlueWinds marked this conversation as resolved.
Show resolved Hide resolved

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",
Copy link
Contributor Author

@BlueWinds BlueWinds Oct 31, 2022

Choose a reason for hiding this comment

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

This is a fork of cypress-testing-library which uses queries. My current plan is to wait for this PR to merge into issue-7306, which will build the cypress binary; I can then update my fork of cypress-testing-library to point to that branch for its own tests, and open a PR upstream.

I was running into issues while pointing only to the fork / branch, where the node_modules cache wouldn't update when I updated my fork - pointing to a specific commit here in the monorepo helped.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Specific commit is always the way to go, IMO. Using a branch causes the caching issue/bug with yarn, plus it makes builds unpredictable since the branch's head commit can change at any time.

"@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