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.

Sign up for GitHub

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

Jump to bottom

fix: .within() now throws an error if given more than one subject. #24975

Merged
merged 7 commits into from Dec 5, 2022
Merged

fix: .within() now throws an error if given more than one subject. #24975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e4b033c
fix: .within() now throws an error if given more than one subject.
BlueWinds Dec 5, 2022
fa14e9d
Fix own tests for new .within() behavior
BlueWinds Dec 5, 2022
b36e41e
Merge branch 'develop' into within-single-subject
Dec 5, 2022
450c2b0
Another test fix
BlueWinds Dec 5, 2022
0217047
Merge branch 'within-single-subject' of github.com:cypress-io/cypress…
BlueWinds Dec 5, 2022
7e70b33
More fixes from launchpad
BlueWinds Dec 5, 2022
11b2684
Update packages/app/cypress/e2e/runs.cy.ts
Dec 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/app/cypress/e2e/runner/sessions.ui.cy.ts
View file
Open in desktop
Expand Up @@ -106,6 +106,7 @@ describe('runner/cypress sessions.ui.spec', {
validateSessionsInstrumentPanel(['blank_session'])

cy.get('.command-name-session')
.first()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are a couple of examples where our tests used to work, but required a change because of this PR. For example, this test had multiple commands - we now are explicitly (rather than implicitly) asserting on the first of them.

.within(() => {
cy.contains('blank_session')
cy.contains('failed')
Expand Down Expand Up @@ -301,6 +302,7 @@ describe('runner/cypress sessions.ui.spec', {
validateSessionsInstrumentPanel(['user1'])

cy.get('.command-name-session')
.first()
.within(() => {
cy.contains('failed')

Expand Down
5 changes: 4 additions & 1 deletion packages/app/cypress/e2e/runs.cy.ts
View file
Open in desktop
Expand Up @@ -941,7 +941,10 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {
cy.get('[data-cy="run-card-icon-RUNNING"]').should('have.length', 3).should('be.visible')
cy.wrap(obj).invoke('toCall')

cy.get('[data-cy="run-card-icon-PASSED"]').should('have.length', 3).should('be.visible').within(() => {
cy.get('[data-cy="run-card-icon-PASSED"]')
.should('have.length', 3)
.should('be.visible')
.first().within(() => {
cy.get('[data-cy="runResults-passed-count"]').should('contain', 100)
})

Expand Down
4 changes: 2 additions & 2 deletions packages/driver/cypress/e2e/commands/querying/root.cy.js
View file
Open in desktop
Expand Up @@ -17,7 +17,7 @@ describe('src/cy/commands/querying', () => {
it('returns withinSubject if exists', () => {
const form = cy.$$('form')

cy.get('form').within(() => {
cy.get('form').first().within(() => {
cy
.get('input')
.root().then(($root) => {
Expand Down Expand Up @@ -100,7 +100,7 @@ describe('src/cy/commands/querying', () => {
it('sets $el to withinSubject', () => {
const form = cy.$$('form')

cy.get('form').within(() => {
cy.get('form').first().within(() => {
cy
.get('input')
.root().then(function ($root) {
Expand Down
13 changes: 11 additions & 2 deletions packages/driver/cypress/e2e/commands/querying/within.cy.js
View file
Open in desktop
Expand Up @@ -228,7 +228,7 @@ describe('src/cy/commands/querying/within', () => {
})

it('provides additional information in console prop', () => {
cy.get('div').within(() => {})
cy.get('div').first().within(() => {})
cy.then(function () {
const { lastLog } = this

Expand All @@ -237,7 +237,6 @@ describe('src/cy/commands/querying/within', () => {
expect(consoleProps).to.be.an('object')
expect(consoleProps.Command).to.eq('within')
expect(consoleProps.Yielded).to.not.be.null
expect(consoleProps.Yielded).to.have.length(55)
})
})
})
Expand Down Expand Up @@ -304,6 +303,16 @@ describe('src/cy/commands/querying/within', () => {

cy.get('#list').within(() => {})
})

it('throws when given multiple subjects', (done) => {
cy.on('fail', (err) => {
expect(err.message).to.include('`cy.within()` can only be called on a single element. Your subject contained 9 elements.')

done()
})

cy.get('ul').within(() => {})
})
})
})

Expand Down
4 changes: 4 additions & 0 deletions packages/driver/src/cy/commands/querying/within.ts
View file
Open in desktop
Expand Up @@ -107,6 +107,10 @@ export default (Commands, Cypress, cy, state) => {
$errUtils.throwErrByPath('within.invalid_argument', { onFail: log })
}

if (subject.length > 1) {
$errUtils.throwErrByPath('within.multiple_elements', { args: { num: subject.length }, onFail: log })
}

return withinFn(subject, fn)
})
},
Expand Down
14 changes: 14 additions & 0 deletions packages/driver/src/cypress/error_messages.ts
View file
Open in desktop
Expand Up @@ -2404,6 +2404,20 @@ export default {
message: `${cmd('within')} must be called with a function.`,
docsUrl: 'https://on.cypress.io/within',
},
multiple_elements (args) {
return {
message: stripIndent`
${cmd('within')} can only be called on a single element. Your subject contained {{num}} elements. Narrow down your subject to a single element (using \`.first()\`, for example) before calling \`.within()\`.

To run \`.within()\` over multiple subjects, use \`.each()\`.

\`cy.get('div').each($div => {\`
\` cy.wrap($div).within(() => { ... })\`
\`})\`
`,
docsUrl: 'https://on.cypress.io/within',
}
},
},

wrap: {
Expand Down
60 changes: 24 additions & 36 deletions packages/launchpad/cypress/e2e/project-setup.cy.ts
View file
Open in desktop
Expand Up @@ -87,12 +87,10 @@ describe('Launchpad: Setup Project', () => {
cy.contains('h1', 'Configuration files')
cy.findByText('We added the following files to your project:')

cy.get('[data-cy=valid]').within(() => {
cy.contains('cypress.config.js')
cy.containsPath('cypress/support/e2e.js')
cy.containsPath('cypress/support/commands.js')
cy.containsPath('cypress/fixtures/example.json')
})
cy.get('[data-cy=valid]').as('valid').contains('cypress.config.js')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's an example of where we were genuinely relying on multiple subjects from .within(), and it required something more than just .first() to fix the test.

cy.get('@valid').containsPath('cypress/support/e2e.js')
cy.get('@valid').containsPath('cypress/support/commands.js')
cy.get('@valid').containsPath('cypress/fixtures/example.json')

cy.get('[data-cy=valid] [data-cy=collapsible-header]').each((element) => {
cy.wrap(element).should('have.attr', 'aria-expanded', 'false')
Expand Down Expand Up @@ -247,12 +245,10 @@ describe('Launchpad: Setup Project', () => {
cy.contains('h1', 'Configuration files')
cy.findByText('We added the following files to your project:')

cy.get('[data-cy=valid]').within(() => {
cy.contains('cypress.config.js')
cy.containsPath('cypress/support/e2e.js')
cy.containsPath('cypress/support/commands.js')
cy.containsPath('cypress/fixtures/example.json')
})
cy.get('[data-cy=valid]').as('valid').contains('cypress.config.js')
cy.get('@valid').containsPath('cypress/support/e2e.js')
cy.get('@valid').containsPath('cypress/support/commands.js')
cy.get('@valid').containsPath('cypress/fixtures/example.json')

cy.get('[data-cy=valid] [data-cy=collapsible-header]').each((element) => {
cy.wrap(element).should('have.attr', 'aria-expanded', 'false')
Expand All @@ -278,12 +274,10 @@ describe('Launchpad: Setup Project', () => {
cy.contains('h1', 'Configuration files')
cy.findByText('We added the following files to your project:')

cy.get('[data-cy=valid]').within(() => {
cy.contains('cypress.config.js')
cy.containsPath('cypress/support/e2e.js')
cy.containsPath('cypress/support/commands.js')
cy.containsPath('cypress/fixtures/example.json')
})
cy.get('[data-cy=valid]').as('valid').contains('cypress.config.js')
cy.get('@valid').containsPath('cypress/support/e2e.js')
cy.get('@valid').containsPath('cypress/support/commands.js')
cy.get('@valid').containsPath('cypress/fixtures/example.json')

verifyScaffoldedFiles('e2e')

Expand Down Expand Up @@ -317,12 +311,10 @@ describe('Launchpad: Setup Project', () => {
cy.contains('h1', 'Configuration files')
cy.findByText('We added the following files to your project:')

cy.get('[data-cy=valid]').within(() => {
cy.contains('cypress.config.js')
cy.containsPath('cypress/support/e2e.js')
cy.containsPath('cypress/support/commands.js')
cy.containsPath('cypress/fixtures/example.json')
})
cy.get('[data-cy=valid]').as('valid').contains('cypress.config.js')
cy.get('@valid').containsPath('cypress/support/e2e.js')
cy.get('@valid').containsPath('cypress/support/commands.js')
cy.get('@valid').containsPath('cypress/fixtures/example.json')

cy.get('[data-cy=valid] [data-cy=collapsible-header]').each((element) => {
cy.wrap(element).should('have.attr', 'aria-expanded', 'false')
Expand Down Expand Up @@ -353,12 +345,10 @@ describe('Launchpad: Setup Project', () => {
cy.contains('h1', 'Configuration files')
cy.findByText('We added the following files to your project:')

cy.get('[data-cy=valid]').within(() => {
cy.contains('cypress.config.ts')
cy.containsPath('cypress/support/e2e.ts')
cy.containsPath('cypress/support/commands.ts')
cy.containsPath('cypress/fixtures/example.json')
})
cy.get('[data-cy=valid]').as('valid').contains('cypress.config.ts')
cy.get('@valid').containsPath('cypress/support/e2e.ts')
cy.get('@valid').containsPath('cypress/support/commands.ts')
cy.get('@valid').containsPath('cypress/fixtures/example.json')

cy.get('[data-cy=valid] [data-cy=collapsible-header]').each((element) => {
cy.wrap(element).should('have.attr', 'aria-expanded', 'false')
Expand Down Expand Up @@ -410,12 +400,10 @@ describe('Launchpad: Setup Project', () => {

cy.findByRole('button', { name: 'Skip' }).click()

cy.get('[data-cy=valid]').within(() => {
cy.contains('cypress.config.js')
cy.containsPath('cypress/support/component-index.html')
cy.containsPath('cypress/support/component.js')
cy.containsPath('cypress/support/commands.js')
})
cy.get('[data-cy=valid]').as('valid').contains('cypress.config.js')
cy.get('@valid').containsPath('cypress/support/component-index.html')
cy.get('@valid').containsPath('cypress/support/component.js')
cy.get('@valid').containsPath('cypress/support/commands.js')

verifyScaffoldedFiles('component')
})
Expand Down