From 82077cf4daccdc88f3ae6336bc720f17168b7602 Mon Sep 17 00:00:00 2001 From: Bill Glesias Date: Tue, 24 Jun 2025 11:31:24 -0400 Subject: [PATCH] chore: fix cy.location() not retrying chained `its()` then chained `should()` (#31928) * chore: fix cy.location() to retry if its() assertion fails * return the cached URL while the url is being retried --- .../cypress/e2e/commands/location.cy.js | 8 ++- .../src/cy/commands/helpers/location.ts | 26 ++++++- packages/driver/src/cy/commands/location.ts | 2 +- .../unit/cy/commands/helpers/location.spec.ts | 61 ++++++++++++++++ .../test/unit/cy/commands/location.spec.ts | 70 +++++++++++++++++++ 5 files changed, 161 insertions(+), 6 deletions(-) diff --git a/packages/driver/cypress/e2e/commands/location.cy.js b/packages/driver/cypress/e2e/commands/location.cy.js index 1bbd5f93b8..b7c8414a72 100644 --- a/packages/driver/cypress/e2e/commands/location.cy.js +++ b/packages/driver/cypress/e2e/commands/location.cy.js @@ -416,11 +416,15 @@ describe('src/cy/commands/location', () => { it('eventually returns a given key', function () { cy.stub(Cypress, 'automation').withArgs('get:aut:url') .onFirstCall().resolves('http://localhost:3500') - .onSecondCall().resolves('http://localhost:3500/my/path') + .resolves('http://localhost:3500/my/path') cy.location('pathname').should('equal', '/my/path') .then(() => { - expect(Cypress.automation).to.have.been.calledTwice + // should be called 3 times: + // 1. initial call cy.location('pathname') + // 2. the should() assertion + // 3. the then() callback + expect(Cypress.automation).to.have.been.calledThrice }) }) diff --git a/packages/driver/src/cy/commands/helpers/location.ts b/packages/driver/src/cy/commands/helpers/location.ts index e82da1c259..a0754aa50c 100644 --- a/packages/driver/src/cy/commands/helpers/location.ts +++ b/packages/driver/src/cy/commands/helpers/location.ts @@ -13,6 +13,7 @@ export function getUrlFromAutomation (Cypress: Cypress.Cypress, options: Partial this.set('timeout', timeout) let fullUrlObj: any = null + let hasBeenInitiallyResolved = false let automationPromise: Promise | null = null // need to set a valid type on this let mostRecentError = new UrlNotYetAvailableError() @@ -22,8 +23,6 @@ export function getUrlFromAutomation (Cypress: Cypress.Cypress, options: Partial return automationPromise } - fullUrlObj = null - automationPromise = Cypress.automation('get:aut:url', {}) .timeout(timeout) .then((url) => { @@ -59,8 +58,29 @@ export function getUrlFromAutomation (Cypress: Cypress.Cypress, options: Partial } }) - return () => { + return (options: { + retryAfterResolve?: boolean + } = { + retryAfterResolve: false, + }) => { if (fullUrlObj) { + // In some cases, Cypress will want to retry fetching the url object after it is resolved. + // For instance, in the case of the command yielding an object, like cy.location(). + + // If cy.location().its('url').should('equal', 'https://www.foobar.com') initially fails the 'should' assertion, + // Cypress will want to retry fetching the url object as the onFail handler is NOT called when the subject is chained after 'its'. + + // This does NOT apply if the assertion is chained directly after the command, like cy.location().should('equal', 'https://www.foobar.com'). + // This examples DOES call the onFail handler and fetching the url will be retried from the context of the onFail handler. + if (options?.retryAfterResolve && hasBeenInitiallyResolved) { + // tslint:disable-next-line no-floating-promises + getUrlFromAutomation() + } + + // We only want to retry if the url object has been resolved at least once. + // Otherwise, this will always fetch n + 1 times which is usually unnecessary. + hasBeenInitiallyResolved = true + return fullUrlObj } diff --git a/packages/driver/src/cy/commands/location.ts b/packages/driver/src/cy/commands/location.ts index 43fdeb9054..07c4a01f68 100644 --- a/packages/driver/src/cy/commands/location.ts +++ b/packages/driver/src/cy/commands/location.ts @@ -93,7 +93,7 @@ export function locationQueryCommand (Cypress: Cypress.Cypress, cy: Cypress.Cypr const fn = Cypress.isBrowser('webkit') ? cy.getRemoteLocation : getUrlFromAutomation.bind(this)(Cypress, options) return () => { - const location = fn() + const location = Cypress.isBrowser('webkit') ? fn() : fn({ retryAfterResolve: true }) if (location === '') { // maybe the page's domain is "invisible" to us diff --git a/packages/driver/test/unit/cy/commands/helpers/location.spec.ts b/packages/driver/test/unit/cy/commands/helpers/location.spec.ts index 8165017c71..c1222e7e96 100644 --- a/packages/driver/test/unit/cy/commands/helpers/location.spec.ts +++ b/packages/driver/test/unit/cy/commands/helpers/location.spec.ts @@ -132,6 +132,67 @@ describe('cy/commands/helpers/location', () => { }) }) + it('retries returning the url object after the automation promise is resolved and { retryAfterResolve: true } is passed', async () => { + // @ts-expect-error + mockCypress.automation.mockImplementationOnce(() => { + // no-op promise to simulate the waiting for the automation client + return new Bluebird.Promise((resolve) => resolve('https://www.example.com#foobar')) + }) + + // @ts-expect-error + mockCypress.automation.mockImplementation(() => { + // no-op promise to simulate the waiting for the automation client + return new Bluebird.Promise((resolve) => resolve('https://www.foobar.com#foobar')) + }) + + const fn = getUrlFromAutomation.call(mockContext, mockCypress, mockOptions) + + expect(() => { + fn({ retryAfterResolve: true }) + }).toThrow() + + // flush the microtask queue so we have a url value next time we call fn() + await flushPromises() + + const url = fn({ retryAfterResolve: true }) + + expect(url).toEqual({ + protocol: 'https:', + host: 'www.example.com', + hostname: 'www.example.com', + hash: '#foobar', + search: '', + pathname: '/', + port: '', + origin: 'https://www.example.com', + href: 'https://www.example.com/#foobar', + searchParams: expect.any(Object), + }) + + expect(() => { + // in this case the fn will returned the cached url object until the new one is available + fn({ retryAfterResolve: true }) + }).not.toThrow() + + // flush the microtask queue so we have a url value next time we call fn() + await flushPromises() + + const url2 = fn({ retryAfterResolve: true }) + + expect(url2).toEqual({ + protocol: 'https:', + host: 'www.foobar.com', + hostname: 'www.foobar.com', + hash: '#foobar', + search: '', + pathname: '/', + port: '', + origin: 'https://www.foobar.com', + href: 'https://www.foobar.com/#foobar', + searchParams: expect.any(Object), + }) + }) + it('throws an error when the automation promise is rejected and propagates the error', async () => { // @ts-expect-error mockCypress.automation.mockImplementation(() => { diff --git a/packages/driver/test/unit/cy/commands/location.spec.ts b/packages/driver/test/unit/cy/commands/location.spec.ts index 8546e0dd99..0ad5db514b 100644 --- a/packages/driver/test/unit/cy/commands/location.spec.ts +++ b/packages/driver/test/unit/cy/commands/location.spec.ts @@ -303,6 +303,76 @@ describe('cy/commands/location', () => { locationQueryCommand.call(mockContext, mockCypress, mockCy, 'doesnotexist', {})() }).toThrow('Location object does not have key: `doesnotexist`') }) + + it('retries the command even after the location has resolved', () => { + // @ts-expect-error + getUrlFromAutomation.mockReturnValueOnce((opts) => { + expect(opts).toEqual({ retryAfterResolve: true }) + + return { + protocol: 'https:', + host: 'www.example.com', + hostname: 'www.example.com', + hash: '#foobar', + search: '', + pathname: '/', + port: '', + origin: 'https://www.example.com', + href: 'https://www.example.com/#foobar', + searchParams: expect.any(Object), + } + }) + + // @ts-expect-error + getUrlFromAutomation.mockReturnValueOnce((opts) => { + expect(opts).toEqual({ retryAfterResolve: true }) + + return { + protocol: 'https:', + host: 'www.foobar.com', + hostname: 'www.foobar.com', + hash: '#foobar', + search: '', + pathname: '/', + port: '', + origin: 'https://www.foobar.com', + href: 'https://www.foobar.com/#foobar', + searchParams: expect.any(Object), + } + }) + + const urlObj = locationQueryCommand.call(mockContext, mockCypress, mockCy, undefined, {})() + + expect(urlObj).toEqual({ + protocol: 'https:', + host: 'www.example.com', + hostname: 'www.example.com', + hash: '#foobar', + search: '', + pathname: '/', + port: '', + origin: 'https://www.example.com', + href: 'https://www.example.com/#foobar', + searchParams: expect.any(Object), + }) + + const urlObj2 = locationQueryCommand.call(mockContext, mockCypress, mockCy, undefined, {})() + + expect(urlObj2).toEqual({ + protocol: 'https:', + host: 'www.foobar.com', + hostname: 'www.foobar.com', + hash: '#foobar', + search: '', + pathname: '/', + port: '', + origin: 'https://www.foobar.com', + href: 'https://www.foobar.com/#foobar', + searchParams: expect.any(Object), + }) + + expect(getUrlFromAutomation).toHaveBeenCalledTimes(2) + }) }) describe('webkit', () => {