fix: Leverage Puppeteer's native support for Firefox (#356)

BREAKING CHANGE: browser config is deprecated. Use launch.product instead. Co-authored-by: Tony Brix <[email protected]>
argos-ci · Apr 16, 2021 · e54fb7e · e54fb7e
1 parent ea3b189
commit e54fb7e
Show file tree

Hide file tree

Showing 12 changed files with 124 additions and 47 deletions.
diff --git a/README.md b/README.md
@@ -146,7 +146,7 @@ Other options are documented in [jest-dev-server](https://github.com/smooth-code
 
 Jest Puppeteer automatically detects the best config to start Puppeteer but sometimes you may need to specify custom options. All Puppeteer [launch](https://github.com/GoogleChrome/puppeteer/blob/master/docs/api.md#puppeteerlaunchoptions) or [connect](https://github.com/GoogleChrome/puppeteer/blob/master/docs/api.md#puppeteerconnectoptions) options can be specified in `jest-puppeteer.config.js` at the root of the project. Since it is JavaScript, you can use all the stuff you need, including environment.
 
-To run Puppeteer on Firefox using [puppeteer-firefox](https://github.com/GoogleChrome/puppeteer/tree/master/experimental/puppeteer-firefox), you can set `browser` to `firefox`. By default, the value is `chromium` which will use Puppeteer on Chrome.
+To run Puppeteer on Firefox, you can set the `launch.product` property to `firefox`. By default, the value is `chrome` which will use Puppeteer on Chromium.
 
 The browser context can be also specified. By default, the browser context is shared. `incognito` is available if you want more isolation between running instances. More information available in [jest-puppeteer-environment readme](https://github.com/smooth-code/jest-puppeteer/blob/master/packages/jest-environment-puppeteer/README.md)
 
@@ -156,8 +156,8 @@ module.exports = {
   launch: {
     dumpio: true,
     headless: process.env.HEADLESS !== 'false',
+    product: 'chrome',
   },
-  browser: 'chromium',
   browserContext: 'default',
 }
 ```

diff --git a/package.json b/package.json
@@ -38,7 +38,6 @@
     "lint-staged": "^10.5.4",
     "prettier": "^2.2.1",
     "puppeteer": "^8.0.0",
-    "puppeteer-firefox": "0.5.0",
     "rimraf": "^3.0.2"
   },
   "dependencies": {}

diff --git a/packages/jest-environment-puppeteer/README.md b/packages/jest-environment-puppeteer/README.md
@@ -103,9 +103,6 @@ You can specify a `jest-puppeteer.config.js` at the root of the project or defin
 
 - `launch` <[object]> [All Puppeteer launch options](https://github.com/GoogleChrome/puppeteer/blob/master/docs/api.md#puppeteerlaunchoptions) can be specified in config. Since it is JavaScript, you can use all stuff you need, including environment.
 - `connect` <[object]> [All Puppeteer connect options](https://github.com/GoogleChrome/puppeteer/blob/master/docs/api.md#puppeteerconnectoptions) can be specified in config. This is an alternative to `launch` config, allowing you to connect to an already running instance of Chrome.
-- `browser` <[string]>. Define a browser to run tests into.
-  - `chromium` Each test uses [puppeteer](https://npmjs.com/package/puppeteer) and runs Chromium
-  - `firefox` Each test uses [puppeteer-firefox](https://npmjs.com/package/puppeteer-firefox) and runs Firefox. This option requires `puppeteer-firefox` as a peer dependency.
 - `browserContext` <[string]>. By default, the browser context (cookies, localStorage, etc) is shared between all tests. The following options are available for `browserContext`:
   - `default` Each test starts a tab, so all tests share the same context.
   - `incognito` Each tests starts an incognito window, so all tests have a separate, isolated context. Useful when running tests that could interfere with one another. (_Example: testing multiple users on the same app at once with login, transactions, etc._)

diff --git a/packages/jest-environment-puppeteer/src/PuppeteerEnvironment.js b/packages/jest-environment-puppeteer/src/PuppeteerEnvironment.js
@@ -27,7 +27,7 @@ class PuppeteerEnvironment extends NodeEnvironment {
 
   async setup() {
     const config = await readConfig()
-    const puppeteer = getPuppeteer(config)
+    const puppeteer = getPuppeteer()
     this.global.puppeteerConfig = config
 
     const wsEndpoint = process.env.PUPPETEER_WS_ENDPOINT

diff --git a/packages/jest-environment-puppeteer/src/global.js b/packages/jest-environment-puppeteer/src/global.js
@@ -14,7 +14,7 @@ let didAlreadyRunInWatchMode = false
 
 export async function setup(jestConfig = {}) {
   const config = await readConfig()
-  const puppeteer = getPuppeteer(config)
+  const puppeteer = getPuppeteer()
   if (config.connect) {
     browser = await puppeteer.connect(config.connect)
   } else {

diff --git a/packages/jest-environment-puppeteer/src/readConfig.js b/packages/jest-environment-puppeteer/src/readConfig.js
@@ -8,7 +8,6 @@ const exists = promisify(fs.exists)
 
 const DEFAULT_CONFIG = {
   launch: {},
-  browser: 'chromium',
   browserContext: 'default',
   exitOnPageError: true,
 }
@@ -46,24 +45,38 @@ export async function readConfig() {
 
   // eslint-disable-next-line global-require, import/no-dynamic-require
   const localConfig = await require(absConfigPath)
+
+  const product = localConfig.launch ? localConfig.launch.product : undefined
+
+  // Move browser config to launch.product
+  if (product === undefined && localConfig.browser) {
+    // eslint-disable-next-line no-console
+    console.warn(
+      '`browser` config has been deprecated and will be removed in future versions. Use `launch.product` config with `chrome` or `firefox` instead.',
+    )
+    let launch = {}
+    if (localConfig.launch) {
+      launch = localConfig.launch
+    }
+    launch.product =
+      localConfig.browser === 'chromium' ? 'chrome' : localConfig.browser
+    localConfig.launch = launch
+  }
+
+  // Ensure that launch.product is equal to 'chrome', or 'firefox'
+  if (product !== undefined && !['chrome', 'firefox'].includes(product)) {
+    throw new Error(`Error: Invalid product value '${product}'`)
+  }
+
   return merge({}, defaultConfig, localConfig)
 }
 
-export function getPuppeteer(config) {
-  switch (config.browser.toLowerCase()) {
-    /* eslint-disable global-require, import/no-dynamic-require, import/no-extraneous-dependencies, import/no-unresolved */
-    case 'chromium':
-      try {
-        return require('puppeteer')
-      } catch (e) {
-        return require('puppeteer-core')
-      }
-    case 'firefox':
-      return require('puppeteer-firefox')
-    /* eslint-enable */
-    default:
-      throw new Error(
-        `Error: "browser" config option is given an unsupported value: ${browser}`,
-      )
+export function getPuppeteer() {
+  /* eslint-disable global-require, import/no-dynamic-require, import/no-extraneous-dependencies, import/no-unresolved */
+  try {
+    return require('puppeteer')
+  } catch (e) {
+    return require('puppeteer-core')
   }
+  /* eslint-enable */
 }
diff --git a/packages/jest-environment-puppeteer/tests/__fixtures__/browserConfig.js b/packages/jest-environment-puppeteer/tests/__fixtures__/browserConfig.js
@@ -0,0 +1,3 @@
+module.exports = {
+  browser: 'firefox',
+}
diff --git a/packages/jest-environment-puppeteer/tests/__fixtures__/browserConfigChromium.js b/packages/jest-environment-puppeteer/tests/__fixtures__/browserConfigChromium.js
@@ -0,0 +1,3 @@
+module.exports = {
+  browser: 'chromium',
+}
diff --git a/packages/jest-environment-puppeteer/tests/__fixtures__/browserLaunchConfig.js b/packages/jest-environment-puppeteer/tests/__fixtures__/browserLaunchConfig.js
@@ -0,0 +1,6 @@
+module.exports = {
+  browser: 'firefox',
+  launch: {
+    some: 'other property',
+  },
+}
diff --git a/packages/jest-environment-puppeteer/tests/__fixtures__/browserLaunchProductConfig.js b/packages/jest-environment-puppeteer/tests/__fixtures__/browserLaunchProductConfig.js
@@ -0,0 +1,6 @@
+module.exports = {
+  browser: 'firefox',
+  launch: {
+    product: 'chrome',
+  },
+}
diff --git a/packages/jest-environment-puppeteer/tests/__fixtures__/invalidProduct.js b/packages/jest-environment-puppeteer/tests/__fixtures__/invalidProduct.js
@@ -0,0 +1,5 @@
+module.exports = {
+  launch: {
+    product: 'foobar',
+  },
+}
diff --git a/packages/jest-environment-puppeteer/tests/readConfig.test.js b/packages/jest-environment-puppeteer/tests/readConfig.test.js
@@ -1,34 +1,13 @@
 import fs from 'fs'
 import path from 'path'
-// eslint-disable-next-line import/no-extraneous-dependencies
-import pptrChromium from 'puppeteer'
-// eslint-disable-next-line import/no-extraneous-dependencies
-import pptrFirefox from 'puppeteer-firefox'
-import { readConfig, getPuppeteer } from '../src/readConfig'
+import { readConfig } from '../src/readConfig'
 
 jest.mock('fs')
 
 function mockExists(value) {
   fs.exists.mockImplementation((path, callback) => callback(null, value))
 }
 
-describe('getPuppeteer', () => {
-  it('should return chromium when specified', async () => {
-    expect(
-      getPuppeteer({
-        browser: 'Chromium',
-      }),
-    ).toBe(pptrChromium)
-  })
-  it('should return firefox when specified', async () => {
-    expect(
-      getPuppeteer({
-        browser: 'Firefox',
-      }),
-    ).toBe(pptrFirefox)
-  })
-})
-
 describe('readConfig', () => {
   describe('with custom config path', () => {
     beforeEach(() => {
@@ -101,4 +80,70 @@ describe('readConfig', () => {
       expect(config.server).toBeDefined()
     })
   })
+
+  describe('with custom product type', () => {
+    it('should return an error if invalid product', async () => {
+      process.env.JEST_PUPPETEER_CONFIG = path.resolve(
+        __dirname,
+        '__fixtures__/invalidProduct.js',
+      )
+      mockExists(true)
+      try {
+        await readConfig()
+      } catch (error) {
+        expect(error.message).toBe("Error: Invalid product value 'foobar'")
+      }
+    })
+  })
+
+  describe('with browser config', () => {
+    beforeEach(() => {
+      jest.spyOn(console, 'warn').mockImplementation(() => {})
+    })
+
+    it('should set launch.product', async () => {
+      process.env.JEST_PUPPETEER_CONFIG = path.resolve(
+        __dirname,
+        '__fixtures__/browserConfig.js',
+      )
+      mockExists(true)
+      const config = await readConfig()
+      // eslint-disable-next-line no-console
+      expect(console.warn).toHaveBeenCalledWith(
+        '`browser` config has been deprecated and will be removed in future versions. Use `launch.product` config with `chrome` or `firefox` instead.',
+      )
+      expect(config.launch.product).toBe('firefox')
+    })
+
+    it('should set launch.product to chrome', async () => {
+      process.env.JEST_PUPPETEER_CONFIG = path.resolve(
+        __dirname,
+        '__fixtures__/browserConfigChromium.js',
+      )
+      mockExists(true)
+      const config = await readConfig()
+      expect(config.launch.product).toBe('chrome')
+    })
+
+    it('should not overwrite launch', async () => {
+      process.env.JEST_PUPPETEER_CONFIG = path.resolve(
+        __dirname,
+        '__fixtures__/browserLaunchConfig.js',
+      )
+      mockExists(true)
+      const config = await readConfig()
+      expect(config.launch.product).toBe('firefox')
+      expect(config.launch.some).toBe('other property')
+    })
+
+    it('should not overwrite launch.product', async () => {
+      process.env.JEST_PUPPETEER_CONFIG = path.resolve(
+        __dirname,
+        '__fixtures__/browserLaunchProductConfig.js',
+      )
+      mockExists(true)
+      const config = await readConfig()
+      expect(config.launch.product).toBe('chrome')
+    })
+  })
 })