fix: properly run multiple specs in run and headed mode on linux and …

…windows in chrome (#22168) * fix: properly run multiple specs in run and headed mode on linux and windows * fix: properly run multiple specs in run and headed mode on linux and windows * Update test * Update test * Fix issue with running headed in linux and windows * Improve test * Update packages/server/lib/browsers/browser-cri-client.ts * PR comments * PR comments * Fix test failure due to refactor Co-authored-by: Lachlan Miller <lachlan.miller.1990@outlook.com> Co-authored-by: Matt Henkes <mjhenkes@gmail.com>
cypress-io · Jun 10, 2022 · 203758f · 203758f · cypress-bot · Jun 10, 2022
1 parent 600daef
commit 203758f
Show file tree

Hide file tree

Showing 15 changed files with 102 additions and 72 deletions.
diff --git a/packages/extension/app/background.js b/packages/extension/app/background.js
@@ -127,8 +127,8 @@ const connect = function (host, path, extraOpts) {
         return invoke('takeScreenshot', id)
       case 'reset:browser:state':
         return invoke('resetBrowserState', id)
-      case 'close:browser:tabs':
-        return invoke('closeBrowserTabs', id)
+      case 'reset:browser:tabs:for:next:test':
+        return invoke('resetBrowserTabsForNextTest', id)
       default:
         return fail(id, { message: `No handler registered for: '${msg}'` })
     }
@@ -264,8 +264,10 @@ const automation = {
     return browser.browsingData.remove({}, { cache: true, cookies: true, downloads: true, formData: true, history: true, indexedDB: true, localStorage: true, passwords: true, pluginData: true, serviceWorkers: true }).then(fn)
   },
 
-  closeBrowserTabs (fn) {
+  resetBrowserTabsForNextTest (fn) {
     return Promise.try(() => {
+      return browser.tabs.create({ url: 'about:blank' })
+    }).then(() => {
       return browser.windows.getCurrent({ populate: true })
     }).then((windowInfo) => {
       return browser.tabs.remove(windowInfo.tabs.map((tab) => tab.id))

diff --git a/packages/extension/test/integration/background_spec.js b/packages/extension/test/integration/background_spec.js
@@ -30,6 +30,7 @@ const browser = {
   },
   runtime: {},
   tabs: {
+    create () {},
     query () {},
     executeScript () {},
     captureVisibleTab () {},
@@ -48,6 +49,7 @@ const browser = {
 mockRequire('webextension-polyfill', browser)
 
 const background = require('../../app/background')
+const { expect } = require('chai')
 
 const PORT = 12345
 
@@ -846,8 +848,9 @@ describe('app/background', () => {
       })
     })
 
-    describe('close:browser:tabs', () => {
+    describe('reset:browser:tabs:for:next:test', () => {
       beforeEach(() => {
+        sinon.stub(browser.tabs, 'create').withArgs({ url: 'about:blank' })
         sinon.stub(browser.windows, 'getCurrent').withArgs({ populate: true }).resolves({ id: '10', tabs: [{ id: '1' }, { id: '2' }, { id: '3' }] })
         sinon.stub(browser.tabs, 'remove').withArgs(['1', '2', '3']).resolves()
       })
@@ -857,13 +860,14 @@ describe('app/background', () => {
           expect(id).to.eq(123)
           expect(obj.response).to.be.undefined
 
+          expect(browser.tabs.create).to.be.called
           expect(browser.windows.getCurrent).to.be.called
           expect(browser.tabs.remove).to.be.called
 
           done()
         })
 
-        return this.server.emit('automation:request', 123, 'close:browser:tabs')
+        return this.server.emit('automation:request', 123, 'reset:browser:tabs:for:next:test')
       })
     })
   })

diff --git a/packages/server/lib/browsers/browser-cri-client.ts b/packages/server/lib/browsers/browser-cri-client.ts
@@ -67,7 +67,7 @@ const retryWithIncreasingDelay = async <T>(retryable: () => Promise<T>, browserN
 }
 
 export class BrowserCriClient {
-  private currentlyAttachedTarget: CRIWrapper.Client | undefined
+  currentlyAttachedTarget: CRIWrapper.Client | undefined
   private constructor (private browserClient: CRIWrapper.Client, private versionInfo, private port: number, private browserName: string, private onAsynchronousError: Function) {}
 
   /**
@@ -132,28 +132,22 @@ export class BrowserCriClient {
   }
 
   /**
-   * Creates a new target with the given url and then attaches to it
+   * Resets the browser's targets optionally keeping a tab open
    *
-   * @param url the url to create and attach to
-   * @returns the chrome remote interface wrapper for the target
-   */
-  attachToNewUrl = async (url: string): Promise<CRIWrapper.Client> => {
-    debug('Attaching to new url %s', url)
-    const target = await this.browserClient.send('Target.createTarget', { url })
-
-    this.currentlyAttachedTarget = await create(target.targetId, this.onAsynchronousError, HOST, this.port)
-
-    return this.currentlyAttachedTarget
-  }
-
-  /**
-   * Closes the currently attached page target
+   * @param shouldKeepTabOpen whether or not to keep the tab open
    */
-  closeCurrentTarget = async (): Promise<void> => {
+  resetBrowserTargets = async (shouldKeepTabOpen: boolean): Promise<void> => {
     if (!this.currentlyAttachedTarget) {
       throw new Error('Cannot close target because no target is currently attached')
     }
 
+    let target
+
+    // If we are keeping a tab open, we need to first launch a new default tab prior to closing the existing one
+    if (shouldKeepTabOpen) {
+      target = await this.browserClient.send('Target.createTarget', { url: 'about:blank' })
+    }
+
     debug('Closing current target %s', this.currentlyAttachedTarget.targetId)
 
     await Promise.all([
@@ -162,7 +156,9 @@ export class BrowserCriClient {
       this.browserClient.send('Target.closeTarget', { targetId: this.currentlyAttachedTarget.targetId }),
     ])
 
-    this.currentlyAttachedTarget = undefined
+    if (target) {
+      this.currentlyAttachedTarget = await create(target.targetId, this.onAsynchronousError, HOST, this.port)
+    }
   }
 
   /**

diff --git a/packages/server/lib/browsers/cdp_automation.ts b/packages/server/lib/browsers/cdp_automation.ts
@@ -164,7 +164,7 @@ const normalizeResourceType = (resourceType: string | undefined): ResourceType =
 }
 
 type SendDebuggerCommand = (message: string, data?: any) => Promise<any>
-type SendCloseCommand = () => Promise<any>
+type SendCloseCommand = (shouldKeepTabOpen: boolean) => Promise<any>
 type OnFn = (eventName: string, cb: Function) => void
 
 // the intersection of what's valid in CDP and what's valid in FFCDP
@@ -355,8 +355,8 @@ export class CdpAutomation {
           this.sendDebuggerCommandFn('Storage.clearDataForOrigin', { origin: '*', storageTypes: 'all' }),
           this.sendDebuggerCommandFn('Network.clearBrowserCache'),
         ])
-      case 'close:browser:tabs':
-        return this.sendCloseCommandFn()
+      case 'reset:browser:tabs:for:next:test':
+        return this.sendCloseCommandFn(data.shouldKeepTabOpen)
       case 'focus:browser:window':
         return this.sendDebuggerCommandFn('Page.bringToFront')
       default:

diff --git a/packages/server/lib/browsers/chrome.ts b/packages/server/lib/browsers/chrome.ts
@@ -433,8 +433,8 @@ const _handlePausedRequests = async (client) => {
   })
 }
 
-const _setAutomation = async (client: CRIWrapper.Client, automation: Automation, closeCurrentTarget: () => Promise<void>, options: CypressConfiguration = {}) => {
-  const cdpAutomation = await CdpAutomation.create(client.send, client.on, closeCurrentTarget, automation, options.experimentalSessionAndOrigin)
+const _setAutomation = async (client: CRIWrapper.Client, automation: Automation, resetBrowserTargets: (shouldKeepTabOpen: boolean) => Promise<void>, options: CypressConfiguration = {}) => {
+  const cdpAutomation = await CdpAutomation.create(client.send, client.on, resetBrowserTargets, automation, options.experimentalSessionAndOrigin)
 
   return automation.use(cdpAutomation)
 }
@@ -555,9 +555,9 @@ export = {
     debug('connecting to new chrome tab in existing instance with url and debugging port', { url: options.url })
 
     const browserCriClient = this._getBrowserCriClient()
-    const pageCriClient = await browserCriClient.attachToNewUrl('about:blank')
+    const pageCriClient = browserCriClient.currentlyAttachedTarget
 
-    await this._setAutomation(pageCriClient, automation, browserCriClient.closeCurrentTarget, options)
+    await this._setAutomation(pageCriClient, automation, browserCriClient.resetBrowserTargets, options)
 
     // make sure page events are re enabled or else frame tree updates will NOT work as well as other items listening for page events
     await pageCriClient.send('Page.enable')
@@ -584,7 +584,7 @@ export = {
     const browserCriClient = await BrowserCriClient.create(port, browser.displayName, options.onError, onReconnect)
     const pageCriClient = await browserCriClient.attachToTargetUrl(options.url)
 
-    await this._setAutomation(pageCriClient, automation, browserCriClient.closeCurrentTarget, options)
+    await this._setAutomation(pageCriClient, automation, browserCriClient.resetBrowserTargets, options)
   },
 
   async open (browser: Browser, url, options: CypressConfiguration = {}, automation: Automation): Promise<LaunchedBrowser> {
@@ -679,7 +679,7 @@ export = {
 
     const pageCriClient = await browserCriClient.attachToTargetUrl('about:blank')
 
-    await this._setAutomation(pageCriClient, automation, browserCriClient.closeCurrentTarget, options)
+    await this._setAutomation(pageCriClient, automation, browserCriClient.resetBrowserTargets, options)
 
     await pageCriClient.send('Page.enable')
 

diff --git a/packages/server/lib/browsers/firefox-util.ts b/packages/server/lib/browsers/firefox-util.ts
@@ -126,7 +126,7 @@ async function connectToNewSpec (options, automation: Automation, browserCriClie
 
   const pageCriClient = await browserCriClient.attachToTargetUrl('about:blank')
 
-  await CdpAutomation.create(pageCriClient.send, pageCriClient.on, browserCriClient.closeCurrentTarget, automation, options.experimentalSessionAndOrigin)
+  await CdpAutomation.create(pageCriClient.send, pageCriClient.on, browserCriClient.resetBrowserTargets, automation, options.experimentalSessionAndOrigin)
 
   await options.onInitializeNewBrowserTab()
 
@@ -138,7 +138,7 @@ async function setupRemote (remotePort, automation, onError, options): Promise<B
   const browserCriClient = await BrowserCriClient.create(remotePort, 'Firefox', onError)
   const pageCriClient = await browserCriClient.attachToTargetUrl('about:blank')
 
-  await CdpAutomation.create(pageCriClient.send, pageCriClient.on, browserCriClient.closeCurrentTarget, automation, options.experimentalSessionAndOrigin)
+  await CdpAutomation.create(pageCriClient.send, pageCriClient.on, browserCriClient.resetBrowserTargets, automation, options.experimentalSessionAndOrigin)
 
   return browserCriClient
 }

diff --git a/packages/server/lib/modes/run.js b/packages/server/lib/modes/run.js
@@ -1169,7 +1169,7 @@ module.exports = {
   },
 
   waitForTestsToFinishRunning (options = {}) {
-    const { project, screenshots, startedVideoCapture, endVideoCapture, videoName, compressedVideoName, videoCompression, videoUploadOnPasses, exit, spec, estimated, quiet, config } = options
+    const { project, screenshots, startedVideoCapture, endVideoCapture, videoName, compressedVideoName, videoCompression, videoUploadOnPasses, exit, spec, estimated, quiet, config, shouldKeepTabOpen } = options
 
     // https://github.com/cypress-io/cypress/issues/2370
     // delay 1 second if we're recording a video to give
@@ -1251,7 +1251,7 @@ module.exports = {
       //   await openProject.closeBrowser()
       // } else {
       debug('attempting to close the browser tab')
-      await openProject.closeBrowserTabs()
+      await openProject.resetBrowserTabsForNextTest(shouldKeepTabOpen)
       // }
 
       debug('resetting server state')
@@ -1330,16 +1330,16 @@ module.exports = {
       })
     }
 
-    let firstSpec = true
+    let isFirstSpec = true
 
     const runEachSpec = (spec, index, length, estimated) => {
       if (!options.quiet) {
         displaySpecHeader(spec.baseName, index + 1, length, estimated)
       }
 
-      return this.runSpec(config, spec, options, estimated, firstSpec)
+      return this.runSpec(config, spec, options, estimated, isFirstSpec, index === length - 1)
       .tap(() => {
-        firstSpec = false
+        isFirstSpec = false
       })
       .get('results')
       .tap((results) => {
@@ -1435,7 +1435,7 @@ module.exports = {
     })
   },
 
-  runSpec (config, spec = {}, options = {}, estimated, firstSpec) {
+  runSpec (config, spec = {}, options = {}, estimated, isFirstSpec, isLastSpec) {
     const { project, browser, onError } = options
 
     const { isHeadless } = browser
@@ -1484,6 +1484,7 @@ module.exports = {
           videoUploadOnPasses: options.videoUploadOnPasses,
           quiet: options.quiet,
           browser,
+          shouldKeepTabOpen: !isLastSpec,
         }),
 
         connection: this.waitForBrowserToConnect({
@@ -1496,7 +1497,7 @@ module.exports = {
           socketId: options.socketId,
           webSecurity: options.webSecurity,
           projectRoot: options.projectRoot,
-          shouldLaunchNewTab: !firstSpec, // !process.env.CYPRESS_INTERNAL_FORCE_BROWSER_RELAUNCH && !firstSpec,
+          shouldLaunchNewTab: !isFirstSpec, // !process.env.CYPRESS_INTERNAL_FORCE_BROWSER_RELAUNCH && !isFirstSpec,
           // TODO(tim): investigate the socket disconnect
         }),
       })

diff --git a/packages/server/lib/open_project.ts b/packages/server/lib/open_project.ts
@@ -196,8 +196,8 @@ export class OpenProject {
     return browsers.close()
   }
 
-  async closeBrowserTabs () {
-    return this.projectBase?.closeBrowserTabs()
+  async resetBrowserTabsForNextTest (shouldKeepTabOpen: boolean) {
+    return this.projectBase?.resetBrowserTabsForNextTest(shouldKeepTabOpen)
   }
 
   async resetBrowserState () {

diff --git a/packages/server/lib/project-base.ts b/packages/server/lib/project-base.ts
@@ -409,8 +409,8 @@ export class ProjectBase<TServer extends Server> extends EE {
     this.ctx.setAppSocketServer(io)
   }
 
-  async closeBrowserTabs () {
-    return this.server.socket.closeBrowserTabs()
+  async resetBrowserTabsForNextTest (shouldKeepTabOpen: boolean) {
+    return this.server.socket.resetBrowserTabsForNextTest(shouldKeepTabOpen)
   }
 
   async resetBrowserState () {

diff --git a/packages/server/lib/socket-base.ts b/packages/server/lib/socket-base.ts
@@ -76,7 +76,7 @@ const retry = (fn: (res: any) => void) => {
 }
 
 export class SocketBase {
-  private _sendCloseBrowserTabsMessage
+  private _sendResetBrowserTabsForNextTestMessage
   private _sendResetBrowserStateMessage
   private _isRunnerSocketConnected
   private _sendFocusBrowserMessage
@@ -294,8 +294,8 @@ export class SocketBase {
         })
       })
 
-      this._sendCloseBrowserTabsMessage = async () => {
-        await automationRequest('close:browser:tabs', {})
+      this._sendResetBrowserTabsForNextTestMessage = async (shouldKeepTabOpen: boolean) => {
+        await automationRequest('reset:browser:tabs:for:next:test', { shouldKeepTabOpen })
       }
 
       this._sendResetBrowserStateMessage = async () => {
@@ -583,9 +583,9 @@ export class SocketBase {
     return this._io?.emit('tests:finished')
   }
 
-  async closeBrowserTabs () {
-    if (this._sendCloseBrowserTabsMessage) {
-      await this._sendCloseBrowserTabsMessage()
+  async resetBrowserTabsForNextTest (shouldKeepTabOpen: boolean) {
+    if (this._sendResetBrowserTabsForNextTestMessage) {
+      await this._sendResetBrowserTabsForNextTestMessage(shouldKeepTabOpen)
     }
   }