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

feat: _addQuery() #23665

Merged
merged 32 commits into from Sep 28, 2022
Merged

feat: _addQuery() #23665

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
0cc8b88
feat: Commands.addSelector, and migrate .get() to be a selector
BlueWinds Sep 1, 2022
0379058
Merge remote-tracking branch 'origin/develop' into issue-7306-addSele…
BlueWinds Sep 1, 2022
fc1bdb2
Merge remote-tracking branch 'origin/develop' into issue-7306-addSele…
BlueWinds Sep 6, 2022
e0ebba5
Fix for failed tests
BlueWinds Sep 6, 2022
b9be83b
Last test fix
BlueWinds Sep 6, 2022
c4de5d9
More test fixes
BlueWinds Sep 6, 2022
518bd72
Self review changes
BlueWinds Sep 6, 2022
e33e29f
Remove the concept of prevSubject from selectors entirely
BlueWinds Sep 7, 2022
1876e83
Merge remote-tracking branch 'origin/develop' into issue-7306-addSele…
BlueWinds Sep 7, 2022
789f71b
Rename addSelector to addQuery
BlueWinds Sep 7, 2022
3857845
Quick fix for last commit
BlueWinds Sep 7, 2022
a52b361
Fix TS
BlueWinds Sep 7, 2022
2ffa3c5
Fix merge from develop
BlueWinds Sep 7, 2022
661c305
Add types and other review updates
BlueWinds Sep 13, 2022
0009d3c
Merge branch 'develop' into issue-7306-addSelector-redux
Sep 13, 2022
443b636
Increase timeout to try fixing flakiness
BlueWinds Sep 14, 2022
caff894
Merge branch 'issue-7306-addSelector-redux' of github.com:cypress-io/…
BlueWinds Sep 14, 2022
ef57371
Rename addQuery to _addQuery
BlueWinds Sep 20, 2022
f9da4bb
Fix typo in previous commit
BlueWinds Sep 20, 2022
5d6e930
Merge branch 'develop' into issue-7306-addSelector-redux
BlueWinds Sep 20, 2022
98042e8
Merge remote-tracking branch 'origin/develop' into issue-7306-addSele…
BlueWinds Sep 26, 2022
5ce6250
Fix TS
BlueWinds Sep 26, 2022
14aa153
Include AUT assertion in cy.get()
BlueWinds Sep 26, 2022
1b4ebb8
Fix for previous commit
BlueWinds Sep 26, 2022
cd263af
Review feedback
BlueWinds Sep 26, 2022
7a88624
Merge branch 'develop' into issue-7306-addSelector-redux
Sep 26, 2022
dd05b4f
Minor test improvement
BlueWinds Sep 26, 2022
1ec423c
Swifter failure on sizzle syntax error
BlueWinds Sep 26, 2022
ec4a682
Better solution for refetching current subject in verifyUpcomingAsser…
BlueWinds Sep 26, 2022
514233a
Command IDs now include their chainerId
BlueWinds Sep 26, 2022
5a2db61
Merge branch 'develop' into issue-7306-addSelector-redux
Sep 27, 2022
9263b69
Merge branch 'develop' into issue-7306-addSelector-redux
Sep 28, 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
8 changes: 0 additions & 8 deletions packages/driver/cypress/e2e/commands/agents.cy.js
View file
Open in desktop
Expand Up @@ -323,10 +323,6 @@ describe('src/cy/commands/agents', () => {
expect(cy.state('aliases').myStub).to.exist
})

it('stores the agent as the subject', function () {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We already have tests around the user-facing behavior - this test asserting on the internal state that leads to this behavior is unnecessary.

Copy link
Member

Choose a reason for hiding this comment

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

@BlueWinds Can you link that test?

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 rest of this file is asserting on the output of the alias (eg, that we can store and retrieve aliases). It's not a specific test, but the whole file. 🤷‍♀️

expect(cy.state('aliases').myStub.subject).to.eq(this.stub)
})

it('assigns subject to runnable ctx', function () {
expect(this.myStub).to.eq(this.stub)
})
Expand Down Expand Up @@ -404,10 +400,6 @@ describe('src/cy/commands/agents', () => {
expect(cy.state('aliases')['my.stub']).to.exist
})

it('stores the agent as the subject', function () {
expect(cy.state('aliases')['my.stub'].subject).to.eq(this.stub)
})

it('assigns subject to runnable ctx', function () {
expect(this['my.stub']).to.eq(this.stub)
})
Expand Down
261 changes: 70 additions & 191 deletions packages/driver/cypress/e2e/commands/aliasing.cy.js
View file
Open in desktop
@@ -1,20 +1,12 @@
const { assertLogLength } = require('../../support/utils')
const { _, $ } = Cypress
const { _ } = Cypress

describe('src/cy/commands/aliasing', () => {
beforeEach(() => {
cy.visit('/fixtures/dom.html')
})

context('#as', () => {
it('is special utility command', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aliasing is no longer a 'special utility command', it exists in the command queue and runs the same as any other.

cy.wrap('foo').as('f').then(() => {
const cmd = cy.queue.find({ name: 'as' })

expect(cmd.get('type')).to.eq('utility')
})
})

it('does not change the subject', () => {
const body = cy.$$('body')

Expand All @@ -34,11 +26,13 @@ describe('src/cy/commands/aliasing', () => {
cy.get('@body')
})

it('stores the resulting subject as the alias', () => {
const $body = cy.$$('body')

it('stores the resulting subject chain as the alias', () => {
cy.get('body').as('b').then(() => {
expect(cy.state('aliases').b.subject.get(0)).to.eq($body.get(0))
const { subjectChain } = cy.state('aliases').b

expect(subjectChain.length).to.eql(2)
expect(subjectChain[0]).to.be.undefined
Copy link
Member

Choose a reason for hiding this comment

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

Why is the first subject chain undefined? Shouldn't the only subject chain be .get()? Shouldn't we assert the second subject chain is actually .get()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Subject chains always start with a "primitive" value, and then append any number of functions that then chain off this value.

Consider these two cases:

cy.get('button').contains('Login')

cy.get('button').click().contains('Login')

In the first case, we make a subject chain like [undefined, getFn(), containsFn()]. In the second one, we start with that same value, but .click() replaces the subject chain rather than adds to it - so we end up with [<button>, containsFn()].

I start subject chains with 'undefined' so that they always have a consistent shape: a primitive value, followed by 0 or more functions that are applied to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the second part - yes, yes we should assert that the second entry is .get(). Added an assertion.

// subjectChain[1] is the selector function for .get()
})
})

Expand All @@ -50,6 +44,15 @@ describe('src/cy/commands/aliasing', () => {
})
})

it('retries previous commands invoked inside custom commands', () => {
Cypress.Commands.add('get2', (selector) => cy.get(selector))

cy.get2('body').children('div').as('divs')
cy.visit('/fixtures/dom.html')

cy.get('@divs')
})

it('retries primitives and assertions', () => {
const obj = {}

Expand Down Expand Up @@ -87,29 +90,13 @@ describe('src/cy/commands/aliasing', () => {
})
})

context('DOM subjects', () => {
it('assigns the remote jquery instance', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the alias is now uses subject chains - which invoke the aliased commands directly - we've effectively made "using the correct jquery instance" the responsibility of the executing commands (which all already handle it properly).

const obj = {}

const jquery = () => {
return obj
}

cy.state('jQuery', jquery)

cy.get('input:first').as('input').then(function () {
expect(this.input).to.eq(obj)
})
})

it('retries previous commands invoked inside custom commands', () => {
Cypress.Commands.add('get2', (selector) => cy.get(selector))
it('retries previous commands invoked inside custom commands', () => {
Cypress.Commands.add('get2', (selector) => cy.get(selector))

cy.get2('body').children('div').as('divs')
cy.visit('/fixtures/dom.html')
cy.get2('body').children('div').as('divs')
cy.visit('/fixtures/dom.html')

cy.get('@divs')
})
cy.get('@divs')
})

context('#assign', () => {
Expand Down Expand Up @@ -328,9 +315,8 @@ describe('src/cy/commands/aliasing', () => {
// sanity check without command overwrite
cy.wrap('alias value').as('myAlias')
.then(() => {
expect(cy.getAlias('@myAlias'), 'alias exists').to.exist
expect(cy.getAlias('@myAlias'), 'alias value')
.to.have.property('subject', 'alias value')
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 series of tests asserts on the internal state of Cypress - I've changed the shape of how aliases are stored, so the tests needed updating. This is not a "user-facing" test update.

expect(cy.getAlias('@myAlias')).to.exist
expect(cy.getAlias('@myAlias').subjectChain).to.eql(['alias value'])
})
.then(() => {
// cy.get returns the alias
Expand All @@ -349,10 +335,9 @@ describe('src/cy/commands/aliasing', () => {

cy.wrap('alias value').as('myAlias')
.then(() => {
expect(wrapCalled, 'overwrite was called').to.be.true
expect(cy.getAlias('@myAlias'), 'alias exists').to.exist
expect(cy.getAlias('@myAlias'), 'alias value')
.to.have.property('subject', 'alias value')
expect(wrapCalled).to.be.true
expect(cy.getAlias('@myAlias')).to.exist
expect(cy.getAlias('@myAlias').subjectChain).to.eql(['alias value'])
})
.then(() => {
// verify cy.get works in arrow function
Expand Down Expand Up @@ -382,9 +367,8 @@ describe('src/cy/commands/aliasing', () => {
.then(() => {
expect(wrapCalled, 'overwrite was called').to.be.true
expect(thenCalled, 'then was called').to.be.true
expect(cy.getAlias('@myAlias'), 'alias exists').to.exist
expect(cy.getAlias('@myAlias'), 'alias value')
.to.have.property('subject', 'alias value')
expect(cy.getAlias('@myAlias')).to.exist
expect(cy.getAlias('@myAlias').subjectChain).to.eql(['alias value'])
})
.then(() => {
// verify cy.get works in arrow function
Expand All @@ -400,9 +384,9 @@ describe('src/cy/commands/aliasing', () => {
// sanity test before the next one
cy.wrap(1).as('myAlias')
cy.wrap(2).then(function (subj) {
expect(subj, 'subject').to.equal(2)
expect(this, 'this is defined').to.not.be.undefined
expect(this.myAlias, 'this has the alias as a property').to.eq(1)
expect(subj).to.equal(2)
expect(this).to.not.be.undefined
expect(this.myAlias).to.eq(1)
})
})

Expand All @@ -414,8 +398,8 @@ describe('src/cy/commands/aliasing', () => {

cy.wrap(1).as('myAlias')
cy.wrap(2).then(function (subj) {
expect(subj, 'subject').to.equal(2)
expect(this, 'this is defined').to.not.be.undefined
expect(subj).to.equal(2)
expect(this).to.not.be.undefined
expect(this.myAlias).to.eq(1)
})
})
Expand All @@ -428,166 +412,61 @@ describe('src/cy/commands/aliasing', () => {

cy.wrap(1).as('myAlias')
cy.wrap(2).then(function (subj) {
expect(subj, 'subject').to.equal(2)
expect(this, 'this is defined').to.not.be.undefined
expect(subj).to.equal(2)
expect(this).to.not.be.undefined
expect(this.myAlias).to.eq(1)
})
})
})
})

context('#replayCommandsFrom', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aliases no longer run through the command queue, so this set of tests doesn't make much sense anymore. They instead use subject chains to accomplish the same goal.

"A subject chain" is essentially the minimal set of information needed to "replay commands that led of to a current subject" - except now it's used everywhere in Cypress, so all commands can benefit from it, rather than as a special case in aliasing.

describe('subject in document', () => {
it('returns if subject is still in the document', () => {
cy.get('#list').as('list').then(() => {
const currentLength = cy.queue.length

cy.get('@list').then(() => {
// should only add the .get() and the .then()
expect(cy.queue.length).to.eq(currentLength + 2)
})
context('#replaying subjects', () => {
it('returns if subject is still in the document', () => {
cy.get('#list').as('list').then((firstList) => {
cy.get('@list').then((secondList) => {
expect(firstList).to.eql(secondList)
})
})
})

describe('subject not in document', () => {
it('inserts into the queue', () => {
const existingNames = cy.queue.names()

cy
.get('#list li').eq(0).as('firstLi').then(($li) => {
return $li.remove()
})
.get('@firstLi').then(() => {
expect(cy.queue.names()).to.deep.eq(
existingNames.concat(
['get', 'eq', 'as', 'then', 'get', 'get', 'eq', 'then'],
),
)
})
})

it('replays from last root to current', () => {
const first = cy.$$('#list li').eq(0)
const second = cy.$$('#list li').eq(1)

cy
.get('#list li').eq(0).as('firstLi').then(($li) => {
expect($li.get(0)).to.eq(first.get(0))

return $li.remove()
})
.get('@firstLi').then(($li) => {
expect($li.get(0)).to.eq(second.get(0))
})
})

it('replays up until first root command', () => {
const existingNames = cy.queue.names()

cy
.get('body').noop({})
.get('#list li').eq(0).as('firstLi').then(($li) => {
return $li.remove()
})
.get('@firstLi').then(() => {
expect(cy.queue.names()).to.deep.eq(
existingNames.concat(
['get', 'noop', 'get', 'eq', 'as', 'then', 'get', 'get', 'eq', 'then'],
),
)
})
})

it('resets the chainerId allow subjects to be carried on', () => {
cy.get('#dom').find('#button').as('button').then(($button) => {
$button.remove()

cy.$$('#dom').append($('<button />', { id: 'button' }))

return null
})
it('requeries when reading alias', () => {
cy
.get('#list li')
.as('items').then((firstItems) => {
cy.$$('#list').append('<li class="foobar">123456789</li>')

// when cy is a separate chainer there *was* a bug
// that cause the subject to null because of different
// chainer id's
cy.get('@button').then(($button) => {
expect($button).to.have.id('button')
cy.get('@items').then((secondItems) => {
expect(firstItems).to.have.length(3)
expect(secondItems).to.have.length(4)
})
})
})

it('skips commands which did not change, and starts at the first valid subject or parent command', () => {
const existingNames = cy.queue.names()

cy.$$('#list li').click(function () {
const ul = $(this).parent()
const lis = ul.children().clone()

// this simulates a re-render
ul.children().remove()
ul.append(lis)

return lis.first().remove()
})
it('requeries when subject is not in the DOM', () => {
cy
.get('#list li')
.as('items').then((firstItems) => {
firstItems.remove()
setTimeout(() => {
cy.$$('#list').append('<li class="foobar">123456789</li>')
}, 50)

cy
.get('#list li')
.then(($lis) => {
return $lis
})
.as('items')
.first()
.click()
.as('firstItem')
.then(() => {
expect(cy.queue.names()).to.deep.eq(
existingNames.concat(
['get', 'then', 'as', 'first', 'click', 'as', 'then', 'get', 'should', 'then', 'get', 'should', 'then'],
),
)
})
.get('@items')
.should('have.length', 2)
.then(() => {
expect(cy.queue.names()).to.deep.eq(
existingNames.concat(
['get', 'then', 'as', 'first', 'click', 'as', 'then', 'get', 'get', 'should', 'then', 'get', 'should', 'then'],
),
)
})
.get('@firstItem')
.should('contain', 'li 1')
.then(() => {
expect(cy.queue.names()).to.deep.eq(
existingNames.concat(
['get', 'then', 'as', 'first', 'click', 'as', 'then', 'get', 'get', 'should', 'then', 'get', 'get', 'first', 'should', 'then'],
),
)
cy.get('@items').then((secondItems) => {
expect(secondItems).to.have.length(1)
})
})
})

it('inserts assertions', (done) => {
const existingNames = cy.queue.names()

cy
.get('#checkboxes input')
.eq(0)
.should('be.checked', 'cockatoo')
.as('firstItem')
.then(($input) => {
return $input.remove()
})
.get('@firstItem')
.then(() => {
expect(cy.queue.names()).to.deep.eq(
existingNames.concat(
['get', 'eq', 'should', 'as', 'then', 'get', 'get', 'eq', 'should', 'then'],
),
)

done()
})
})
it('only retries up to last action command', () => {
cy
.get('#list li')
.then((items) => items.length)
.as('itemCount')
.then(() => cy.$$('#list li').remove())

// Even though the list items have been removed from the DOM, 'then' can't be retried
// so we just have the primitive value "3" as our subject.
cy.get('@itemCount').should('eq', 3)
})
})

Expand Down