fix: don't await an empty timeout after every test (#7281)

vitest-dev · Jan 17, 2025 · ef1aa89 · ef1aa89
1 parent 450de3b
commit ef1aa89
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 16 deletions.
diff --git a/packages/runner/src/context.ts b/packages/runner/src/context.ts
@@ -9,6 +9,8 @@ import type {
 import { getSafeTimers } from '@vitest/utils'
 import { PendingError } from './errors'
 
+const now = Date.now
+
 export const collectorContext: RuntimeContext = {
   tasks: [],
   currentSuite: null,
@@ -41,19 +43,55 @@ export function withTimeout<T extends (...args: any[]) => any>(
 
   // this function name is used to filter error in test/cli/test/fails.test.ts
   return (function runWithTimeout(...args: T extends (...args: infer A) => any ? A : never) {
-    return Promise.race([
-      new Promise((resolve, reject) => {
-        const timer = setTimeout(() => {
-          clearTimeout(timer)
-          reject(new Error(makeTimeoutMsg(isHook, timeout)))
-        }, timeout)
-        // `unref` might not exist in browser
-        timer.unref?.()
-      }),
-      Promise.resolve(fn(...args)).then((result) => {
-        return new Promise(resolve => setTimeout(resolve, 0, result))
-      }),
-    ]) as Awaitable<void>
+    const startTime = now()
+    return new Promise((resolve_, reject_) => {
+      const timer = setTimeout(() => {
+        clearTimeout(timer)
+        reject(new Error(makeTimeoutMsg(isHook, timeout)))
+      }, timeout)
+      // `unref` might not exist in browser
+      timer.unref?.()
+
+      function resolve(result: unknown) {
+        clearTimeout(timer)
+        resolve_(result)
+      }
+
+      function reject(error: unknown) {
+        clearTimeout(timer)
+        reject_(error)
+      }
+
+      // sync test/hook will be caught by try/catch
+      try {
+        const result = fn(...args) as PromiseLike<unknown>
+        // the result is a thenable, we don't wrap this in Promise.resolve
+        // to avoid creating new promises
+        if (typeof result === 'object' && result != null && typeof result.then === 'function') {
+          result.then(
+            (result) => {
+              // if sync test/hook took too long, setTimeout won't be triggered,
+              // but we still need to fail the test, see
+              // https://github.com/vitest-dev/vitest/issues/2920
+              if (now() - startTime >= timeout) {
+                reject(new Error(makeTimeoutMsg(isHook, timeout)))
+              }
+              else {
+                resolve(result)
+              }
+            },
+            reject,
+          )
+        }
+        else {
+          resolve(result)
+        }
+      }
+      // user sync test/hook throws an error
+      catch (error) {
+        reject(error)
+      }
+    })
   }) as T
 }
 

diff --git a/test/cli/test/__snapshots__/fails.test.ts.snap b/test/cli/test/__snapshots__/fails.test.ts.snap
@@ -7,9 +7,15 @@ exports[`should fail async-assertion.test.ts 1`] = `
 AssertionError: expected 'xx' to be 'yy' // Object.is equality"
 `;
 
-exports[`should fail concurrent-suite-deadlock.test.ts 1`] = `"Error: Test timed out in 500ms."`;
+exports[`should fail concurrent-suite-deadlock.test.ts 1`] = `
+"Error: Test timed out in 500ms.
+Error: Test timed out in 500ms."
+`;
 
-exports[`should fail concurrent-test-deadlock.test.ts 1`] = `"Error: Test timed out in 500ms."`;
+exports[`should fail concurrent-test-deadlock.test.ts 1`] = `
+"Error: Test timed out in 500ms.
+Error: Test timed out in 500ms."
+`;
 
 exports[`should fail each-timeout.test.ts 1`] = `"Error: Test timed out in 10ms."`;
 

diff --git a/test/cli/test/fails.test.ts b/test/cli/test/fails.test.ts
@@ -18,7 +18,7 @@ it.each(files)('should fail %s', async (file) => {
   const msg = String(stderr)
     .split(/\n/g)
     .reverse()
-    .filter(i => i.includes('Error: ') && !i.includes('Command failed') && !i.includes('stackStr') && !i.includes('at runTest') && !i.includes('at runWithTimeout'))
+    .filter(i => i.includes('Error: ') && !i.includes('Command failed') && !i.includes('stackStr') && !i.includes('at runTest') && !i.includes('at runWithTimeout') && !i.includes('file:'))
     .map(i => i.trim().replace(root, '<rootDir>'),
     )
     .join('\n')