Skip to content

Commit a8a8836

Browse files
authored
fix(pool): validate environment options when reusing the worker (#9349)
1 parent 2dd3dd8 commit a8a8836

File tree

7 files changed

+69
-13
lines changed

7 files changed

+69
-13
lines changed

packages/vitest/src/node/pool.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,8 @@ export function createPool(ctx: Vitest): ProcessPool {
151151
invalidates,
152152
providedContext: project.getProvidedContext(),
153153
workerId: workerId++,
154+
environment,
154155
},
155-
environment,
156156
project,
157157
env,
158158
execArgv,

packages/vitest/src/node/pools/pool.ts

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { ContextTestEnvironment } from '../../types/worker'
12
import type { Logger } from '../logger'
23
import type { StateManager } from '../state'
34
import type { PoolOptions, PoolTask, WorkerResponse } from './types'
@@ -213,15 +214,17 @@ export class Pool {
213214
const index = this.sharedRunners.findIndex(runner => isEqualRunner(runner, task))
214215

215216
if (index !== -1) {
216-
return this.sharedRunners.splice(index, 1)[0]
217+
const runner = this.sharedRunners.splice(index, 1)[0]
218+
runner.reconfigure(task)
219+
return runner
217220
}
218221
}
219222

220223
const options: PoolOptions = {
221224
distPath: this.options.distPath,
222225
project: task.project,
223226
method,
224-
environment: task.environment,
227+
environment: task.context.environment,
225228
env: task.env,
226229
execArgv: task.execArgv,
227230
}
@@ -289,11 +292,47 @@ function isEqualRunner(runner: PoolRunner, task: PoolTask) {
289292
if (task.isolate) {
290293
throw new Error('Isolated tasks should not share runners')
291294
}
295+
if (runner.worker.name !== task.worker || runner.project !== task.project) {
296+
return false
297+
}
298+
// by default, check that the environments are the same
299+
// some workers (like vmThreads/vmForks) do not need this check
300+
if (!runner.worker.canReuse) {
301+
return isEnvironmentEqual(task.context.environment, runner.environment)
302+
}
303+
return runner.worker.canReuse(task)
304+
}
305+
306+
function isEnvironmentEqual(env1: ContextTestEnvironment, env2: ContextTestEnvironment): boolean {
307+
if (env1.name !== env2.name) {
308+
return false
309+
}
310+
return deepEqual(env1.options, env2.options)
311+
}
312+
313+
function deepEqual(obj1: any, obj2: any): boolean {
314+
if (obj1 === obj2) {
315+
return true
316+
}
317+
if (obj1 == null || obj2 == null) {
318+
return obj1 === obj2
319+
}
320+
if (typeof obj1 !== 'object' || typeof obj2 !== 'object') {
321+
return false
322+
}
323+
324+
const keys1 = Object.keys(obj1)
325+
const keys2 = Object.keys(obj2)
326+
327+
if (keys1.length !== keys2.length) {
328+
return false
329+
}
330+
331+
for (const key of keys1) {
332+
if (!keys2.includes(key) || !deepEqual(obj1[key], obj2[key])) {
333+
return false
334+
}
335+
}
292336

293-
return (
294-
runner.worker.name === task.worker
295-
&& runner.project === task.project
296-
&& runner.environment.name === task.environment.name
297-
&& (!runner.worker.canReuse || runner.worker.canReuse(task))
298-
)
337+
return true
299338
}

packages/vitest/src/node/pools/poolRunner.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import type { RunnerRPC, RuntimeRPC } from '../../types/rpc'
55
import type { ContextTestEnvironment, WorkerExecuteContext } from '../../types/worker'
66
import type { Traces } from '../../utils/traces'
77
import type { TestProject } from '../project'
8-
import type { PoolOptions, PoolRunnerOTEL, PoolWorker, WorkerRequest, WorkerResponse } from './types'
8+
import type { PoolOptions, PoolRunnerOTEL, PoolTask, PoolWorker, WorkerRequest, WorkerResponse } from './types'
99
import { EventEmitter } from 'node:events'
1010
import { createDefer } from '@vitest/utils/helpers'
1111
import { createBirpc } from 'birpc'
@@ -43,7 +43,7 @@ export class PoolRunner {
4343
public poolId: number | undefined = undefined
4444

4545
public readonly project: TestProject
46-
public readonly environment: ContextTestEnvironment
46+
public environment: ContextTestEnvironment
4747

4848
private _state: RunnerState = RunnerState.IDLE
4949
private _operationLock: DeferPromise<void> | null = null
@@ -113,6 +113,15 @@ export class PoolRunner {
113113
this._offCancel = vitest.onCancel(reason => this._rpc.onCancel(reason))
114114
}
115115

116+
/**
117+
* "reconfigure" can only be called if `environment` is different, since different project always
118+
* requires a new PoolRunner instance.
119+
*/
120+
public reconfigure(task: PoolTask): void {
121+
this.environment = task.context.environment
122+
this._otel?.span.setAttribute('vitest.environment', this.environment.name)
123+
}
124+
116125
postMessage(message: WorkerRequest): void {
117126
// Only send messages when runner is active (not fully stopped)
118127
// Allow sending during STOPPING state for the 'stop' message itself

packages/vitest/src/node/pools/types.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ export interface PoolWorker {
3434

3535
/**
3636
* This is called on workers that already satisfy certain constraints:
37+
* - The task has the same worker name
3738
* - The task has the same project
38-
* - The task has the same environment
3939
*/
4040
canReuse?: (task: PoolTask) => boolean
4141
}
@@ -55,7 +55,6 @@ export interface PoolTask {
5555
*/
5656
execArgv: string[]
5757
context: WorkerExecuteContext
58-
environment: ContextTestEnvironment
5958
memoryLimit: number | null
6059
}
6160

packages/vitest/src/node/pools/workers/vmForksWorker.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,8 @@ export class VmForksPoolWorker extends ForksPoolWorker {
1414
/** Loads {@link file://./../../../runtime/workers/vmForks.ts} */
1515
this.entrypoint = resolve(options.distPath, 'workers/vmForks.js')
1616
}
17+
18+
canReuse(): boolean {
19+
return true
20+
}
1721
}

packages/vitest/src/node/pools/workers/vmThreadsWorker.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,8 @@ export class VmThreadsPoolWorker extends ThreadsPoolWorker {
1414
/** Loads {@link file://./../../../runtime/workers/vmThreads.ts} */
1515
this.entrypoint = resolve(options.distPath, 'workers/vmThreads.js')
1616
}
17+
18+
canReuse(): boolean {
19+
return true
20+
}
1721
}

packages/vitest/src/types/worker.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export interface WorkerExecuteContext {
2424
files: FileSpecification[]
2525
providedContext: Record<string, any>
2626
invalidates?: string[]
27+
environment: ContextTestEnvironment
2728

2829
/** Exposed to test runner as `VITEST_WORKER_ID`. Value is unique per each isolated worker. */
2930
workerId: number

0 commit comments

Comments
 (0)