fix: fix vm tests flakiness (#7741)

sheremet-va · web-flow · commit 2702cf4245ff · 2025-03-26T20:34:54.000+01:00
diff --git a/packages/runner/src/types/tasks.ts b/packages/runner/src/types/tasks.ts
@@ -647,7 +647,7 @@ export interface TestContext {
   /**
    * Metadata of the current test
    */
-  task: Readonly<Task>
+  task: Readonly<Test<TestContext>>
 
   /**
    * Extract hooks on test failed
diff --git a/packages/vitest/src/runtime/external-executor.ts b/packages/vitest/src/runtime/external-executor.ts
@@ -1,19 +1,16 @@
+import type vm from 'node:vm'
 import type { RuntimeRPC } from '../types/rpc'
 import type { FileMap } from './vm/file-map'
-import type { VMModule, VMSyntheticModule } from './vm/types'
+import type { VMModule } from './vm/types'
 import fs from 'node:fs'
 import { dirname } from 'node:path'
 import { fileURLToPath, pathToFileURL } from 'node:url'
-import vm from 'node:vm'
 import { extname, join, normalize } from 'pathe'
 import { getCachedData, isNodeBuiltin, setCacheData } from 'vite-node/utils'
 import { CommonjsExecutor } from './vm/commonjs-executor'
 import { EsmExecutor } from './vm/esm-executor'
-import { interopCommonJsModule } from './vm/utils'
 import { ViteExecutor } from './vm/vite-executor'
 
-const SyntheticModule: typeof VMSyntheticModule = (vm as any).SyntheticModule
-
 const { existsSync, statSync } = fs
 
 // always defined when we use vm pool
@@ -64,6 +61,7 @@ export class ExternalModulesExecutor {
       context: this.context,
       importModuleDynamically: this.importModuleDynamically,
       fileMap: options.fileMap,
+      interopDefault: options.interopDefault,
     })
     this.vite = new ViteExecutor({
       esmExecutor: this.esm,
@@ -156,52 +154,6 @@ export class ExternalModulesExecutor {
     return {}
   }
 
-  private wrapCoreSyntheticModule(
-    identifier: string,
-    exports: Record<string, unknown>,
-  ) {
-    const moduleKeys = Object.keys(exports)
-    const m = new SyntheticModule(
-      [...moduleKeys, 'default'],
-      () => {
-        for (const key of moduleKeys) {
-          m.setExport(key, exports[key])
-        }
-        m.setExport('default', exports)
-      },
-      {
-        context: this.context,
-        identifier,
-      },
-    )
-    return m
-  }
-
-  private wrapCommonJsSynteticModule(
-    identifier: string,
-    exports: Record<string, unknown>,
-  ) {
-    // TODO: technically module should be parsed to find static exports, implement for strict mode in #2854
-    const { keys, moduleExports, defaultExport } = interopCommonJsModule(
-      this.options.interopDefault,
-      exports,
-    )
-    const m = new SyntheticModule(
-      [...keys, 'default'],
-      () => {
-        for (const key of keys) {
-          m.setExport(key, moduleExports[key])
-        }
-        m.setExport('default', defaultExport)
-      },
-      {
-        context: this.context,
-        identifier,
-      },
-    )
-    return m
-  }
-
   private getModuleInformation(identifier: string): ModuleInformation {
     if (identifier.startsWith('data:')) {
       return { type: 'data', url: identifier, path: identifier }
@@ -248,7 +200,7 @@ export class ExternalModulesExecutor {
     return { type, path: pathUrl, url: fileUrl }
   }
 
-  private async createModule(identifier: string): Promise<VMModule> {
+  private createModule(identifier: string): VMModule | Promise<VMModule> {
     const { type, url, path } = this.getModuleInformation(identifier)
 
     // create ERR_MODULE_NOT_FOUND on our own since latest NodeJS's import.meta.resolve doesn't throw on non-existing namespace or path
@@ -264,25 +216,21 @@ export class ExternalModulesExecutor {
 
     switch (type) {
       case 'data':
-        return await this.esm.createDataModule(identifier)
-      case 'builtin': {
-        const exports = this.require(identifier)
-        return this.wrapCoreSyntheticModule(identifier, exports)
-      }
+        return this.esm.createDataModule(identifier)
+      case 'builtin':
+        return this.cjs.getCoreSyntheticModule(identifier)
       case 'vite':
-        return await this.vite.createViteModule(url)
+        return this.vite.createViteModule(url)
       case 'wasm':
-        return await this.esm.createWebAssemblyModule(url, () =>
+        return this.esm.createWebAssemblyModule(url, () =>
           this.fs.readBuffer(path))
       case 'module':
-        return await this.esm.createEsModule(url, () =>
+        return this.esm.createEsModule(url, () =>
           this.fs.readFileAsync(path))
-      case 'commonjs': {
-        const exports = this.require(path)
-        return this.wrapCommonJsSynteticModule(identifier, exports)
-      }
+      case 'commonjs':
+        return this.cjs.getCjsSyntheticModule(path, identifier)
       case 'network':
-        return await this.esm.createNetworkModule(url)
+        return this.esm.createNetworkModule(url)
       default: {
         const _deadend: never = type
         return _deadend
diff --git a/packages/vitest/src/runtime/vm/commonjs-executor.ts b/packages/vitest/src/runtime/vm/commonjs-executor.ts
@@ -1,12 +1,14 @@
 import type { FileMap } from './file-map'
-import type { ImportModuleDynamically, VMModule } from './types'
+import type { ImportModuleDynamically, VMSyntheticModule } from './types'
 import { Module as _Module, createRequire } from 'node:module'
 import vm from 'node:vm'
 import { basename, dirname, extname } from 'pathe'
 import { isNodeBuiltin } from 'vite-node/utils'
+import { interopCommonJsModule, SyntheticModule } from './utils'
 
 interface CommonjsExecutorOptions {
   fileMap: FileMap
+  interopDefault?: boolean
   context: vm.Context
   importModuleDynamically: ImportModuleDynamically
 }
@@ -24,7 +26,7 @@ export class CommonjsExecutor {
   private requireCache = new Map<string, NodeJS.Module>()
   private publicRequireCache = this.createProxyCache()
 
-  private moduleCache = new Map<string, VMModule | Promise<VMModule>>()
+  private moduleCache = new Map<string, VMSyntheticModule>()
   private builtinCache: Record<string, NodeJS.Module> = Object.create(null)
   private extensions: Record<
     string,
@@ -33,10 +35,12 @@ export class CommonjsExecutor {
 
   private fs: FileMap
   private Module: typeof _Module
+  private interopDefault: boolean | undefined
 
   constructor(options: CommonjsExecutorOptions) {
     this.context = options.context
     this.fs = options.fileMap
+    this.interopDefault = options.interopDefault
 
     const primitives = vm.runInContext(
       '({ Object, Array, Error })',
@@ -132,7 +136,7 @@ export class CommonjsExecutor {
       static SourceMap = _Module.SourceMap
       static syncBuiltinESMExports = _Module.syncBuiltinESMExports
 
-      static _cache = executor.moduleCache
+      static _cache = executor.publicRequireCache
       static _extensions = executor.extensions
 
       static createRequire = (filename: string | URL) => {
@@ -256,6 +260,86 @@ export class CommonjsExecutor {
     return '.js'
   }
 
+  public getCoreSyntheticModule(identifier: string): VMSyntheticModule {
+    if (this.moduleCache.has(identifier)) {
+      return this.moduleCache.get(identifier)!
+    }
+    const exports = this.require(identifier)
+    const keys = Object.keys(exports)
+    const module = new SyntheticModule([...keys, 'default'], () => {
+      for (const key of keys) {
+        module.setExport(key, exports[key])
+      }
+      module.setExport('default', exports)
+    }, { context: this.context, identifier })
+    this.moduleCache.set(identifier, module)
+    return module
+  }
+
+  public getCjsSyntheticModule(path: string, identifier: string): VMSyntheticModule {
+    if (this.moduleCache.has(identifier)) {
+      return this.moduleCache.get(identifier)!
+    }
+    const exports = this.require(path)
+    // TODO: technically module should be parsed to find static exports, implement for strict mode in #2854
+    const { keys, moduleExports, defaultExport } = interopCommonJsModule(
+      this.interopDefault,
+      exports,
+    )
+    const module = new SyntheticModule([...keys, 'default'], function () {
+      for (const key of keys) {
+        this.setExport(key, moduleExports[key])
+      }
+      this.setExport('default', defaultExport)
+    }, { context: this.context, identifier })
+    this.moduleCache.set(identifier, module)
+    return module
+  }
+
+  // TODO: use this in strict mode, when available in #2854
+  // private _getNamedCjsExports(path: string): Set<string> {
+  //   const cachedNamedExports = this.cjsNamedExportsMap.get(path)
+
+  //   if (cachedNamedExports) {
+  //     return cachedNamedExports
+  //   }
+
+  //   if (extname(path) === '.node') {
+  //     const moduleExports = this.require(path)
+  //     const namedExports = new Set(Object.keys(moduleExports))
+  //     this.cjsNamedExportsMap.set(path, namedExports)
+  //     return namedExports
+  //   }
+
+  //   const code = this.fs.readFile(path)
+  //   const { exports, reexports } = parseCjs(code, path)
+  //   const namedExports = new Set(exports)
+  //   this.cjsNamedExportsMap.set(path, namedExports)
+
+  //   for (const reexport of reexports) {
+  //     if (isNodeBuiltin(reexport)) {
+  //       const exports = this.require(reexport)
+  //       if (exports !== null && typeof exports === 'object') {
+  //         for (const e of Object.keys(exports)) {
+  //           namedExports.add(e)
+  //         }
+  //       }
+  //     }
+  //     else {
+  //       const require = this.createRequire(path)
+  //       const resolved = require.resolve(reexport)
+
+  //       const exports = this._getNamedCjsExports(resolved)
+
+  //       for (const e of exports) {
+  //         namedExports.add(e)
+  //       }
+  //     }
+  //   }
+
+  //   return namedExports
+  // }
+
   public require(identifier: string): any {
     const ext = extname(identifier)
     if (ext === '.node' || isNodeBuiltin(identifier)) {
diff --git a/packages/vitest/src/runtime/vm/esm-executor.ts b/packages/vitest/src/runtime/vm/esm-executor.ts
@@ -66,9 +66,9 @@ export class EsmExecutor {
     const code = await getCode()
     // TODO: should not be allowed in strict mode, implement in #2854
     if (fileURL.endsWith('.json')) {
-      const m = new SyntheticModule(['default'], () => {
+      const m = new SyntheticModule(['default'], function () {
         const result = JSON.parse(code)
-        m.setExport('default', result)
+        this.setExport('default', result)
       })
       this.moduleCache.set(fileURL, m)
       return m
@@ -148,15 +148,17 @@ export class EsmExecutor {
       }
     }
 
+    const evaluateModule = (module: VMModule) => this.evaluateModule(module)
+
     const syntheticModule = new SyntheticModule(
       exports.map(({ name }) => name),
-      async () => {
+      async function () {
         const importsObject: WebAssembly.Imports = {}
         for (const { module, name } of imports) {
           if (!importsObject[module]) {
             importsObject[module] = {}
           }
-          await this.evaluateModule(moduleLookup[module])
+          await evaluateModule(moduleLookup[module])
           importsObject[module][name] = (moduleLookup[module].namespace as any)[
             name
           ]
@@ -166,7 +168,7 @@ export class EsmExecutor {
           importsObject,
         )
         for (const { name } of exports) {
-          syntheticModule.setExport(name, wasmInstance.exports[name])
+          this.setExport(name, wasmInstance.exports[name])
         }
       },
       { context: this.context, identifier },
@@ -229,9 +231,9 @@ export class EsmExecutor {
     if (mime === 'application/json') {
       const module = new SyntheticModule(
         ['default'],
-        () => {
+        function () {
           const obj = JSON.parse(code)
-          module.setExport('default', obj)
+          this.setExport('default', obj)
         },
         { context: this.context, identifier },
       )
diff --git a/packages/vitest/src/runtime/vm/vite-executor.ts b/packages/vitest/src/runtime/vm/vite-executor.ts
@@ -83,9 +83,9 @@ export class ViteExecutor {
     const moduleKeys = Object.keys(stub)
     const module = new SyntheticModule(
       moduleKeys,
-      () => {
+      function () {
         moduleKeys.forEach((key) => {
-          module.setExport(key, stub[key])
+          this.setExport(key, stub[key])
         })
       },
       { context: this.options.context, identifier },
diff --git a/test/cli/test/setup-files.test.ts b/test/cli/test/setup-files.test.ts
@@ -2,13 +2,7 @@ import { promises as fs } from 'node:fs'
 import { describe, expect, it, test } from 'vitest'
 import { editFile, runVitest } from '../../test-utils'
 
-const [major] = process.version.slice(1).split('.').map(num => Number(num))
-
 test.each(['threads', 'vmThreads'])('%s: print stdout and stderr correctly when called in the setup file', async (pool) => {
-  if (major >= 22 && pool === 'vmThreads') {
-    return
-  }
-
   const { stdout, stderr } = await runVitest({
     root: 'fixtures/setup-files',
     include: ['empty.test.ts'],
diff --git a/test/cli/test/stacktraces.test.ts b/test/cli/test/stacktraces.test.ts
@@ -3,8 +3,6 @@ import { glob } from 'tinyglobby'
 import { describe, expect, it } from 'vitest'
 import { runVitest } from '../../test-utils'
 
-const [major] = process.version.slice(1).split('.').map(num => Number(num))
-
 // To prevent the warning coming up in snapshots
 process.setMaxListeners(20)
 
@@ -92,7 +90,7 @@ describe('stacktrace in dependency package', () => {
   })
 })
 
-it.runIf(major < 22)('stacktrace in vmThreads', async () => {
+it('stacktrace in vmThreads', async () => {
   const root = resolve(__dirname, '../fixtures/stacktraces')
   const testFile = resolve(root, './error-with-stack.test.js')
   const { stderr } = await runVitest({
diff --git a/test/cli/test/vm-threads.test.ts b/test/cli/test/vm-threads.test.ts
@@ -2,9 +2,7 @@ import { expect, test } from 'vitest'
 
 import { createFile, resolvePath, runVitest } from '../../test-utils'
 
-const [major] = process.version.slice(1).split('.').map(num => Number(num))
-
-test.runIf(major < 22)('importing files in restricted fs works correctly', async () => {
+test('importing files in restricted fs works correctly', async () => {
   createFile(
     resolvePath(import.meta.url, '../fixtures/vm-threads/src/external/package-null/package-null.json'),
     'null',
diff --git a/test/core/test/diff.test.ts b/test/core/test/diff.test.ts
@@ -322,7 +322,7 @@ test('truncate large diff', () => {
   const diff = getErrorDiff(Array.from({ length: 500_000 }).fill(0), 1234)
   expect(diff.length).lessThan(200_000)
   expect(diff.trim()).toMatch(/\.\.\.$/)
-})
+}, 60_000)
 
 test('diff default maxDepth', () => {
   function generateCycle(n: number) {
diff --git a/test/core/test/env.test.ts b/test/core/test/env.test.ts
diff --git a/test/core/test/timeout.spec.ts b/test/core/test/timeout.spec.ts
diff --git a/test/core/vitest.workspace.ts b/test/core/vitest.workspace.ts
diff --git a/test/coverage-test/test/isolation.test.ts b/test/coverage-test/test/isolation.test.ts
