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>

Author: 3 people

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

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

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)
+ }
  }
 
  /**

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:

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

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
 }

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

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

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

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