Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove browser.fileParallelism #5790

Merged
merged 2 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions docs/config/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -1536,20 +1536,6 @@ Run the browser in a `headless` mode. If you are running Vitest in CI, it will b

Run every test in a separate iframe.

### browser.fileParallelism {#browser-fileparallelism}

- **Type:** `boolean`
- **Default:** the same as [`fileParallelism`](#fileparallelism)
- **CLI:** `--browser.fileParallelism=false`

Create all test iframes at the same time so they are running in parallel.

This makes it impossible to use interactive APIs (like clicking or hovering) because there are several iframes on the screen at the same time, but if your tests don't rely on those APIs, it might be much faster to just run all of them at the same time.

::: tip
If you disabled isolation via [`browser.isolate=false`](#browser-isolate), your test files will still run one after another because of the nature of the test runner.
:::

#### browser.api

- **Type:** `number | { port?, strictPort?, host? }`
Expand Down
1 change: 0 additions & 1 deletion docs/guide/cli-table.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
| `--browser.providerOptions <options>` | Options that are passed down to a browser provider. Visit [`browser.providerOptions`](https://vitest.dev/config/#browser-provideroptions) for more information |
| `--browser.slowHijackESM` | Let Vitest use its own module resolution on the browser to enable APIs such as vi.mock and vi.spyOn. Visit [`browser.slowHijackESM`](https://vitest.dev/config/#browser-slowhijackesm) for more information (default: `false`) |
| `--browser.isolate` | Run every browser test file in isolation. To disable isolation, use `--browser.isolate=false` (default: `true`) |
| `--browser.fileParallelism` | Should all test files run in parallel. Use `--browser.file-parallelism=false` to disable (default: same as `--file-parallelism`) |
| `--pool <pool>` | Specify pool, if not running in the browser (default: `threads`) |
| `--poolOptions.threads.isolate` | Isolate tests in threads pool (default: `true`) |
| `--poolOptions.threads.singleThread` | Run tests inside a single thread (default: `false`) |
Expand Down
43 changes: 15 additions & 28 deletions packages/browser/src/client/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,42 +100,29 @@ client.ws.addEventListener('open', async () => {
}
})

const fileParallelism = config.browser.fileParallelism ?? config.fileParallelism

if (config.isolate === false) {
createIframe(
container,
ID_ALL,
)
}
else {
// if fileParallelism is enabled, we can create all iframes at once
if (fileParallelism) {
for (const file of testFiles) {
createIframe(
container,
file,
)
}
}
else {
// otherwise, we need to wait for each iframe to finish before creating the next one
// this is the most stable way to run tests in the browser
for (const file of testFiles) {
createIframe(
container,
file,
)
await new Promise<void>((resolve) => {
channel.addEventListener('message', function handler(e: MessageEvent<IframeChannelEvent>) {
// done and error can only be triggered by the previous iframe
if (e.data.type === 'done' || e.data.type === 'error') {
channel.removeEventListener('message', handler)
resolve()
}
})
// otherwise, we need to wait for each iframe to finish before creating the next one
// this is the most stable way to run tests in the browser
for (const file of testFiles) {
createIframe(
container,
file,
)
await new Promise<void>((resolve) => {
channel.addEventListener('message', function handler(e: MessageEvent<IframeChannelEvent>) {
// done and error can only be triggered by the previous iframe
if (e.data.type === 'done' || e.data.type === 'error') {
channel.removeEventListener('message', handler)
resolve()
}
})
}
})
}
}
})
3 changes: 0 additions & 3 deletions packages/vitest/src/node/cli/cli-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -350,9 +350,6 @@ export const cliOptionsConfig: VitestCLIOptions = {
ui: {
description: 'Show Vitest UI when running tests',
},
fileParallelism: {
description: 'Should all test files run in parallel. Use `--browser.file-parallelism=false` to disable (default: same as `--file-parallelism`)',
},
indexScripts: null,
testerScripts: null,
commands: null,
Expand Down
1 change: 0 additions & 1 deletion packages/vitest/src/node/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ export function resolveConfig(
resolved.minWorkers = Number(resolved.minWorkers)

resolved.browser ??= {} as any
resolved.browser.fileParallelism ??= resolved.fileParallelism ?? false

// run benchmark sequentially by default
resolved.fileParallelism ??= mode !== 'benchmark'
Expand Down
7 changes: 0 additions & 7 deletions packages/vitest/src/types/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,6 @@ export interface BrowserConfigOptions {
*/
ui?: boolean

/**
* Run test files in parallel. Fallbacks to `test.fileParallelism`.
*
* @default test.fileParallelism
*/
fileParallelism?: boolean

/**
* Scripts injected into the tester iframe.
*/
Expand Down
27 changes: 10 additions & 17 deletions test/browser/specs/runner.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
import { beforeAll, describe, expect, onTestFailed, test } from 'vitest'
import { runBrowserTests } from './utils'

describe.each([
['non parallel', false],
['parallel', true],
])('[%s] running browser tests', async (description, fileParallelism) => {
describe('running browser tests', async () => {
let stderr: string
let stdout: string
let browserResultJson: any
Expand All @@ -18,14 +15,10 @@ describe.each([
browserResultJson,
passedTests,
failedTests,
} = await runBrowserTests({
browser: {
fileParallelism,
},
}))
} = await runBrowserTests())
})

test(`[${description}] tests are actually running`, () => {
test('tests are actually running', () => {
onTestFailed(() => {
console.error(stderr)
})
Expand All @@ -38,14 +31,14 @@ describe.each([
expect(stderr).not.toContain('Unhandled Error')
})

test(`[${description}] correctly prints error`, () => {
test('correctly prints error', () => {
expect(stderr).toContain('expected 1 to be 2')
expect(stderr).toMatch(/- 2\s+\+ 1/)
expect(stderr).toContain('Expected to be')
expect(stderr).toContain('But got')
})

test(`[${description}] logs are redirected to stdout`, () => {
test('logs are redirected to stdout', () => {
expect(stdout).toContain('stdout | test/logs.test.ts > logging to stdout')
expect(stdout).toContain('hello from console.log')
expect(stdout).toContain('hello from console.info')
Expand All @@ -65,26 +58,26 @@ describe.each([
expect(stdout).toMatch(/time: [\d.]+ ms/)
})

test(`[${description}] logs are redirected to stderr`, () => {
test('logs are redirected to stderr', () => {
expect(stderr).toContain('stderr | test/logs.test.ts > logging to stderr')
expect(stderr).toContain('hello from console.error')
expect(stderr).toContain('hello from console.warn')
expect(stderr).toContain('Timer "invalid timeLog" does not exist')
expect(stderr).toContain('Timer "invalid timeEnd" does not exist')
})

test(`[${description}] stack trace points to correct file in every browser`, () => {
// dependeing on the browser it references either `.toBe()` or `expect()`
test('stack trace points to correct file in every browser', () => {
// dependeing on the browser it references either '.toBe()' or 'expect()'
expect(stderr).toMatch(/test\/failing.test.ts:4:(12|17)/)
})

test(`[${description}] popup apis should log a warning`, () => {
test('popup apis should log a warning', () => {
expect(stderr).toContain('Vitest encountered a `alert("test")`')
expect(stderr).toContain('Vitest encountered a `confirm("test")`')
expect(stderr).toContain('Vitest encountered a `prompt("test")`')
})

test(`[${description}] snapshot inaccessible file debuggability`, () => {
test('snapshot inaccessible file debuggability', () => {
expect(stderr).toContain('Access denied to "/inaccesible/path".')
})
})
Loading