Skip to content

Commit

Permalink
fix: isolate workers between envs and workspaces
Browse files Browse the repository at this point in the history
  • Loading branch information
AriPerkkio authored and sheremet-va committed Jul 11, 2023
1 parent 8dd5ea5 commit ed4e042
Show file tree
Hide file tree
Showing 12 changed files with 160 additions and 13 deletions.
2 changes: 1 addition & 1 deletion packages/vitest/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@
"std-env": "^3.3.3",
"strip-literal": "^1.0.1",
"tinybench": "^2.5.0",
"tinypool": "^0.6.0",
"tinypool": "^0.7.0",
"vite": "^3.0.0 || ^4.0.0",
"vite-node": "workspace:*",
"why-is-node-running": "^2.2.2"
Expand Down
36 changes: 28 additions & 8 deletions packages/vitest/src/node/pools/threads.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,29 @@ export function createThreadsPool(ctx: Vitest, { execArgv, env }: PoolProcessOpt

if (multipleThreads.length) {
const filesByEnv = await groupFilesByEnv(multipleThreads)
const promises = Object.values(filesByEnv).flat()
const results = await Promise.allSettled(promises
.map(({ file, environment, project }) => runFiles(project, getConfig(project), [file], environment, invalidates)))
const files = Object.values(filesByEnv).flat()
const results: PromiseSettledResult<void>[] = []

if (ctx.config.isolate) {
results.push(...await Promise.allSettled(files.map(({ file, environment, project }) =>
runFiles(project, getConfig(project), [file], environment, invalidates))))
}
else {
// When isolation is disabled, we still need to isolate environments and workspace projects from each other.
// Tasks are still running parallel but environments are isolated between tasks.
const grouped = groupBy(files, ({ project, environment }) => project.getName() + environment.name + JSON.stringify(environment.options))

for (const group of Object.values(grouped)) {
// Push all files to pool's queue
results.push(...await Promise.allSettled(group.map(({ file, environment, project }) =>
runFiles(project, getConfig(project), [file], environment, invalidates))))

// Once all tasks are running or finished, recycle worker for isolation.
// On-going workers will run in the previous environment.
await new Promise<void>(resolve => pool.queueSize === 0 ? resolve() : pool.once('drain', resolve))
await pool.recycleWorkers()
}
}

const errors = results.filter((r): r is PromiseRejectedResult => r.status === 'rejected').map(r => r.reason)
if (errors.length > 0)
Expand All @@ -162,7 +182,6 @@ export function createThreadsPool(ctx: Vitest, { execArgv, env }: PoolProcessOpt
Object.keys(filesByEnv).filter(env => !envsOrder.includes(env)),
)

// always run environments isolated between each other
for (const env of envs) {
const files = filesByEnv[env]

Expand All @@ -171,12 +190,13 @@ export function createThreadsPool(ctx: Vitest, { execArgv, env }: PoolProcessOpt

const filesByOptions = groupBy(files, ({ project, environment }) => project.getName() + JSON.stringify(environment.options))

const promises = Object.values(filesByOptions).map(async (files) => {
for (const files of Object.values(filesByOptions)) {
// Always run environments isolated between each other
await pool.recycleWorkers()

const filenames = files.map(f => f.file)
await runFiles(files[0].project, getConfig(files[0].project), filenames, files[0].environment, invalidates)
})

await Promise.all(promises)
}
}
}
}
Expand Down
20 changes: 16 additions & 4 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions test/env-mixed/fixtures/project/test/happy-dom.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* @vitest-environment happy-dom
*/

import { expect, test } from 'vitest'

test('Leaking globals not found', async () => {
(globalThis as any).__leaking_from_happy_dom = 'leaking'
expect((globalThis as any).__leaking_from_node).toBe(undefined)
expect((globalThis as any).__leaking_from_jsdom).toBe(undefined)
})
11 changes: 11 additions & 0 deletions test/env-mixed/fixtures/project/test/jsdom.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* @vitest-environment jsdom
*/

import { expect, test } from 'vitest'

test('Leaking globals not found', async () => {
(globalThis as any).__leaking_from_jsdom = 'leaking'
expect((globalThis as any).__leaking_from_node).toBe(undefined)
expect((globalThis as any).__leaking_from_happy_dom).toBe(undefined)
})
11 changes: 11 additions & 0 deletions test/env-mixed/fixtures/project/test/node.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* @vitest-environment node
*/

import { expect, test } from 'vitest'

test('Leaking globals not found', async () => {
(globalThis as any).__leaking_from_node = 'leaking'
expect((globalThis as any).__leaking_from_jsdom).toBe(undefined)
expect((globalThis as any).__leaking_from_happy_dom).toBe(undefined)
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { expect, test } from 'vitest'

test('Leaking globals not found', async () => {
expect((globalThis as any).__leaking_from_workspace_project).toBe(undefined)

;(globalThis as any).__leaking_from_workspace_project = 'leaking'
})
5 changes: 5 additions & 0 deletions test/env-mixed/fixtures/vitest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { defineConfig } from 'vitest/config'

export default defineConfig({
test: {},
})
21 changes: 21 additions & 0 deletions test/env-mixed/fixtures/vitest.workspace.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { resolve } from 'node:path'
import { fileURLToPath } from 'node:url'
import { defineWorkspace } from 'vitest/config'

const __filename = fileURLToPath(import.meta.url)
const __dirname = resolve(__filename, '..')

export default defineWorkspace([
{
test: {
name: 'Project #1',
root: resolve(__dirname, './project'),
},
},
{
test: {
name: 'Project #2',
root: resolve(__dirname, './project'),
},
},
])
12 changes: 12 additions & 0 deletions test/env-mixed/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"name": "@vitest/test-env-mixed",
"private": true,
"scripts": {
"test": "vitest"
},
"devDependencies": {
"happy-dom": "latest",
"vite": "latest",
"vitest": "workspace:*"
}
}
26 changes: 26 additions & 0 deletions test/env-mixed/test/mixed-environments.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { type UserConfig, expect, test } from 'vitest'

import { runVitest } from '../../test-utils'

const configs: UserConfig[] = [
{ isolate: false, singleThread: true },
{ isolate: false, singleThread: false },
{ isolate: false, threads: true, minThreads: 1, maxThreads: 1 },
{ isolate: false, threads: false },
]

test.each(configs)('should isolate environments when %s', async (config) => {
const { stderr, stdout } = await runVitest({
root: './fixtures',
...config,
})

expect(stderr).toBe('')

expect(stdout).toContain('✓ test/node.test.ts')
expect(stdout).toContain('✓ test/jsdom.test.ts')
expect(stdout).toContain('✓ test/happy-dom.test.ts')
expect(stdout).toContain('✓ test/workspace-project.test.ts')
expect(stdout).toContain('Test Files 8 passed (8)')
expect(stdout).toContain('Tests 8 passed (8)')
})
11 changes: 11 additions & 0 deletions test/env-mixed/vitest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { defineConfig } from 'vitest/config'

export default defineConfig({
test: {
include: ['./test/**/*'],
testTimeout: process.env.CI ? 30_000 : 10_000,
chaiConfig: {
truncateThreshold: 0,
},
},
})

0 comments on commit ed4e042

Please sign in to comment.