Skip to content

Commit

Permalink
fix(forks): wrap defines to support undefined values (#5284)
Browse files Browse the repository at this point in the history
  • Loading branch information
AriPerkkio authored Feb 27, 2024
1 parent 9abef3d commit 5b58b39
Show file tree
Hide file tree
Showing 10 changed files with 83 additions and 41 deletions.
16 changes: 2 additions & 14 deletions packages/vitest/src/node/pools/forks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type { ContextRPC, ContextTestEnvironment, ResolvedConfig, RunnerRPC, Run
import type { PoolProcessOptions, ProcessPool, RunWithFiles } from '../pool'
import type { WorkspaceProject } from '../workspace'
import { envsOrder, groupFilesByEnv } from '../../utils/test-helpers'
import { wrapSerializableConfig } from '../../utils/config-helpers'
import { groupBy, resolve } from '../../utils'
import { createMethodsRPC } from './rpc'

Expand Down Expand Up @@ -44,12 +45,6 @@ function createChildProcessChannel(project: WorkspaceProject) {
return { channel, cleanup }
}

function stringifyRegex(input: RegExp | string): string {
if (typeof input === 'string')
return input
return `$$vitest:${input.toString()}`
}

export function createForksPool(ctx: Vitest, { execArgv, env }: PoolProcessOptions): ProcessPool {
const numCpus
= typeof nodeos.availableParallelism === 'function'
Expand Down Expand Up @@ -144,14 +139,7 @@ export function createForksPool(ctx: Vitest, { execArgv, env }: PoolProcessOptio
return configs.get(project)!

const _config = project.getSerializableConfig()

const config = {
..._config,
// v8 serialize does not support regex
testNamePattern: _config.testNamePattern
? stringifyRegex(_config.testNamePattern)
: undefined,
} as ResolvedConfig
const config = wrapSerializableConfig(_config)

configs.set(project, config)
return config
Expand Down
15 changes: 2 additions & 13 deletions packages/vitest/src/node/pools/vmForks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { groupFilesByEnv } from '../../utils/test-helpers'
import { AggregateError } from '../../utils/base'
import type { WorkspaceProject } from '../workspace'
import { getWorkerMemoryLimit, stringToBytes } from '../../utils/memory-limit'
import { wrapSerializableConfig } from '../../utils/config-helpers'
import { createMethodsRPC } from './rpc'

const suppressWarningsPath = resolve(rootDir, './suppress-warnings.cjs')
Expand Down Expand Up @@ -49,12 +50,6 @@ function createChildProcessChannel(project: WorkspaceProject) {
return { channel, cleanup }
}

function stringifyRegex(input: RegExp | string): string {
if (typeof input === 'string')
return input
return `$$vitest:${input.toString()}`
}

export function createVmForksPool(ctx: Vitest, { execArgv, env }: PoolProcessOptions): ProcessPool {
const numCpus
= typeof nodeos.availableParallelism === 'function'
Expand Down Expand Up @@ -149,14 +144,8 @@ export function createVmForksPool(ctx: Vitest, { execArgv, env }: PoolProcessOpt
return configs.get(project)!

const _config = project.getSerializableConfig()
const config = wrapSerializableConfig(_config)

const config = {
..._config,
// v8 serialize does not support regex
testNamePattern: _config.testNamePattern
? stringifyRegex(_config.testNamePattern)
: undefined,
} as ResolvedConfig
configs.set(project, config)
return config
}
Expand Down
4 changes: 2 additions & 2 deletions packages/vitest/src/runtime/workers/forks.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import v8 from 'node:v8'
import type { WorkerGlobalState } from '../../types/worker'
import { createForksRpcOptions, unwrapForksConfig } from './utils'
import { createForksRpcOptions, unwrapSerializableConfig } from './utils'
import { runBaseTests } from './base'
import type { VitestWorker } from './types'

Expand All @@ -13,7 +13,7 @@ class ForksBaseWorker implements VitestWorker {
// TODO: don't rely on reassigning process.exit
// https://github.com/vitest-dev/vitest/pull/4441#discussion_r1443771486
const exit = process.exit
state.ctx.config = unwrapForksConfig(state.ctx.config)
state.ctx.config = unwrapSerializableConfig(state.ctx.config)

try {
await runBaseTests(state)
Expand Down
32 changes: 23 additions & 9 deletions packages/vitest/src/runtime/workers/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import type { WorkerContext } from '../../types/worker'
import type { ResolvedConfig } from '../../types/config'
import type { WorkerRpcOptions } from './types'

const REGEXP_WRAP_PREFIX = '$$vitest:'

export function createThreadsRpcOptions({ port }: WorkerContext): WorkerRpcOptions {
return {
post: (v) => { port.postMessage(v) },
Expand All @@ -28,15 +30,27 @@ export function createForksRpcOptions(nodeV8: typeof import('v8')): WorkerRpcOpt
}
}

function parsePossibleRegexp(str: string | RegExp) {
const prefix = '$$vitest:'
if (typeof str === 'string' && str.startsWith(prefix))
return parseRegexp(str.slice(prefix.length))
return str
}
/**
* Reverts the wrapping done by `utils/config-helpers.ts`'s `wrapSerializableConfig`
*/
export function unwrapSerializableConfig(config: ResolvedConfig) {
if (config.testNamePattern && typeof config.testNamePattern === 'string') {
const testNamePattern = config.testNamePattern as string

if (testNamePattern.startsWith(REGEXP_WRAP_PREFIX))
config.testNamePattern = parseRegexp(testNamePattern.slice(REGEXP_WRAP_PREFIX.length))
}

if (config.defines && Array.isArray(config.defines.keys) && config.defines.original) {
const { keys, original } = config.defines
const defines: ResolvedConfig['defines'] = {}

// Apply all keys from the original. Entries which had undefined value are missing from original now
for (const key of keys)
defines[key] = original[key]

config.defines = defines
}

export function unwrapForksConfig(config: ResolvedConfig) {
if (config.testNamePattern)
config.testNamePattern = parsePossibleRegexp(config.testNamePattern) as RegExp
return config
}
4 changes: 2 additions & 2 deletions packages/vitest/src/runtime/workers/vmForks.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import v8 from 'node:v8'
import type { WorkerGlobalState } from '../../types/worker'
import { createForksRpcOptions, unwrapForksConfig } from './utils'
import { createForksRpcOptions, unwrapSerializableConfig } from './utils'
import type { VitestWorker } from './types'
import { runVmTests } from './vm'

Expand All @@ -11,7 +11,7 @@ class ForksVmWorker implements VitestWorker {

async runTests(state: WorkerGlobalState) {
const exit = process.exit
state.ctx.config = unwrapForksConfig(state.ctx.config)
state.ctx.config = unwrapSerializableConfig(state.ctx.config)

try {
await runVmTests(state)
Expand Down
25 changes: 25 additions & 0 deletions packages/vitest/src/utils/config-helpers.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import type { ResolvedConfig } from '../types/config'
import type { BenchmarkBuiltinReporters, BuiltinReporters } from '../node/reporters'

const REGEXP_WRAP_PREFIX = '$$vitest:'

interface PotentialConfig {
outputFile?: string | Partial<Record<string, string>>
}
Expand All @@ -13,3 +16,25 @@ export function getOutputFile(config: PotentialConfig | undefined, reporter: Bui

return config.outputFile[reporter]
}

/**
* Prepares `ResolvedConfig` for serialization, e.g. `node:v8.serialize`
*/
export function wrapSerializableConfig(config: ResolvedConfig) {
let testNamePattern = config.testNamePattern
let defines = config.defines

// v8 serialize does not support regex
if (testNamePattern && typeof testNamePattern !== 'string')
testNamePattern = `${REGEXP_WRAP_PREFIX}${testNamePattern.toString()}` as unknown as RegExp

// v8 serialize drops properties with undefined value
if (defines)
defines = { keys: Object.keys(defines), original: defines }

return {
...config,
testNamePattern,
defines,
} as ResolvedConfig
}
2 changes: 1 addition & 1 deletion packages/vitest/src/workers.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export { createForksRpcOptions, createThreadsRpcOptions, unwrapForksConfig } from './runtime/workers/utils'
export { createForksRpcOptions, createThreadsRpcOptions, unwrapSerializableConfig } from './runtime/workers/utils'
export { provideWorkerState } from './utils/global'
export { run as runVitestWorker } from './runtime/worker'
export { runVmTests } from './runtime/workers/vm'
Expand Down
11 changes: 11 additions & 0 deletions test/core/test/define-ssr.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ import { afterAll, expect, test } from 'vitest'
declare let __DEFINE__: string
declare let __JSON__: any
declare let __MODE__: string
declare let __UNDEFINED__: undefined
declare let __NULL__: null
declare let __ZERO__: 0
declare let __FALSE__: false
declare let SOME: {
VARIABLE: string
SOME: {
Expand Down Expand Up @@ -64,3 +68,10 @@ test('dotted defines are processed by Vite, but cannot be reassigned', () => {
SOME.VARIABLE = 'new variable'
expect(SOME.VARIABLE).not.toBe('new variable')
})

test('falsy defines are passed', () => {
expect(__UNDEFINED__).toBe(undefined)
expect(__NULL__).toBe(null)
expect(__ZERO__).toBe(0)
expect(__FALSE__).toBe(false)
})
11 changes: 11 additions & 0 deletions test/core/test/define-web.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ import { afterAll, expect, test } from 'vitest'
declare let __DEFINE__: string
declare let __JSON__: any
declare let __MODE__: string
declare let __UNDEFINED__: undefined
declare let __NULL__: null
declare let __ZERO__: 0
declare let __FALSE__: false
declare let SOME: {
VARIABLE: string
SOME: {
Expand Down Expand Up @@ -61,3 +65,10 @@ test('dotted defines can be reassigned', () => {
SOME.VARIABLE = 'new variable'
expect(SOME.VARIABLE).toBe('new variable')
})

test('falsy defines are passed', () => {
expect(__UNDEFINED__).toBe(undefined)
expect(__NULL__).toBe(null)
expect(__ZERO__).toBe(0)
expect(__FALSE__).toBe(false)
})
4 changes: 4 additions & 0 deletions test/core/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ export default defineConfig({
'__MODE__': 'process.env.MODE',
'SOME.VARIABLE': '"variable"',
'SOME.SOME.VARIABLE': '"nested variable"',
'__UNDEFINED__': undefined,
'__NULL__': null,
'__ZERO__': 0,
'__FALSE__': false,
},
resolve: {
alias: [
Expand Down

0 comments on commit 5b58b39

Please sign in to comment.