-
Notifications
You must be signed in to change notification settings - Fork 3.3k
feat: rework window bound commands to use automation clients #31862
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
Changes from 3 commits
ce9fdee
e67426a
b5a2b4f
cf1a4d0
626f426
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
# Bump this version to force CI to re-create the cache from scratch. | ||
|
||
5-21-2025 | ||
6-9-2025 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,12 +23,15 @@ describe('src/cy/commands/location', () => { | |
cy.url().should('match', /baz/).and('eq', 'http://localhost:3500/foo/bar/baz.html') | ||
}) | ||
|
||
it('catches thrown errors', () => { | ||
cy.stub(Cypress.utils, 'locToString') | ||
.onFirstCall().throws(new Error) | ||
.onSecondCall().returns('http://localhost:3500/baz.html') | ||
it('propagates thrown errors from CDP', (done) => { | ||
cy.on('fail', (err) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this might be better off in a vite test, but most of these tests are really functioning as some type of integration test 🤷🏻 |
||
expect(err.message).to.include('CDP was unable to find the AUT iframe') | ||
done() | ||
}) | ||
|
||
cy.stub(Cypress, 'automation').withArgs('get:aut:url').rejects(new Error('CDP was unable to find the AUT iframe')) | ||
|
||
cy.url().should('include', '/baz.html') | ||
cy.url() | ||
}) | ||
|
||
// https://github.com/cypress-io/cypress/issues/17399 | ||
|
@@ -380,7 +383,16 @@ describe('src/cy/commands/location', () => { | |
context('#location', () => { | ||
it('returns the location object', () => { | ||
cy.location().then((loc) => { | ||
expect(loc).to.have.keys(['auth', 'authObj', 'hash', 'href', 'host', 'hostname', 'pathname', 'port', 'protocol', 'search', 'origin', 'superDomainOrigin', 'superDomain', 'toString']) | ||
expect(loc).to.have.property('hash') | ||
expect(loc).to.have.property('host') | ||
expect(loc).to.have.property('hostname') | ||
expect(loc).to.have.property('href') | ||
expect(loc).to.have.property('origin') | ||
expect(loc).to.have.property('pathname') | ||
expect(loc).to.have.property('port') | ||
expect(loc).to.have.property('protocol') | ||
expect(loc).to.have.property('search') | ||
expect(loc).to.have.property('searchParams') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this breaking the properties returned? Notice there's no test to toString: https://docs.cypress.io/api/commands/location#When-not-given-a-key-argument There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I need to add the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jennifer-shehane I am going to remove the |
||
}) | ||
}) | ||
|
||
|
@@ -402,15 +414,13 @@ describe('src/cy/commands/location', () => { | |
|
||
// https://github.com/cypress-io/cypress/issues/16463 | ||
it('eventually returns a given key', function () { | ||
cy.stub(cy, 'getRemoteLocation') | ||
.onFirstCall().returns('') | ||
.onSecondCall().returns({ | ||
pathname: '/my/path', | ||
}) | ||
cy.stub(Cypress, 'automation').withArgs('get:aut:url') | ||
.onFirstCall().resolves('http://localhost:3500') | ||
.onSecondCall().resolves('http://localhost:3500/my/path') | ||
|
||
cy.location('pathname').should('equal', '/my/path') | ||
.then(() => { | ||
expect(cy.getRemoteLocation).to.have.been.calledTwice | ||
expect(Cypress.automation).to.have.been.calledTwice | ||
}) | ||
}) | ||
|
||
|
@@ -614,7 +624,8 @@ describe('src/cy/commands/location', () => { | |
expect(_.keys(consoleProps)).to.deep.eq(['name', 'type', 'props']) | ||
expect(consoleProps.name).to.eq('location') | ||
expect(consoleProps.type).to.eq('command') | ||
expect(_.keys(consoleProps.props.Yielded)).to.deep.eq(['auth', 'authObj', 'hash', 'href', 'host', 'hostname', 'origin', 'pathname', 'port', 'protocol', 'search', 'superDomainOrigin', 'superDomain', 'toString']) | ||
|
||
expect(_.keys(consoleProps.props.Yielded)).to.deep.eq(['hash', 'host', 'hostname', 'href', 'origin', 'pathname', 'port', 'protocol', 'search', 'searchParams']) | ||
}) | ||
}) | ||
}) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,35 +11,23 @@ describe('src/cy/commands/navigation', () => { | |
}) | ||
|
||
it('calls into window.location.reload', () => { | ||
const locReload = cy.spy(Cypress.utils, 'locReload') | ||
|
||
cy.reload().then(() => { | ||
expect(locReload).to.be.calledWith(false) | ||
cy.on('fail', () => { | ||
expect(Cypress.automation).to.be.calledWith('reload:aut:frame', { forceReload: false }) | ||
}) | ||
}) | ||
|
||
it('can pass forceReload', () => { | ||
const locReload = cy.spy(Cypress.utils, 'locReload') | ||
cy.stub(Cypress, 'automation').withArgs('reload:aut:frame', { forceReload: false }).resolves() | ||
|
||
cy.reload(true).then(() => { | ||
expect(locReload).to.be.calledWith(true) | ||
}) | ||
cy.reload({ timeout: 1000 }) | ||
}) | ||
|
||
it('can pass forceReload + options', () => { | ||
const locReload = cy.spy(Cypress.utils, 'locReload') | ||
|
||
cy.reload(true, {}).then(() => { | ||
expect(locReload).to.be.calledWith(true) | ||
cy.on('fail', () => { | ||
expect(Cypress.automation).to.be.calledWith('reload:aut:frame', { forceReload: true }) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not a great way to test this besides asserting on the failure that automation client was called |
||
}) | ||
}) | ||
|
||
it('can pass just options', () => { | ||
const locReload = cy.spy(Cypress.utils, 'locReload') | ||
cy.stub(Cypress, 'automation').withArgs('reload:aut:frame', { forceReload: true }).resolves() | ||
|
||
cy.reload({}).then(() => { | ||
expect(locReload).to.be.calledWith(false) | ||
}) | ||
cy.reload(true, { timeout: 1000 }) | ||
}) | ||
|
||
it('returns the window object', () => { | ||
|
@@ -415,7 +403,7 @@ describe('src/cy/commands/navigation', () => { | |
const rel = cy.stub(win, 'removeEventListener') | ||
|
||
cy.go('back').then(() => { | ||
const unloadEvent = cy.browser.family === 'chromium' ? 'pagehide' : 'unload' | ||
const unloadEvent = Cypress.browser.family === 'chromium' ? 'pagehide' : 'unload' | ||
|
||
expect(rel).to.be.calledWith('beforeunload') | ||
expect(rel).to.be.calledWith(unloadEvent) | ||
|
@@ -600,14 +588,15 @@ describe('src/cy/commands/navigation', () => { | |
const { lastLog } = this | ||
|
||
beforeunload = true | ||
expect(lastLog.get('snapshots').length).to.eq(1) | ||
expect(lastLog.get('snapshots').length).to.eq(2) | ||
expect(lastLog.get('snapshots')[0].name).to.eq('before') | ||
expect(lastLog.get('snapshots')[0].body).to.be.an('object') | ||
|
||
return undefined | ||
}) | ||
|
||
cy.go('back').then(function () { | ||
// wait for the beforeunload event to be fired after the history navigation | ||
cy.go('back').wait(100).then(function () { | ||
const { lastLog } = this | ||
|
||
expect(beforeunload).to.be.true | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,19 +62,16 @@ context('cy.origin location', { browser: '!webkit' }, () => { | |
expect(consoleProps.name).to.equal('location') | ||
expect(consoleProps.type).to.equal('command') | ||
|
||
expect(consoleProps.props.Yielded).to.have.property('auth').that.is.a('string') | ||
expect(consoleProps.props.Yielded).to.have.property('authObj').that.is.undefined | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the signature has slightly changed on the location object, but still matches the docs in https://docs.cypress.io/api/commands/location. The legacy signature (that isn't public) will still be used by Webkit as it is using the window location API |
||
expect(consoleProps.props.Yielded).to.have.property('hash').that.is.a('string') | ||
expect(consoleProps.props.Yielded).to.have.property('host').that.is.a('string') | ||
expect(consoleProps.props.Yielded).to.have.property('hostname').that.is.a('string') | ||
expect(consoleProps.props.Yielded).to.have.property('href').that.is.a('string') | ||
expect(consoleProps.props.Yielded).to.have.property('origin').that.is.a('string') | ||
expect(consoleProps.props.Yielded).to.have.property('superDomainOrigin').that.is.a('string') | ||
expect(consoleProps.props.Yielded).to.have.property('pathname').that.is.a('string') | ||
expect(consoleProps.props.Yielded).to.have.property('port').that.is.a('string') | ||
expect(consoleProps.props.Yielded).to.have.property('protocol').that.is.a('string') | ||
expect(consoleProps.props.Yielded).to.have.property('search').that.is.a('string') | ||
expect(consoleProps.props.Yielded).to.have.property('superDomain').that.is.a('string') | ||
expect(consoleProps.props.Yielded).to.have.property('searchParams').that.is.an('object') | ||
}) | ||
}) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to indicate why this would be breaking at all? It's hard to understand what situation would break here and what action I would need to take as a user reading this (or maybe this would be good for the migration guide?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not a breaking change but I marked it as a feature. It's more of an improvement if anything I would think