fix(server): close Firefox completely between specs on Windows (#7106)

Co-authored-by: Zach Bloomquist <github@chary.us>
Co-authored-by: Jennifer Shehane <jennifer@cypress.io>
Co-authored-by: Gleb Bahmutov <gleb.bahmutov@gmail.com>

packages/server/lib/browsers/firefox-util.ts

@@ -177,6 +177,10 @@ export default {
 
     const { browser } = foxdriver
 
+    browser.on('error', (err) => {
+      debug('received error from foxdriver connection, ignoring %o', err)
+    })
+
     forceGcCc = () => {
       let gcDuration; let ccDuration
 

packages/server/lib/browsers/firefox.ts

@@ -9,7 +9,10 @@ import FirefoxProfile from 'firefox-profile'
 import firefoxUtil from './firefox-util'
 import utils from './utils'
 import * as launcherDebug from '@packages/launcher/lib/log'
-import { Browser } from './types'
+import { Browser, BrowserInstance } from './types'
+import { EventEmitter } from 'events'
+import os from 'os'
+import treeKill from 'tree-kill'
 const errors = require('../errors')
 
 const debug = Debug('cypress:server:browsers:firefox')
@@ -287,7 +290,23 @@ const defaultPreferences = {
   'marionette.log.level': launcherDebug.log.enabled ? 'Debug' : undefined,
 }
 
-export async function open (browser: Browser, url, options: any = {}) {
+export function _createDetachedInstance (browserInstance: BrowserInstance): BrowserInstance {
+  const detachedInstance: BrowserInstance = new EventEmitter() as BrowserInstance
+
+  detachedInstance.pid = browserInstance.pid
+
+  // kill the entire process tree, from the spawned instance up
+  detachedInstance.kill = (): void => {
+    treeKill(browserInstance.pid, (err?, result?) => {
+      debug('force-exit of process tree complete %o', { err, result })
+      detachedInstance.emit('exit')
+    })
+  }
+
+  return detachedInstance
+}
+
+export async function open (browser: Browser, url, options: any = {}): Bluebird<BrowserInstance> {
   const defaultLaunchOptions = utils.getDefaultLaunchOptions({
     extensions: [] as string[],
     preferences: _.extend({}, defaultPreferences),
@@ -347,7 +366,7 @@ export async function open (browser: Browser, url, options: any = {}) {
     launchOptions,
   ] = await Bluebird.all([
     utils.ensureCleanCache(browser, options.isTextTerminal),
-    utils.writeExtension(browser, options.isTextTerminal, options.proxyUrl, options.socketIoRoute, options.onScreencastFrame),
+    utils.writeExtension(browser, options.isTextTerminal, options.proxyUrl, options.socketIoRoute),
     utils.executeBeforeBrowserLaunch(browser, defaultLaunchOptions, options),
   ])
 
@@ -419,5 +438,11 @@ export async function open (browser: Browser, url, options: any = {}) {
     errors.throw('FIREFOX_COULD_NOT_CONNECT', err)
   })
 
+  if (os.platform() === 'win32') {
+    // override the .kill method for Windows so that the detached Firefox process closes between specs
+    // @see https://github.com/cypress-io/cypress/issues/6392
+    return _createDetachedInstance(browserInstance)
+  }
+
   return browserInstance
 }

packages/server/lib/browsers/types.ts

@@ -1,7 +1,13 @@
 import { FoundBrowser } from '@packages/launcher'
+import { EventEmitter } from 'events'
 
 export type Browser = FoundBrowser & {
   majorVersion: number
   isHeadless: boolean
   isHeaded: boolean
 }
+
+export type BrowserInstance = EventEmitter & {
+  kill: () => void
+  pid: number
+}

packages/server/lib/browsers/utils.ts

@@ -205,12 +205,12 @@ export = {
 
   launch: launcher.launch,
 
-  writeExtension (browser, isTextTerminal, proxyUrl, socketIoRoute, onScreencastFrame) {
+  writeExtension (browser, isTextTerminal, proxyUrl, socketIoRoute) {
     debug('writing extension')
 
     // debug('writing extension to chrome browser')
     // get the string bytes for the final extension file
-    return extension.setHostAndPath(proxyUrl, socketIoRoute, onScreencastFrame)
+    return extension.setHostAndPath(proxyUrl, socketIoRoute)
     .then((str) => {
       const extensionDest = getExtensionDir(browser, isTextTerminal)
       const extensionBg = path.join(extensionDest, 'background.js')
@@ -231,7 +231,7 @@ export = {
     debug('getBrowsers')
 
     return launcher.detect()
-    .then((browsers = []) => {
+    .then((browsers: FoundBrowser[] = []) => {
       let majorVersion
 
       debug('found browsers %o', { browsers })

packages/server/package.json

@@ -110,6 +110,7 @@
     "term-size": "2.1.0",
     "tough-cookie": "4.0.0",
     "trash": "5.2.0",
+    "tree-kill": "1.2.2",
     "ts-node": "8.5.4",
     "underscore": "1.9.1",
     "underscore.string": "3.3.5",

packages/server/test/e2e/1_firefox_spec.ts

@@ -68,4 +68,12 @@ describe('e2e firefox', function () {
       expect(stderr).not.to.include('foxdriver')
     },
   })
+
+  // NOTE: only an issue on windows
+  // https://github.com/cypress-io/cypress/issues/6392
+  e2e.it.skip('can run multiple specs', {
+    browser: 'firefox',
+    project: Fixtures.projectPath('e2e'),
+    spec: 'simple_spec.coffee,simple_passing_spec.coffee',
+  })
 })

packages/server/test/unit/browsers/firefox_spec.ts

@@ -8,6 +8,7 @@ import * as firefox from '../../../lib/browsers/firefox'
 import { EventEmitter } from 'events'
 import Marionette from 'marionette-client'
 import Foxdriver from '@benmalka/foxdriver'
+import os from 'os'
 const mockfs = require('mock-fs')
 const FirefoxProfile = require('firefox-profile')
 const utils = require('../../../lib/browsers/utils')
@@ -67,6 +68,7 @@ describe('lib/browsers/firefox', () => {
     const browser = {
       listTabs: sinon.stub().resolves([foxdriverTab]),
       request: sinon.stub().withArgs('listTabs').resolves({ tabs: [foxdriverTab] }),
+      on: sinon.stub(),
     }
 
     foxdriver = {
@@ -102,14 +104,18 @@ describe('lib/browsers/firefox', () => {
         browser: this.browser,
       }
 
+      this.browserInstance = {
+        // should be high enough to not kill any real PIDs
+        pid: Number.MAX_SAFE_INTEGER,
+      }
+
       sinon.stub(process, 'pid').value(1111)
 
       protocol.foo = 'bar'
 
       sinon.stub(plugins, 'has')
       sinon.stub(plugins, 'execute')
       sinon.stub(utils, 'writeExtension').resolves('/path/to/ext')
-      this.browserInstance = {}
       sinon.stub(utils, 'launch').resolves(this.browserInstance)
       sinon.spy(FirefoxProfile.prototype, 'setPreference')
       sinon.spy(FirefoxProfile.prototype, 'updatePreferences')
@@ -201,7 +207,7 @@ describe('lib/browsers/firefox', () => {
 
     it('writes extension', function () {
       return firefox.open(this.browser, 'http://', this.options).then(() => {
-        expect(utils.writeExtension).to.be.calledWith(this.options.browser, this.options.isTextTerminal, this.options.proxyUrl, this.options.socketIoRoute, this.options.onScreencastFrame)
+        expect(utils.writeExtension).to.be.calledWith(this.options.browser, this.options.isTextTerminal, this.options.proxyUrl, this.options.socketIoRoute)
       })
     })
 
@@ -322,6 +328,29 @@ describe('lib/browsers/firefox', () => {
         expect(wrapperErr.message).to.include(err.message)
       })
     })
+
+    context('returns BrowserInstance', function () {
+      it('from browsers.launch', async function () {
+        const instance = await firefox.open(this.browser, 'http://', this.options)
+
+        expect(instance).to.eq(this.browserInstance)
+      })
+
+      // @see https://github.com/cypress-io/cypress/issues/6392
+      it('detached on Windows', async function () {
+        sinon.stub(os, 'platform').returns('win32')
+        const instance = await firefox.open(this.browser, 'http://', this.options)
+
+        expect(instance).to.not.eq(this.browserInstance)
+        expect(instance.pid).to.eq(this.browserInstance.pid)
+
+        await new Promise((resolve) => {
+          // ensure events are wired as expected
+          instance.on('exit', resolve)
+          instance.kill()
+        })
+      })
+    })
   })
 
   context('firefox-util', () => {

yarn.lock

@@ -13931,16 +13931,7 @@ http-proxy-agent@^2.1.0:
     agent-base "4"
     debug "3.1.0"
 
-http-proxy@^1.17.0:
-  version "1.18.1"
-  resolved "https://registry.yarnpkg.com/http-proxy/-/http-proxy-1.18.1.tgz#401541f0534884bbf95260334e72f88ee3976549"
-  integrity sha512-7mz/721AbnJwIVbnaSv1Cz3Am0ZLT/UBwkC92VlxhXv/k/BBQfM2fXElQNC27BVGr0uwUpplYPQM9LnaBMR5NQ==
-  dependencies:
-    eventemitter3 "^4.0.0"
-    follow-redirects "^1.0.0"
-    requires-port "^1.0.0"
-
-http-proxy@cypress-io/node-http-proxy#9322b4b69b34f13a6f3874e660a35df3305179c6:
+http-proxy@^1.17.0, http-proxy@cypress-io/node-http-proxy#9322b4b69b34f13a6f3874e660a35df3305179c6:
   version "1.18.0"
   resolved "https://codeload.github.com/cypress-io/node-http-proxy/tar.gz/9322b4b69b34f13a6f3874e660a35df3305179c6"
   dependencies:
@@ -24433,7 +24424,7 @@ traverse-chain@~0.1.0:
   resolved "https://registry.yarnpkg.com/traverse-chain/-/traverse-chain-0.1.0.tgz#61dbc2d53b69ff6091a12a168fd7d433107e40f1"
   integrity sha1-YdvC1Ttp/2CRoSoWj9fUMxB+QPE=
 
-tree-kill@^1.1.0:
+tree-kill@1.2.2, tree-kill@^1.1.0:
   version "1.2.2"
   resolved "https://registry.yarnpkg.com/tree-kill/-/tree-kill-1.2.2.tgz#4ca09a9092c88b73a7cdc5e8a01b507b0790a0cc"
   integrity sha512-L0Orpi8qGpRG//Nd+H90vFB+3iHnue1zSSGmNOOCh1GLJ7rUKVwV2HvijphGQS2UmhUZewS9VgvxYIdgr+fG1A==