Some improvements (#479)

Author: mmarkelov

README.md

@@ -120,7 +120,7 @@ module.exports = {
 - `serverOptions` <[object]>. [All `jest-process-manager` options](https://github.com/playwright-community/jest-process-manager#options).
 - `selectors` <[array]>. Define [selectors](https://github.com/microsoft/playwright/blob/v0.11.1/docs/api.md#class-selectors). Each selector must be an object with name and script properties.
 - `skipInitialization` <[boolean]>. Add you ability to skip first setup `playwright` process. Possible use cases can be found [here](https://github.com/playwright-community/jest-playwright/issues/424)
-- `useDefaultBrowserType` <[boolean]>. [Sometimes](https://github.com/microsoft/playwright/issues/2787) `browser` + `device` combinations don't have any sense. With this option tests will be run with [`defaultBrowserType`](https://github.com/microsoft/playwright/pull/3731) of device
+- `useDefaultBrowserType` <[boolean]>. [Sometimes](https://github.com/microsoft/playwright/issues/2787) `browser` + `device` combinations don't have any sense. With this option tests will be run with [`defaultBrowserType`](https://github.com/microsoft/playwright/pull/3731) of device. Pay attention that you should define **devices** to correct usage of this option.
 
 ### Specific browser options
 

package-lock.json

@@ -46,30 +46,30 @@
     "expect-playwright": "^0.2.6",
     "jest-circus": "^26.6.3",
     "jest-environment-node": "^26.6.2",
-    "jest-process-manager": "^0.2.6",
+    "jest-process-manager": "^0.2.7",
     "nyc": "^15.1.0",
     "playwright-core": ">=1.2.0",
     "rimraf": "^3.0.2",
     "uuid": "^8.3.1"
   },
   "devDependencies": {
     "@types/debug": "4.1.5",
-    "@types/jest": "26.0.15",
+    "@types/jest": "26.0.16",
     "@types/jest-dev-server": "4.2.0",
     "@types/node": "14.14.10",
     "@types/rimraf": "^3.0.0",
     "@types/uuid": "^8.3.0",
-    "@typescript-eslint/eslint-plugin": "4.8.2",
-    "@typescript-eslint/parser": "4.8.1",
+    "@typescript-eslint/eslint-plugin": "4.9.0",
+    "@typescript-eslint/parser": "4.9.0",
     "coveralls": "3.1.0",
     "eslint": "7.14.0",
     "eslint-config-airbnb-base": "14.2.1",
     "eslint-config-prettier": "6.15.0",
     "eslint-plugin-import": "2.22.1",
-    "eslint-plugin-prettier": "3.1.4",
+    "eslint-plugin-prettier": "3.2.0",
     "husky": "4.3.0",
     "jest": "26.6.3",
-    "lint-staged": "10.5.2",
+    "lint-staged": "10.5.3",
     "playwright": ">=1.2.0",
     "playwright-chromium": ">=1.2.0",
     "prettier": "2.2.1",

package.json

@@ -241,9 +241,7 @@ export const getPlaywrightEnv = (basicEnv = 'node'): unknown => {
         resetContext: async (newOptions?: ConnectOptions): Promise<void> => {
           const { browser, context } = this.global
 
-          if (context) {
-            await context.close()
-          }
+          await context?.close()
 
           let newContextOptions = contextOptions
 
@@ -258,9 +256,7 @@ export const getPlaywrightEnv = (basicEnv = 'node'): unknown => {
         resetBrowser: async (newOptions?: ConnectOptions): Promise<void> => {
           const { browser } = this.global
 
-          if (browser) {
-            await browser.close()
-          }
+          await browser?.close()
 
           this.global.browser = await getBrowserPerProcess(
             playwrightInstance as GenericBrowser,
@@ -319,8 +315,7 @@ export const getPlaywrightEnv = (basicEnv = 'node'): unknown => {
       // Hack to set testTimeout for jestPlaywright debugging
       if (
         event.name === 'add_test' &&
-        event.fn &&
-        event.fn.toString().includes('jestPlaywright.debug()')
+        event.fn?.toString().includes('jestPlaywright.debug()')
       ) {
         // Set timeout to 4 days
         state.testTimeout = 4 * 24 * 60 * 60 * 1000
@@ -330,9 +325,7 @@ export const getPlaywrightEnv = (basicEnv = 'node'): unknown => {
     async teardown(): Promise<void> {
       const { browser, context, page } = this.global
       const { collectCoverage } = this._jestPlaywrightConfig
-      if (page) {
-        page.removeListener('pageerror', handleError)
-      }
+      page?.removeListener('pageerror', handleError)
       if (collectCoverage) {
         await Promise.all(
           (context as BrowserContext).pages().map((p) =>
@@ -345,9 +338,7 @@ export const getPlaywrightEnv = (basicEnv = 'node'): unknown => {
         await new Promise((resolve) => setTimeout(resolve, 10))
       }
 
-      if (browser) {
-        await browser.close()
-      }
+      await browser?.close()
 
       await super.teardown()
     }

src/PlaywrightEnvironment.ts

@@ -12,7 +12,6 @@ import type { Config as JestConfig } from '@jest/types'
 import type {
   BrowserType,
   BrowserTest,
-  CustomDeviceType,
   DeviceType,
   WsEndpointType,
   JestPlaywrightTest,
@@ -21,7 +20,7 @@ import type {
 } from '../types/global'
 import {
   checkBrowserEnv,
-  checkDeviceEnv,
+  checkDevice,
   getDisplayName,
   readConfig,
   getPlaywrightInstance,
@@ -61,9 +60,7 @@ const getBrowserTest = ({
         device,
         displayName: {
           name: displayName
-            ? `${playwrightDisplayName} ${
-                typeof displayName === 'string' ? displayName : displayName.name
-              }`
+            ? `${playwrightDisplayName} ${displayName.name || displayName}`
             : playwrightDisplayName,
           color: 'yellow',
         },
@@ -76,7 +73,7 @@ const getDevices = (
   devices: JestPlaywrightConfig['devices'],
   availableDevices: typeof PlaywrightDevices,
 ) => {
-  let resultDevices: (string | CustomDeviceType)[] = []
+  let resultDevices: ConfigDeviceType[] = []
 
   if (devices) {
     if (devices instanceof RegExp) {
@@ -99,11 +96,7 @@ const getBrowser = (
     return availableDevices[device].defaultBrowserType
   }
 
-  if (device.defaultBrowserType) {
-    return device.defaultBrowserType
-  }
-
-  return CHROMIUM
+  return device?.defaultBrowserType || CHROMIUM
 }
 
 class PlaywrightRunner extends JestRunner {
@@ -132,7 +125,7 @@ class PlaywrightRunner extends JestRunner {
         this.browser2Server[browser] = await instance.launchServer(options)
       }
     }
-    return wsEndpoint || this.browser2Server[browser]!.wsEndpoint()
+    return wsEndpoint || this.browser2Server[browser]?.wsEndpoint() || null
   }
 
   async getTests(tests: Test[], config: JestPlaywrightConfig): Promise<Test[]> {
@@ -157,10 +150,7 @@ class PlaywrightRunner extends JestRunner {
               (instance as Record<BrowserType, GenericBrowser>)[browser],
             )
 
-            if (typeof device === 'string') {
-              const availableDeviceNames = Object.keys(availableDevices)
-              checkDeviceEnv(device, availableDeviceNames)
-            }
+            checkDevice(device, availableDevices)
 
             pwTests.push(
               getBrowserTest({ ...browserTest, browser, wsEndpoint, device }),
@@ -190,11 +180,7 @@ class PlaywrightRunner extends JestRunner {
 
           if (resultDevices.length) {
             resultDevices.forEach((device: DeviceType) => {
-              if (typeof device === 'string') {
-                const availableDeviceNames = Object.keys(availableDevices)
-                checkDeviceEnv(device, availableDeviceNames)
-              }
-
+              checkDevice(device, availableDevices)
               pwTests.push(getBrowserTest({ ...browserTest, device }))
             })
           } else {

src/PlaywrightRunner.ts

@@ -3,13 +3,15 @@ import path from 'path'
 import * as Utils from './utils'
 import { DEFAULT_CONFIG, CHROMIUM, FIREFOX } from './constants'
 import type { BrowserType, JestPlaywrightConfig } from '../types/global'
+import { Playwright } from '../types/global'
 
 const {
   readConfig,
   getBrowserType,
   getDeviceType,
   checkBrowserEnv,
   checkDeviceEnv,
+  checkDevice,
   getPlaywrightInstance,
   getDisplayName,
   getSkipFlag,
@@ -230,6 +232,29 @@ describe('checkDeviceEnv', () => {
   })
 })
 
+describe('checkDevice', () => {
+  const devices = {
+    'iPhone 11': {},
+    'Pixel 2': {},
+    'Nexus 4': {},
+  } as Playwright['devices']
+
+  it('should not throw Error if device is exist', async () => {
+    const device = 'Nexus 4'
+    expect(() => checkDevice(device, devices)).not.toThrow()
+  })
+
+  it('should not throw Error if device is not string', async () => {
+    const device = null
+    expect(() => checkDevice(device, devices)).not.toThrow()
+  })
+
+  it('should throw Error with unknown type', async () => {
+    const device = 'unknown'
+    expect(() => checkDevice(device, devices)).toThrow()
+  })
+})
+
 describe('getSkipFlag', () => {
   it('should return true if skipOption.browsers includes browserName', async () => {
     const skipOptions = { browsers: [CHROMIUM as BrowserType] }

src/utils.test.ts

@@ -21,10 +21,13 @@ import {
 const fsPromises = fs.promises
 
 export const checkBrowserEnv = (param: BrowserType): void => {
-  if (param !== CHROMIUM && param !== FIREFOX && param !== WEBKIT) {
+  const browsers = [CHROMIUM, FIREFOX, WEBKIT]
+  if (!browsers.includes(param)) {
     throw new Error(
       formatError(
-        `Wrong browser type. Should be one of [${CHROMIUM}, ${FIREFOX}, ${WEBKIT}], but got ${param}`,
+        `Wrong browser type. Should be one of [${browsers.join(
+          ', ',
+        )}], but got ${param}`,
       ),
     )
   }
@@ -70,6 +73,16 @@ export const checkDeviceEnv = (
   }
 }
 
+export const checkDevice = (
+  device: DeviceType,
+  availableDevices: Playwright['devices'],
+): void => {
+  if (typeof device === 'string') {
+    const availableDeviceNames = Object.keys(availableDevices)
+    checkDeviceEnv(device, availableDeviceNames)
+  }
+}
+
 export const getDisplayName = (
   browser: BrowserType,
   device: DeviceType,
@@ -87,10 +100,7 @@ export const getDisplayName = (
 
 export const getDeviceType = (device: DeviceType): DeviceType => {
   const processDevice = process.env.DEVICE
-  if (processDevice) {
-    return processDevice
-  }
-  return device
+  return processDevice || device
 }
 
 export const getBrowserType = (browser?: BrowserType): BrowserType => {

src/utils.ts

@@ -132,6 +132,8 @@ declare global {
     }
     interface Describe {
       jestPlaywrightSkip: JestParams<SkipOption>
+      jestPlaywrightDebug: JestPlaywrightTestDebug
+      jestPlaywrightConfig: JestPlaywrightTestConfig
     }
   }
 }