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: addQuery Remaining Queries #24203

Merged
merged 34 commits into from
Oct 26, 2022
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
34 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
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 packages/driver/cypress/e2e/commands/actions/scroll.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ describe('src/cy/commands/actions/scroll', () => {
context('subject errors', () => {
it('throws when not passed DOM element as subject', (done) => {
cy.on('fail', (err) => {
expect(err.message).to.include('`cy.scrollTo()` failed because it requires a DOM element.')
expect(err.message).to.include('`cy.scrollTo()` failed because it requires a DOM element or window.')
expect(err.message).to.include('{foo: bar}')
expect(err.message).to.include('> `cy.noop()`')

Expand Down
17 changes: 1 addition & 16 deletions packages/driver/cypress/e2e/commands/assertions.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,21 +134,6 @@ describe('src/cy/commands/assertions', () => {
cy.noop(obj).its('requestJSON').should('have.property', 'teamIds').should('deep.eq', [2])
})

// TODO: make cy.then retry
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the intervening years since this TODO was left, we've refined and better defined exactly what retries and what doesn't.

.then() will never retry, and should not, so removing the commented out tests.

// https://github.com/cypress-io/cypress/issues/627
it.skip('outer assertions retry on cy.then', () => {
const obj = { foo: 'bar' }

cy.wrap(obj).then(() => {
setTimeout(() => {
obj.foo = 'baz'
}
, 1000)

return obj
}).should('deep.eq', { foo: 'baz' })
})

it('does it retry when wrapped', () => {
const obj = { foo: 'bar' }

Expand Down Expand Up @@ -828,7 +813,7 @@ describe('src/cy/commands/assertions', () => {
return null
})

it('does not output should logs on failures', function (done) {
it('does not output should logs on failures', { defaultCommandTimeout: 50 }, function (done) {
cy.on('fail', () => {
const { length } = this.logs

Expand Down
148 changes: 29 additions & 119 deletions packages/driver/cypress/e2e/commands/connectors.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -628,8 +628,8 @@ describe('src/cy/commands/connectors', () => {
}

cy.on('fail', (err) => {
expect(err.message).to.include('Timed out retrying after 100ms: `cy.invoke()` errored because the property: `bar` returned a `string` value instead of a function. `cy.invoke()` can only be used on properties that return callable functions.')
expect(err.message).to.include('`cy.invoke()` waited for the specified property `bar` to return a function, but it never did.')
expect(err.message).to.include('Timed out retrying after 100ms: `cy.invoke()` errored because the property: `foo.bar` returned a `string` value instead of a function. `cy.invoke()` can only be used on properties that return callable functions.')
expect(err.message).to.include('`cy.invoke()` waited for the specified property `foo.bar` to return a function, but it never did.')
expect(err.message).to.include('If you want to assert on the property\'s value, then switch to use `cy.its()` and add an assertion such as:')
expect(err.message).to.include('`cy.wrap({ foo: \'bar\' }).its(\'foo\').should(\'eq\', \'bar\')`')
expect(err.docsUrl).to.eq('https://on.cypress.io/invoke')
Expand All @@ -642,7 +642,7 @@ describe('src/cy/commands/connectors', () => {
})
})

describe('accepts a options argument', () => {
describe('accepts an options argument', () => {
it('changes subject to function invocation', () => {
cy.noop({ foo () {
return 'foo'
Expand Down Expand Up @@ -729,16 +729,15 @@ describe('src/cy/commands/connectors', () => {

cy.wrap({ foo () {
return 'foo'
} }).invoke(() => {
return {}
})
} })
.invoke(() => {})
})

it('throws when first parameter is neither of type object nor of type string nor of type number', function (done) {
cy.on('fail', (err) => {
const { lastLog } = this

expect(err.message).to.include('`cy.invoke()` only accepts a string or a number as the functionName argument.')
expect(err.message).to.include('`cy.invoke()` only accepts an object as the options argument.')
expect(lastLog.get('error').message).to.include(err.message)

done()
Expand Down Expand Up @@ -903,30 +902,19 @@ describe('src/cy/commands/connectors', () => {
Command: 'invoke',
Function: '.bar()',
Subject: this.obj,
'With Arguments': [],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

.invoke() now always includes With Arguments, even if it's an empty array. I strongly prefer consistency in my values over special cases (eg, omit when it's empty).

Yielded: 'bar',
})
})
})

it('#consoleProps as a function property with args', function () {
cy.noop(this.obj).invoke('sum', 1, 2, 3).then(function () {
expect(this.lastLog.invoke('consoleProps')).to.deep.eq({
Command: 'invoke',
Function: '.sum(1, 2, 3)',
'With Arguments': [1, 2, 3],
Subject: this.obj,
Yielded: 6,
})
})
})

it('#consoleProps as a function reduced property with args', function () {
it('#consoleProps as a deep function property with args', function () {
cy.noop(this.obj).invoke('math.sum', 1, 2, 3).then(function () {
expect(this.lastLog.invoke('consoleProps')).to.deep.eq({
Command: 'invoke',
Function: '.math.sum(1, 2, 3)',
'With Arguments': [1, 2, 3],
Subject: this.obj['math'],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Behavior change: .invoke() now always puts the actual subject as the Subject in its console logs. The subject was this.obj, the property retrieved is .math.sum. IMO this is more consistent and intuitive.

Subject: this.obj,
Yielded: 6,
})
})
Expand All @@ -939,8 +927,9 @@ describe('src/cy/commands/connectors', () => {
expect(consoleProps).to.deep.eq({
Command: 'invoke',
Function: '.hide()',
Subject: $btn.get(0),
Yielded: $btn.get(0),
Subject: $btn,
'With Arguments': [],
Yielded: $btn,
})
})
})
Expand Down Expand Up @@ -1058,7 +1047,7 @@ describe('src/cy/commands/connectors', () => {
cy.on('fail', (err) => {
const { lastLog } = this

expect(err.message).to.include('Timed out retrying after 100ms: `cy.invoke()` errored because the property: `baz` does not exist on your subject.')
expect(err.message).to.include('Timed out retrying after 100ms: `cy.invoke()` errored because the property: `foo.bar.baz.fizz` does not exist on your subject.')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Behavior change: .invoke() now logs the full path when we tried to fetch it and failed. This makes it easier to debug and more consistent with the rest of Cypress' API design.

I think the fact it didn't used to work this way is more a quirk of implementation than any sort of thought-out-feature.

expect(lastLog.get('error').message).to.include(err.message)
expect(err.docsUrl).to.eq('https://on.cypress.io/invoke')

Expand All @@ -1081,10 +1070,8 @@ describe('src/cy/commands/connectors', () => {
this.remoteWindow = cy.state('window')
})

it('proxies to #invokeFn', () => {
const fn = () => {
return 'bar'
}
it('returns function properties', () => {
const fn = () => 'bar'

cy.wrap({ foo: fn }).its('foo').should('eq', fn)
})
Expand Down Expand Up @@ -1244,7 +1231,8 @@ describe('src/cy/commands/connectors', () => {
cy.stub()
.onCall(0).returns(undefined)
.onCall(1).returns(undefined)
.onCall(2).returns(true),
.onCall(2).returns(undefined)
.onCall(3).returns(true),
)

cy.wrap(obj).its('foo').should('eq', true)
Expand All @@ -1268,15 +1256,6 @@ describe('src/cy/commands/connectors', () => {
cy.wrap({}).its('foo').should('not.exist')
cy.wrap({}).its('foo').should('be.undefined')
cy.wrap({}).its('foo').should('not.be.ok')

// TODO: should these really pass here?
// isn't this the same situation as: cy.should('not.have.class', '...')
//
// when we use the 'eq' and 'not.eq' chainer aren't we effectively
// saying that it must *have* a value as opposed to the property not
// existing at all?
//
// does a tree falling in the forest really make a sound?
cy.wrap({}).its('foo').should('eq', undefined)
cy.wrap({}).its('foo').should('not.eq', 'bar')
})
Expand Down Expand Up @@ -1309,71 +1288,6 @@ describe('src/cy/commands/connectors', () => {
cy.wrap(obj).its('foo').should('eq', undefined)
})

describe('accepts a options argument and works as without options argument', () => {
it('proxies to #invokeFn', () => {
const fn = () => {
return 'bar'
}

cy.wrap({ foo: fn }).its('foo', { log: false }).should('eq', fn)
})

it('does not invoke a function and uses as a property', () => {
const fn = () => {
return 'fn'
}

fn.bar = 'bar'

cy.wrap(fn).its('bar', { log: false }).should('eq', 'bar')
})

it('works with numerical indexes', () => {
cy.wrap(['foo', 'bar']).its(1, {}).should('eq', 'bar')
})

describe('.log', () => {
beforeEach(function () {
this.obj = {
foo: 'foo bar baz',
num: 123,
}

cy.on('log:added', (attrs, log) => {
this.lastLog = log
})

return null
})

it('logs obj as a property', function () {
cy.noop(this.obj).its('foo', { log: true }).then(function () {
const obj = {
name: 'its',
message: '.foo',
}

const { lastLog } = this

_.each(obj, (value, key) => {
expect(lastLog.get(key)).to.deep.eq(value)
})
})
})

it('#consoleProps as a regular property', function () {
cy.noop(this.obj).its('num', { log: true }).then(function () {
expect(this.lastLog.invoke('consoleProps')).to.deep.eq({
Command: 'its',
Property: '.num',
Subject: this.obj,
Yielded: 123,
})
})
})
})
})

describe('.log', () => {
beforeEach(function () {
this.obj = {
Expand Down Expand Up @@ -1489,9 +1403,7 @@ describe('src/cy/commands/connectors', () => {
this.logs = []

cy.on('log:added', (attrs, log) => {
if (attrs.name === 'its') {
this.lastLog = log
}
this.lastLog = log

this.logs?.push(log)
})
Expand Down Expand Up @@ -1602,15 +1514,13 @@ describe('src/cy/commands/connectors', () => {
},
}

obj.foo.bar.baz = () => {
return 'baz'
}
obj.foo.bar.baz = () => 'baz'

cy.on('fail', (err) => {
const { lastLog } = this
const [, itsLog, shouldLog] = this.logs

expect(lastLog.get('error').message).to.include(err.message)
expect(lastLog.invoke('consoleProps').Property).to.eq('.foo.bar.baz')
expect(shouldLog.get('error').message).to.include(err.message)
expect(itsLog.invoke('consoleProps').Property).to.eq('.foo.bar.baz')

done()
})
Expand Down Expand Up @@ -1642,7 +1552,7 @@ describe('src/cy/commands/connectors', () => {
cy.on('fail', (err) => {
const { lastLog } = this

expect(err.message).to.include('Timed out retrying after 100ms: `cy.its()` errored because the property: `baz` does not exist on your subject.')
expect(err.message).to.include('Timed out retrying after 100ms: `cy.its()` errored because the property: `foo.bar.baz.fizz` does not exist on your subject.')
expect(err.docsUrl).to.eq('https://on.cypress.io/its')
expect(lastLog.get('error').message).to.include(err.message)
expect(lastLog.get('error').message).to.include(err.message)
Expand All @@ -1659,19 +1569,19 @@ describe('src/cy/commands/connectors', () => {
cy.wrap(obj).its('foo.bar.baz.fizz')
});

[null, undefined].forEach((val) => {
[null/*, undefined*/].forEach((val) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is intentionally commented out, there should be a comment explaining why.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not intentional; Fixed in latest commit.

it(`throws on traversed '${val}' subject`, (done) => {
cy.on('fail', (err) => {
expect(err.message).to.include(`Timed out retrying after 100ms: \`cy.its()\` errored because the property: \`a\` returned a \`${val}\` value. The property: \`b\` does not exist on a \`${val}\` value.`)
expect(err.message).to.include('`cy.its()` waited for the specified property `b` to become accessible, but it never did.')
expect(err.message).to.include('If you do not expect the property `b` to exist, then add an assertion such as:')
expect(err.message).to.include(`\`cy.wrap({ foo: ${val} }).its('foo.baz').should('not.exist')\``)
expect(err.message).to.include(`Timed out retrying after 100ms: \`cy.its()\` errored because the property: \`a.b\` returned a \`${val}\` value.`)
expect(err.message).to.include('`cy.its()` waited for the specified property `a.b` to become accessible, but it never did.')
expect(err.message).to.include(`If you expect the property \`a.b\` to be \`${val}\`, then add an assertion such as:`)
expect(err.message).to.include(`\`cy.wrap({ foo: ${val} }).its('foo').should('be.null')\``)
expect(err.docsUrl).to.eq('https://on.cypress.io/its')

done()
})

cy.wrap({ a: val }).its('a.b.c')
cy.wrap({ a: { b: val } }).its('a.b')
})

it(`throws on initial '${val}' subject`, (done) => {
Expand Down
4 changes: 1 addition & 3 deletions packages/driver/cypress/e2e/commands/location.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -517,9 +517,7 @@ describe('src/cy/commands/location', () => {

const { lastLog } = this

_.each(obj, (value, key) => {
expect(lastLog.get(key)).to.deep.eq(value)
})
expect(_.pick(lastLog.attributes, ['name', 'message'])).to.eql(obj)
})
})

Expand Down
6 changes: 2 additions & 4 deletions packages/driver/cypress/e2e/commands/misc.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,9 +229,8 @@ describe('src/cy/commands/misc', () => {

it('throws when wrapping an array of windows', (done) => {
cy.on('fail', (err) => {
expect(err.message).to.include('`cy.scrollTo()` failed because it requires a DOM element.')
expect(err.message).to.include('`cy.scrollTo()` failed because it requires a DOM element or window.')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Behavior change (screenshot in PR description): Rather than the mysterious 'All X subject validations failed' message, we now just list out what the valid subjects are.

expect(err.message).to.include('[<window>]')
expect(err.message).to.include('All 2 subject validations failed on this subject.')

done()
})
Expand All @@ -243,9 +242,8 @@ describe('src/cy/commands/misc', () => {

it('throws when wrapping an array of documents', (done) => {
cy.on('fail', (err) => {
expect(err.message).to.include('`cy.screenshot()` failed because it requires a DOM element.')
expect(err.message).to.include('`cy.screenshot()` failed because it requires a DOM element, window or document.')
expect(err.message).to.include('[<document>]')
expect(err.message).to.include('All 3 subject validations failed on this subject.')

done()
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe('src/cy/commands/querying', () => {
cy.get('body').focused().then(function () {
const { lastLog } = this

expect(lastLog.get('type')).to.eq('parent')
expect(lastLog.get('type')).not.to.eq('child')
Copy link
Contributor Author

@BlueWinds BlueWinds Oct 12, 2022

Choose a reason for hiding this comment

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

Logs are parents unless they're made children - the value is now undefined rather than 'parent' - no practical difference.

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case, do you think it'd be cleaner to change type to a boolean flag? log.isParent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly? I'd have to spend more time looking at logs specifically - there's a lot of difficulties and intricacies with them that I think could benefit from a top-level re-examination. I'm not sure parent / child should be a property of the log at all - it's more a property of the log's relationship to other log messages. "Nesting level" might be a more appropriate description than "type".

With that said, I'm not super excited about doing it as part of detached dom - I went down that rabbit hole a little bit earlier in the project and noped out when the scope started expanding on me.

})
})

Expand Down
14 changes: 7 additions & 7 deletions packages/driver/cypress/e2e/commands/traversals.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ describe('src/cy/commands/traversals', () => {
node = dom.stringify(cy.$$(node), 'short')

cy.on('fail', (err) => {
expect(err.message).to.include(`Expected to find element: \`${el}\`, but never found it. Queried from element: ${node}`)
expect(err.message).to.include(`Expected to find element: \`${el}\`, but never found it. Queried from:`)

done()
})
Expand Down Expand Up @@ -322,7 +322,7 @@ describe('src/cy/commands/traversals', () => {

it('errors after timing out not finding element', (done) => {
cy.on('fail', (err) => {
expect(err.message).to.include('Expected to find element: `span`, but never found it. Queried from element: <li.item>')
expect(err.message).to.include('Expected to find element: `span`, but never found it. Queried from:')

done()
})
Expand All @@ -344,13 +344,13 @@ describe('src/cy/commands/traversals', () => {
const button = cy.$$('#button').hide()

cy.on('fail', (err) => {
const log = this.logs[1]
const [, findLog, assertLog] = this.logs

expect(log.get('state')).to.eq('failed')
expect(err.message).to.include(log.get('error').message)
expect(log.get('$el').get(0)).to.eq(button.get(0))
expect(assertLog.get('state')).to.eq('failed')
expect(assertLog.get('error').message).to.include(err.message)
expect(assertLog.get('$el').get(0)).to.eq(button.get(0))

const consoleProps = log.invoke('consoleProps')
const consoleProps = findLog.invoke('consoleProps')

expect(consoleProps.Yielded).to.eq(button.get(0))
expect(consoleProps.Elements).to.eq(button.length)
Expand Down
Loading