refactor: extract browser pool logic into its own pool

vitest-dev · Mar 27, 2023 · 30618cb · 30618cb
1 parent d39f51d
commit 30618cb
Show file tree

Hide file tree

Showing 16 changed files with 160 additions and 205 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -132,11 +132,11 @@ jobs:
       - name: Build
         run: pnpm run build
 
-      - name: Test Browser
-        run: pnpm run browser:test
+      - name: Test Browser (webdriverio)
+        run: pnpm run test:browser:webdriverio
 
       - name: Test Browser (playwright)
-        run: pnpm run browser:playwright:test
+        run: pnpm run test:browser:playwright
         env:
           BROWSER: ${{ matrix.browser[1] }}
 
@@ -160,8 +160,8 @@ jobs:
       - name: Enable
         run: sudo safaridriver --enable
 
-      - name: Test Browser
-        run: sudo BROWSER=safari pnpm run browser:test
+      - name: Test Browser (webdriverio)
+        run: sudo BROWSER=safari pnpm run test:browser:webdriverio
 
       - name: Test Browser (playwright)
-        run: sudo BROWSER=webkit pnpm run browser:playwright:test
+        run: sudo BROWSER=webkit pnpm run test:browser:playwright
diff --git a/docs/config/index.md b/docs/config/index.md
@@ -1001,7 +1001,7 @@ Listen to port and serve API. When set to true, the default port is 51204
 - **Version:** Since Vitest 0.29.4
 - **CLI:** `--browser`, `--browser=<name>`, `--browser.name=chrome --browser.headless`
 
-Run Vitest tests in a browser. If the browser name is not specified, Vitest will try to determine your default browser automatically. We use [WebdriverIO](https://webdriver.io/) for running tests by default, but it can be configured with [browser.provider](/config/#browser-provider) option.
+Run Vitest tests in a browser. We use [WebdriverIO](https://webdriver.io/) for running tests by default, but it can be configured with [browser.provider](/config/#browser-provider) option.
 
 ::: tip NOTE
 Read more about testing in a real browser in the [guide page](/guide/browser).
@@ -1022,14 +1022,13 @@ Run all tests inside a browser by default. Can be overriden with [`poolMatchGlob
 #### browser&#46;name
 
 - **Type:** `string`
-- **Default:** _tries to find default browser automatically_
 - **CLI:** `--browser=safari`
 
-Run all tests in a specific browser. If not specified, tries to find a browser automatically. Possible options in different providers:
+Run all tests in a specific browser. Possible options in different providers:
 
 - `webdriverio`: `firefox`, `chrome`, `edge`, `safari`
 - `playwright`: `firefox`, `webkit`, `chromium`
-- custom: path to the custom provider
+- custom: any string that will be passed to the provider
 
 #### browser.headless
 
@@ -1049,21 +1048,19 @@ Configure options for Vite server that serves code in the browser. Does not affe
 
 #### browser.provider
 
-- **Type:** `string`
+- **Type:** `'webdriverio' | 'playwright' | string`
 - **Default:** `'webdriverio'`
-- **CLI:** `--browser.provider=./custom-provider.ts`
+- **CLI:** `--browser.provider=playwright`
 
-Path to a provider that will be used when running browser tests. Vitest provides two providers which are `webdriverio` (default) and `playwright`. Provider should be exported using `default` export and have this shape:
+Path to a provider that will be used when running browser tests. Vitest provides two providers which are `webdriverio` (default) and `playwright`. Custom providers should be exported using `default` export and have this shape:
 
 ```ts
 export interface BrowserProvider {
-  initialize(ctx: Vitest): Awaitable<void>
-  createPool(): {
-    runTests: (files: string[], invalidated: string[]) => void
-    close: () => Awaited<void>
-  }
-  // signals that test file stopped running, if it was opened with `id=` query
-  testFinished?(testId: string): Awaitable<void>
+  name: string
+  getSupportedBrowsers(): readonly string[]
+  initialize(ctx: Vitest, options: { browser: string }): Awaitable<void>
+  openPage(url: string): Awaitable<void>
+  close(): Awaitable<void>
 }
 ```
 

diff --git a/docs/guide/browser.md b/docs/guide/browser.md
@@ -1,8 +1,8 @@
 ---
-title: Browser mode | Guide
+title: Browser Mode | Guide
 ---
 
-# Browser mode (experimental)
+# Browser Mode (experimental)
 
 This page provides information about the experimental browser mode feature in the Vitest API, which allows you to run your tests in the browser natively, providing access to browser globals like window and document. This feature is currently under development, and APIs may change in the future.
 
@@ -14,32 +14,26 @@ To activate browser mode in your Vitest configuration, you can use the `--browse
 export default defineConfig({
   test: {
     browser: {
-      enabled: true
+      enabled: true,
+      name: 'chrome', // browser name is required
     },
   }
 })
 ```
 
 ## Browser Option Types:
 
-The browser option in Vitest can be set to either a boolean or a string type. If set to `true`, Vitest will try to automatically find your default browser. You can also specify a browser by providing its name as a `string`. The available browser options are:
-- `firefox`
-- `chrome`
-- `edge`
-- `safari`
-
-Here's an example configuration setting chrome as the browser option:
+The browser option in Vitest depends on the provider. Vitest will fail, if you pass `--browser` and don't specify its name in the config file. Available options:
 
-```ts
-export default defineConfig({
-  test: {
-    browser: {
-      enabled: true,
-      name: 'chrome',
-    },
-  }
-})
-```
+- `webdriverio` (default) supports these browsers:
+  - `firefox`
+  - `chrome`
+  - `edge`
+  - `safari`
+- `playwright` supports these browsers:
+  - `firefox`
+  - `webkit`
+  - `chromium`
 
 ## Cross-browser Testing:
 
@@ -58,7 +52,7 @@ npx vitest --browser.name=chrome --browser.headless
 ```
 
 ::: tip NOTE
-When using the Safari browser option, the `safaridriver` needs to be activated by running `sudo safaridriver --enable` on your device.
+When using the Safari browser option with WebdriverIO, the `safaridriver` needs to be activated by running `sudo safaridriver --enable` on your device.
 
 Additionally, when running your tests, Vitest will attempt to install some drivers for compatibility with `safaridriver`.
 :::

diff --git a/package.json b/package.json
@@ -29,8 +29,8 @@
     "ui:build": "vite build packages/ui",
     "ui:dev": "vite packages/ui",
     "ui:test": "npm -C packages/ui run test:run",
-    "browser:test": "npm -C test/browser run test",
-    "browser:playwright:test": "npm -C test/browser run test:playwright"
+    "test:browser:webdriverio": "npm -C test/browser run test:webdriverio",
+    "test:browser:playwright": "npm -C test/browser run test:playwright"
   },
   "devDependencies": {
     "@antfu/eslint-config": "^0.34.1",

diff --git a/packages/vitest/src/api/setup.ts b/packages/vitest/src/api/setup.ts
@@ -36,7 +36,7 @@ export function setup(ctx: Vitest, server?: ViteDevServer) {
     const rpc = createBirpc<WebSocketEvents, WebSocketHandlers>(
       {
         async onDone(testId) {
-          await ctx.browserProvider?.testFinished?.(testId)
+          return ctx.state.browserTestPromises.get(testId)?.resolve(true)
         },
         async onCollected(files) {
           ctx.state.collectFiles(files)

diff --git a/packages/vitest/src/defaults.ts b/packages/vitest/src/defaults.ts
@@ -39,11 +39,6 @@ export const coverageConfigDefaults: ResolvedCoverageOptions = {
   extension: ['.js', '.cjs', '.mjs', '.ts', '.mts', '.cts', '.tsx', '.jsx', '.vue', '.svelte'],
 }
 
-export const browserConfigDefaults = {
-  enabled: false,
-  headless: isCI,
-} as const
-
 export const fakeTimersDefaults = {
   loopLimit: 10_000,
   shouldClearNativeTimers: true,
@@ -73,7 +68,6 @@ const config = {
   hookTimeout: 10000,
   teardownTimeout: 10000,
   isolate: true,
-  browser: browserConfigDefaults,
   watchExclude: ['**/node_modules/**', '**/dist/**'],
   forceRerunTriggers: [
     '**/package.json/**',

diff --git a/packages/vitest/src/node/browser/playwright.ts b/packages/vitest/src/node/browser/playwright.ts
@@ -1,42 +1,36 @@
 import type { Page } from 'playwright'
-import type { Awaitable } from '@vitest/utils'
-import { createDefer } from '@vitest/utils'
-import { relative } from 'pathe'
-import type { BrowserProvider } from '../../types/browser'
+import type { BrowserProvider, BrowserProviderOptions } from '../../types/browser'
 import { ensurePackageInstalled } from '../pkg'
 import type { Vitest } from '../core'
 
+export const playwrightBrowsers = ['firefox', 'webkit', 'chromium'] as const
+export type PlaywrightBrowser = typeof playwrightBrowsers[number]
+
+export interface PlaywrightProviderOptions extends BrowserProviderOptions {
+  browser: PlaywrightBrowser
+}
+
 export class PlaywrightBrowserProvider implements BrowserProvider {
-  private supportedBrowsers = ['firefox', 'webkit', 'chromium'] as const
+  public name = 'playwright'
+
   private cachedBrowser: Page | null = null
-  private testDefers = new Map<string, ReturnType<typeof createDefer>>()
-  private host = ''
-  private browser!: typeof this.supportedBrowsers[number]
+  private browser!: PlaywrightBrowser
   private ctx!: Vitest
 
-  async initialize(ctx: Vitest) {
+  getSupportedBrowsers() {
+    return playwrightBrowsers
+  }
+
+  async initialize(ctx: Vitest, { browser }: PlaywrightProviderOptions) {
     this.ctx = ctx
-    this.host = `http://${ctx.config.browser.api?.host || 'localhost'}:${ctx.browser.config.server.port}`
+    this.browser = browser
 
     const root = this.ctx.config.root
-    const browser = this.getBrowserName()
-
-    this.browser = browser as any
-
-    if (!browser)
-      throw new Error('Cannot detect browser. Please specify browser.name in the config file.')
-
-    if (!this.supportedBrowsers.includes(this.browser))
-      throw new Error(`Playwright provider does not support this browser, and only supports these browsers: ${this.supportedBrowsers.join(', ')}`)
 
     if (!await ensurePackageInstalled('playwright', root))
       throw new Error('Cannot find "webdriverio" package. Please install it manually.')
   }
 
-  getBrowserName() {
-    return this.ctx.config.browser.name
-  }
-
   async openBrowser() {
     if (this.cachedBrowser)
       return this.cachedBrowser
@@ -55,57 +49,15 @@ export class PlaywrightBrowserProvider implements BrowserProvider {
     return this.cachedBrowser
   }
 
-  testFinished(id: string): Awaitable<void> {
-    this.testDefers.get(id)?.resolve(true)
-  }
-
-  private waitForTest(id: string) {
-    const defer = createDefer()
-    this.testDefers.set(id, defer)
-    return defer
+  async openPage(url: string) {
+    const browserInstance = await this.openBrowser()
+    await browserInstance.goto(url)
   }
 
-  createPool() {
-    const close = async () => {
-      this.testDefers.clear()
-      await Promise.all([
-        this.cachedBrowser?.close(),
-      ])
-      // TODO: right now process can only exit with timeout, if we use browser
-      // needs investigating
-      process.exit()
-    }
-
-    const runTests = async (files: string[]) => {
-      const paths = files.map(file => relative(this.ctx.config.root, file))
-      const browserInstance = await this.openBrowser()
-
-      const isolate = this.ctx.config.isolate
-      if (isolate) {
-        for (const path of paths) {
-          const url = new URL(this.host)
-          url.searchParams.append('path', path)
-          url.searchParams.set('id', path)
-          await browserInstance.goto(url.toString())
-          await this.waitForTest(path)
-        }
-      }
-      else {
-        const url = new URL(this.host)
-        url.searchParams.set('id', 'no-isolate')
-        paths.forEach(path => url.searchParams.append('path', path))
-        await browserInstance.goto(url.toString())
-        await this.waitForTest('no-isolate')
-      }
-      browserInstance.on('close', () => {
-        // if the user closes the browser, then close vitest too
-        close()
-      })
-    }
-
-    return {
-      runTests,
-      close,
-    }
+  async close() {
+    await this.cachedBrowser?.close()
+    // TODO: right now process can only exit with timeout, if we use browser
+    // needs investigating
+    process.exit()
   }
 }