Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(forks): wrap defines to support undefined values #5284

Merged
merged 2 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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) {
AriPerkkio marked this conversation as resolved.
Show resolved Hide resolved
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
Loading