From ed4e042498d8504d185f95de11bd03d97e010ced Mon Sep 17 00:00:00 2001 From: AriPerkkio Date: Tue, 4 Jul 2023 15:18:55 +0300 Subject: [PATCH] fix: isolate workers between envs and workspaces --- packages/vitest/package.json | 2 +- packages/vitest/src/node/pools/threads.ts | 36 ++++++++++++++----- pnpm-lock.yaml | 20 ++++++++--- .../fixtures/project/test/happy-dom.test.ts | 11 ++++++ .../fixtures/project/test/jsdom.test.ts | 11 ++++++ .../fixtures/project/test/node.test.ts | 11 ++++++ .../project/test/workspace-project.test.ts | 7 ++++ test/env-mixed/fixtures/vitest.config.ts | 5 +++ test/env-mixed/fixtures/vitest.workspace.ts | 21 +++++++++++ test/env-mixed/package.json | 12 +++++++ .../env-mixed/test/mixed-environments.test.ts | 26 ++++++++++++++ test/env-mixed/vitest.config.ts | 11 ++++++ 12 files changed, 160 insertions(+), 13 deletions(-) create mode 100644 test/env-mixed/fixtures/project/test/happy-dom.test.ts create mode 100644 test/env-mixed/fixtures/project/test/jsdom.test.ts create mode 100644 test/env-mixed/fixtures/project/test/node.test.ts create mode 100644 test/env-mixed/fixtures/project/test/workspace-project.test.ts create mode 100644 test/env-mixed/fixtures/vitest.config.ts create mode 100644 test/env-mixed/fixtures/vitest.workspace.ts create mode 100644 test/env-mixed/package.json create mode 100644 test/env-mixed/test/mixed-environments.test.ts create mode 100644 test/env-mixed/vitest.config.ts diff --git a/packages/vitest/package.json b/packages/vitest/package.json index 48ee405aec11..b5acbf00a7f6 100644 --- a/packages/vitest/package.json +++ b/packages/vitest/package.json @@ -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" diff --git a/packages/vitest/src/node/pools/threads.ts b/packages/vitest/src/node/pools/threads.ts index cd1b293d4bc4..b405f2052812 100644 --- a/packages/vitest/src/node/pools/threads.ts +++ b/packages/vitest/src/node/pools/threads.ts @@ -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[] = [] + + 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(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) @@ -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] @@ -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) + } } } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9038c09db953..c6284bde59d5 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1335,8 +1335,8 @@ importers: specifier: ^2.5.0 version: 2.5.0 tinypool: - specifier: ^0.6.0 - version: 0.6.0 + specifier: ^0.7.0 + version: 0.7.0 vite: specifier: ^4.3.9 version: 4.3.9(@types/node@18.7.13) @@ -1686,6 +1686,18 @@ importers: specifier: workspace:* version: link:../../packages/vitest + test/env-mixed: + devDependencies: + happy-dom: + specifier: latest + version: 9.20.3 + vite: + specifier: ^4.3.9 + version: 4.3.9(@types/node@18.16.19) + vitest: + specifier: workspace:* + version: link:../../packages/vitest + test/esm: dependencies: css-what: @@ -23640,8 +23652,8 @@ packages: resolution: {integrity: sha512-kRwSG8Zx4tjF9ZiyH4bhaebu+EDz1BOx9hOigYHlUW4xxI/wKIUQUqo018UlU4ar6ATPBsaMrdbKZ+tmPdohFA==} dev: false - /tinypool@0.6.0: - resolution: {integrity: sha512-FdswUUo5SxRizcBc6b1GSuLpLjisa8N8qMyYoP3rl+bym+QauhtJP5bvZY1ytt8krKGmMLYIRl36HBZfeAoqhQ==} + /tinypool@0.7.0: + resolution: {integrity: sha512-zSYNUlYSMhJ6Zdou4cJwo/p7w5nmAH17GRfU/ui3ctvjXFErXXkruT4MWW6poDeXgCaIBlGLrfU6TbTXxyGMww==} engines: {node: '>=14.0.0'} dev: false diff --git a/test/env-mixed/fixtures/project/test/happy-dom.test.ts b/test/env-mixed/fixtures/project/test/happy-dom.test.ts new file mode 100644 index 000000000000..3eb25492b813 --- /dev/null +++ b/test/env-mixed/fixtures/project/test/happy-dom.test.ts @@ -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) +}) diff --git a/test/env-mixed/fixtures/project/test/jsdom.test.ts b/test/env-mixed/fixtures/project/test/jsdom.test.ts new file mode 100644 index 000000000000..683378015016 --- /dev/null +++ b/test/env-mixed/fixtures/project/test/jsdom.test.ts @@ -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) +}) diff --git a/test/env-mixed/fixtures/project/test/node.test.ts b/test/env-mixed/fixtures/project/test/node.test.ts new file mode 100644 index 000000000000..dc6bf00fba82 --- /dev/null +++ b/test/env-mixed/fixtures/project/test/node.test.ts @@ -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) +}) diff --git a/test/env-mixed/fixtures/project/test/workspace-project.test.ts b/test/env-mixed/fixtures/project/test/workspace-project.test.ts new file mode 100644 index 000000000000..4c28414ca528 --- /dev/null +++ b/test/env-mixed/fixtures/project/test/workspace-project.test.ts @@ -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' +}) diff --git a/test/env-mixed/fixtures/vitest.config.ts b/test/env-mixed/fixtures/vitest.config.ts new file mode 100644 index 000000000000..117a67645f43 --- /dev/null +++ b/test/env-mixed/fixtures/vitest.config.ts @@ -0,0 +1,5 @@ +import { defineConfig } from 'vitest/config' + +export default defineConfig({ + test: {}, +}) diff --git a/test/env-mixed/fixtures/vitest.workspace.ts b/test/env-mixed/fixtures/vitest.workspace.ts new file mode 100644 index 000000000000..c88592f946c6 --- /dev/null +++ b/test/env-mixed/fixtures/vitest.workspace.ts @@ -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'), + }, + }, +]) diff --git a/test/env-mixed/package.json b/test/env-mixed/package.json new file mode 100644 index 000000000000..2a771f319b9a --- /dev/null +++ b/test/env-mixed/package.json @@ -0,0 +1,12 @@ +{ + "name": "@vitest/test-env-mixed", + "private": true, + "scripts": { + "test": "vitest" + }, + "devDependencies": { + "happy-dom": "latest", + "vite": "latest", + "vitest": "workspace:*" + } +} diff --git a/test/env-mixed/test/mixed-environments.test.ts b/test/env-mixed/test/mixed-environments.test.ts new file mode 100644 index 000000000000..7b8051eac7b1 --- /dev/null +++ b/test/env-mixed/test/mixed-environments.test.ts @@ -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)') +}) diff --git a/test/env-mixed/vitest.config.ts b/test/env-mixed/vitest.config.ts new file mode 100644 index 000000000000..e8f66bb7fb92 --- /dev/null +++ b/test/env-mixed/vitest.config.ts @@ -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, + }, + }, +})