From 9021e8b84bec120e5c7546d6e0aed88db3b60f3c Mon Sep 17 00:00:00 2001 From: Adrien Cacciaguerra Date: Tue, 31 Oct 2023 16:30:44 +0100 Subject: [PATCH] feat(vitest): expose execArgv to the different pools (#4383) --- docs/config/index.md | 33 +++++++++ packages/vitest/src/node/pool.ts | 2 +- packages/vitest/src/node/pools/child.ts | 5 +- packages/vitest/src/node/pools/threads.ts | 5 +- packages/vitest/src/node/pools/vm-threads.ts | 1 + packages/vitest/src/types/pool-options.ts | 26 +++++++ pnpm-lock.yaml | 3 + .../allowed-exec-argv.test.ts | 16 +++++ .../vitest.config.ts | 3 + test/run/exec-args-fixtures/forks.test.ts | 13 ++++ test/run/exec-args-fixtures/threads.test.ts | 11 +++ test/run/exec-args-fixtures/vmThreads.test.ts | 15 ++++ .../no-exec-argv.test.ts | 12 ++++ .../no-exec-args-fixtures/vitest.config.ts | 3 + test/run/package.json | 1 + test/run/test/exec-args.test.ts | 69 +++++++++++++++++++ 16 files changed, 215 insertions(+), 3 deletions(-) create mode 100644 test/run/allowed-exec-args-fixtures/allowed-exec-argv.test.ts create mode 100644 test/run/allowed-exec-args-fixtures/vitest.config.ts create mode 100644 test/run/exec-args-fixtures/forks.test.ts create mode 100644 test/run/exec-args-fixtures/threads.test.ts create mode 100644 test/run/exec-args-fixtures/vmThreads.test.ts create mode 100644 test/run/no-exec-args-fixtures/no-exec-argv.test.ts create mode 100644 test/run/no-exec-args-fixtures/vitest.config.ts create mode 100644 test/run/test/exec-args.test.ts diff --git a/docs/config/index.md b/docs/config/index.md index 0a391b5628dc..273a519b3017 100644 --- a/docs/config/index.md +++ b/docs/config/index.md @@ -723,6 +723,17 @@ This can improve performance in some cases, but might cause segfault in older No Isolate environment for each test file. +##### poolOptions.threads.execArgv + +- **Type:** `string[]` +- **Default:** `[]` + +Pass additional arguments to `node` in the threads. See [Command-line API | Node.js](https://nodejs.org/docs/latest/api/cli.html) for more information. + +:::warning +Be careful when using, it as some options may crash worker, e.g. --prof, --title. See https://github.com/nodejs/node/issues/41103. +::: + #### poolOptions.forks Options for `forks` pool. @@ -776,6 +787,17 @@ Even though this option will force tests to run one after another, this option i This might cause all sorts of issues, if you are relying on global state (frontend frameworks usually do) or your code relies on environment to be defined separately for each test. But can be a speed boost for your tests (up to 3 times faster), that don't necessarily rely on global state or can easily bypass that. ::: +##### poolOptions.forks.execArgv + +- **Type:** `string[]` +- **Default:** `[]` + +Pass additional arguments to `node` process in the child processes. See [Command-line API | Node.js](https://nodejs.org/docs/latest/api/cli.html) for more information. + +:::warning +Be careful when using, it as some options may crash worker, e.g. --prof, --title. See https://github.com/nodejs/node/issues/41103. +::: + #### poolOptions.vmThreads Options for `vmThreads` pool. @@ -846,6 +868,17 @@ Use Atomics to synchronize threads. This can improve performance in some cases, but might cause segfault in older Node versions. +##### poolOptions.vmThreads.execArgv + +- **Type:** `string[]` +- **Default:** `[]` + +Pass additional arguments to `node` process in the VM context. See [Command-line API | Node.js](https://nodejs.org/docs/latest/api/cli.html) for more information. + +:::warning +Be careful when using, it as some options may crash worker, e.g. --prof, --title. See https://github.com/nodejs/node/issues/41103. +::: + ### testTimeout - **Type:** `number` diff --git a/packages/vitest/src/node/pool.ts b/packages/vitest/src/node/pool.ts index 3f13a2c6ebbc..f44c250709e3 100644 --- a/packages/vitest/src/node/pool.ts +++ b/packages/vitest/src/node/pool.ts @@ -63,7 +63,7 @@ export function createPool(ctx: Vitest): ProcessPool { // Instead of passing whole process.execArgv to the workers, pick allowed options. // Some options may crash worker, e.g. --prof, --title. nodejs/node#41103 const execArgv = process.execArgv.filter(execArg => - execArg.startsWith('--cpu-prof') || execArg.startsWith('--heap-prof'), + execArg.startsWith('--cpu-prof') || execArg.startsWith('--heap-prof') || execArg.startsWith('--diagnostic-dir'), ) const options: PoolProcessOptions = { diff --git a/packages/vitest/src/node/pools/child.ts b/packages/vitest/src/node/pools/child.ts index 6f6cc7351c2b..f1eddcd6dde5 100644 --- a/packages/vitest/src/node/pools/child.ts +++ b/packages/vitest/src/node/pools/child.ts @@ -69,7 +69,10 @@ export function createChildProcessPool(ctx: Vitest, { execArgv, env, forksPath } minThreads, env, - execArgv, + execArgv: [ + ...ctx.config.poolOptions?.forks?.execArgv ?? [], + ...execArgv, + ], terminateTimeout: ctx.config.teardownTimeout, } diff --git a/packages/vitest/src/node/pools/threads.ts b/packages/vitest/src/node/pools/threads.ts index 9e8023c8656a..7b1b8040f54a 100644 --- a/packages/vitest/src/node/pools/threads.ts +++ b/packages/vitest/src/node/pools/threads.ts @@ -56,7 +56,10 @@ export function createThreadsPool(ctx: Vitest, { execArgv, env, workerPath }: Po minThreads, env, - execArgv, + execArgv: [ + ...ctx.config.poolOptions?.threads?.execArgv ?? [], + ...execArgv, + ], terminateTimeout: ctx.config.teardownTimeout, } diff --git a/packages/vitest/src/node/pools/vm-threads.ts b/packages/vitest/src/node/pools/vm-threads.ts index 35a1c6ae5f7d..5c8a298d4931 100644 --- a/packages/vitest/src/node/pools/vm-threads.ts +++ b/packages/vitest/src/node/pools/vm-threads.ts @@ -66,6 +66,7 @@ export function createVmThreadsPool(ctx: Vitest, { execArgv, env, vmPath }: Pool '--experimental-vm-modules', '--require', suppressWarningsPath, + ...ctx.config.poolOptions?.vmThreads?.execArgv ?? [], ...execArgv, ], diff --git a/packages/vitest/src/types/pool-options.ts b/packages/vitest/src/types/pool-options.ts index eb874614e599..30c6f0115f3e 100644 --- a/packages/vitest/src/types/pool-options.ts +++ b/packages/vitest/src/types/pool-options.ts @@ -81,6 +81,19 @@ interface WorkerContextOptions { * @default true */ isolate?: boolean + + /** + * Pass additional arguments to `node` process when spawning `worker_threads` or `child_process`. + * + * See [Command-line API | Node.js](https://nodejs.org/docs/latest/api/cli.html) for more information. + * + * Set to `process.execArgv` to pass all arguments of the current process. + * + * Be careful when using, it as some options may crash worker, e.g. --prof, --title. See https://github.com/nodejs/node/issues/41103 + * + * @default [] // no execution arguments are passed + */ + execArgv?: string[] } interface VmOptions { @@ -92,4 +105,17 @@ interface VmOptions { /** Isolation is always enabled */ isolate?: true + + /** + * Pass additional arguments to `node` process when spawning `worker_threads` or `child_process`. + * + * See [Command-line API | Node.js](https://nodejs.org/docs/latest/api/cli.html) for more information. + * + * Set to `process.execArgv` to pass all arguments of the current process. + * + * Be careful when using, it as some options may crash worker, e.g. --prof, --title. See https://github.com/nodejs/node/issues/41103 + * + * @default [] // no execution arguments are passed + */ + execArgv?: string[] } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9f34f99c9fe3..ed7acbcae594 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1886,6 +1886,9 @@ importers: test/run: devDependencies: + execa: + specifier: ^7.1.1 + version: 7.1.1 vite: specifier: ^4.5.0 version: 4.5.0(@types/node@18.16.19)(less@4.1.3) diff --git a/test/run/allowed-exec-args-fixtures/allowed-exec-argv.test.ts b/test/run/allowed-exec-args-fixtures/allowed-exec-argv.test.ts new file mode 100644 index 000000000000..7835748e07eb --- /dev/null +++ b/test/run/allowed-exec-args-fixtures/allowed-exec-argv.test.ts @@ -0,0 +1,16 @@ +import { describe, expect, it } from 'vitest' + +describe('exec-args', async () => { + it('should have the correct flags', () => { + // flags that should go through + expect(process.execArgv).toContain('--cpu-prof') + expect(process.execArgv).toContain('--cpu-prof-name=cpu.prof') + expect(process.execArgv).toContain('--heap-prof') + expect(process.execArgv).toContain('--heap-prof-name=heap.prof') + expect(process.execArgv).toContain('--diagnostic-dir=/tmp/vitest-diagnostics') + + // added via vitest + expect(process.execArgv).toContain('--conditions') + expect(process.execArgv).toContain('node') + }) +}) diff --git a/test/run/allowed-exec-args-fixtures/vitest.config.ts b/test/run/allowed-exec-args-fixtures/vitest.config.ts new file mode 100644 index 000000000000..abed6b2116e1 --- /dev/null +++ b/test/run/allowed-exec-args-fixtures/vitest.config.ts @@ -0,0 +1,3 @@ +import { defineConfig } from 'vitest/config' + +export default defineConfig({}) diff --git a/test/run/exec-args-fixtures/forks.test.ts b/test/run/exec-args-fixtures/forks.test.ts new file mode 100644 index 000000000000..758837b1992d --- /dev/null +++ b/test/run/exec-args-fixtures/forks.test.ts @@ -0,0 +1,13 @@ +import { describe, expect, it } from 'vitest' + +describe('exec-args', async () => { + it('should have the correct flags', () => { + expect(process.execArgv).toContain('--hash-seed=1') + expect(process.execArgv).toContain('--random-seed=1') + expect(process.execArgv).toContain('--no-opt') + + // added via vitest + expect(process.execArgv).toContain('--conditions') + expect(process.execArgv).toContain('node') + }) +}) diff --git a/test/run/exec-args-fixtures/threads.test.ts b/test/run/exec-args-fixtures/threads.test.ts new file mode 100644 index 000000000000..779e4f106e38 --- /dev/null +++ b/test/run/exec-args-fixtures/threads.test.ts @@ -0,0 +1,11 @@ +import { describe, expect, it } from 'vitest' + +describe('exec-args', async () => { + it('should have the correct flags', () => { + expect(process.execArgv).toContain('--inspect-brk') + + // added via vitest + expect(process.execArgv).toContain('--conditions') + expect(process.execArgv).toContain('node') + }) +}) diff --git a/test/run/exec-args-fixtures/vmThreads.test.ts b/test/run/exec-args-fixtures/vmThreads.test.ts new file mode 100644 index 000000000000..04ee85f6e2fe --- /dev/null +++ b/test/run/exec-args-fixtures/vmThreads.test.ts @@ -0,0 +1,15 @@ +import { describe, expect, it } from 'vitest' + +describe('exec-args', async () => { + it('should have the correct flags', () => { + expect(process.execArgv).toContain('--inspect-brk') + + // added via vitest + expect(process.execArgv).toContain('--experimental-import-meta-resolve') + expect(process.execArgv).toContain('--experimental-vm-modules') + expect(process.execArgv).toContain('--require') + expect(process.execArgv).toContainEqual(expect.stringContaining('/packages/vitest/suppress-warnings.cjs')) + expect(process.execArgv).toContain('--conditions') + expect(process.execArgv).toContain('node') + }) +}) diff --git a/test/run/no-exec-args-fixtures/no-exec-argv.test.ts b/test/run/no-exec-args-fixtures/no-exec-argv.test.ts new file mode 100644 index 000000000000..04162d6205f0 --- /dev/null +++ b/test/run/no-exec-args-fixtures/no-exec-argv.test.ts @@ -0,0 +1,12 @@ +import { describe, expect, it } from 'vitest' + +describe('exec-args', async () => { + it('should have the correct flags', () => { + // flags should not be passed + expect(process.execArgv).not.toContain('--title') + + // added via vitest + expect(process.execArgv).toContain('--conditions') + expect(process.execArgv).toContain('node') + }) +}) diff --git a/test/run/no-exec-args-fixtures/vitest.config.ts b/test/run/no-exec-args-fixtures/vitest.config.ts new file mode 100644 index 000000000000..abed6b2116e1 --- /dev/null +++ b/test/run/no-exec-args-fixtures/vitest.config.ts @@ -0,0 +1,3 @@ +import { defineConfig } from 'vitest/config' + +export default defineConfig({}) diff --git a/test/run/package.json b/test/run/package.json index 5bc48ffdaa9d..1ffa3012ba94 100644 --- a/test/run/package.json +++ b/test/run/package.json @@ -5,6 +5,7 @@ "test": "vitest" }, "devDependencies": { + "execa": "^7.1.1", "vite": "latest", "vitest": "workspace:*" } diff --git a/test/run/test/exec-args.test.ts b/test/run/test/exec-args.test.ts new file mode 100644 index 000000000000..6d37cda4732a --- /dev/null +++ b/test/run/test/exec-args.test.ts @@ -0,0 +1,69 @@ +import { afterAll, beforeAll, expect, test } from 'vitest' +import { execa } from 'execa' +import { runVitest } from '../../test-utils' + +// VITEST_SEGFAULT_RETRY messes with the node flags, as can be seen in packages/vitest/src/node/cli-wrapper.ts +// so here we remove it to make sure the tests are not affected by it +const ORIGIN_VITEST_SEGFAULT_RETRY = process.env.VITEST_SEGFAULT_RETRY +beforeAll(() => { + delete process.env.VITEST_SEGFAULT_RETRY +}) +afterAll(() => { + process.env.VITEST_SEGFAULT_RETRY = ORIGIN_VITEST_SEGFAULT_RETRY +}) + +test.each([ + { pool: 'forks', execArgv: ['--hash-seed=1', '--random-seed=1', '--no-opt'] }, + { pool: 'threads', execArgv: ['--inspect-brk'] }, + { pool: 'vmThreads', execArgv: ['--inspect-brk'] }, +] as const)('should pass execArgv to { pool: $pool } ', async ({ pool, execArgv }) => { + const fileToTest = `exec-args-fixtures/${pool}.test.ts` + + const vitest = await runVitest({ + include: [fileToTest], + pool, + poolOptions: { + [pool]: { + execArgv, + }, + }, + }) + + expect(vitest.stdout).toContain(`✓ ${fileToTest}`) +}) + +test('should not pass execArgv to workers when not specified in the config', async () => { + const { stdout, stderr } = await execa('node', [ + '--title', 'this-works-only-on-main-thread', + '../node_modules/vitest/vitest.mjs', '--run', + ], { + cwd: `${process.cwd()}/no-exec-args-fixtures`, + reject: false, + env: { + VITE_NODE_DEPS_MODULE_DIRECTORIES: '/node_modules/,/packages/', + NO_COLOR: '1', + }, + }) + + expect(stderr).not.toContain('Error: Initiated Worker with invalid execArgv flags: --title') + expect(stderr).not.toContain('ERR_WORKER_INVALID_EXEC_ARGV') + expect(stdout).toContain('✓ no-exec-argv.test.ts') +}) + +test('should let allowed args pass to workers', async () => { + const { stdout, stderr } = await execa('node', [ + '--cpu-prof', '--heap-prof', '--diagnostic-dir=/tmp/vitest-diagnostics', + '--cpu-prof-name=cpu.prof', '--heap-prof-name=heap.prof', + '../node_modules/vitest/vitest.mjs', '--run', + ], { + cwd: `${process.cwd()}/allowed-exec-args-fixtures`, + reject: false, + env: { + VITE_NODE_DEPS_MODULE_DIRECTORIES: '/node_modules/,/packages/', + NO_COLOR: '1', + }, + }) + + expect(stderr).toBe('') + expect(stdout).toContain('✓ allowed-exec-argv.test.ts') +})