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

Author: AtofStryker

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
       })
     })
 

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<void> | 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
     }
 

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

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(() => {

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', () => {