Skip to content

Commit

Permalink
fix: remove browser.fileParallelism (#5790)
Browse files Browse the repository at this point in the history
  • Loading branch information
sheremet-va committed May 29, 2024
1 parent d6700bb commit b881e88
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 71 deletions.
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".')
})
})

0 comments on commit b881e88

Please sign in to comment.