fix: run hook cleanups when a later hook fails (#11605)

Author: sheremet-va

packages/vitest/src/runtime/runner/run.ts

@@ -178,15 +178,15 @@ async function callSuiteHook<T extends keyof SuiteHooks>(
   name: T,
   runner: VitestRunner,
   args: SuiteHooks[T][0] extends (...args: infer A) => Awaitable<any> ? A : never,
-): Promise<unknown[]> {
+  cleanups: unknown[] = [],
+): Promise<void> {
   const sequence = runner.config.sequence.hooks
 
-  const callbacks: unknown[] = []
   // stop at file level
   const parentSuite: Suite | null = 'filepath' in suite ? null : suite.suite || suite.file
 
   if (name === 'beforeEach' && parentSuite) {
-    callbacks.push(...(await callSuiteHook(parentSuite, currentTask, name, runner, args)))
+    await callSuiteHook(parentSuite, currentTask, name, runner, args, cleanups)
   }
 
   const hooks = getSuiteHooks(suite, name, sequence)
@@ -206,10 +206,25 @@ async function callSuiteHook<T extends keyof SuiteHooks>(
   }
 
   if (sequence === 'parallel') {
-    callbacks.push(...(await Promise.all(hooks.map((hook) => runHook(hook)))))
+    const results = await Promise.allSettled(hooks.map((hook) => runHook(hook)))
+    // hooks can reject with the same `signal.reason`
+    const errors = new Set<unknown>()
+    for (const result of results) {
+      if (result.status === 'fulfilled') {
+        cleanups.push(result.value)
+      } else {
+        errors.add(result.reason)
+      }
+    }
+    if (errors.size === 1) {
+      throw [...errors][0]
+    }
+    if (errors.size > 1) {
+      throw new AggregateError([...errors], `Multiple "${name}" hooks failed.`)
+    }
   } else {
     for (const hook of hooks) {
-      callbacks.push(await runHook(hook))
+      cleanups.push(await runHook(hook))
     }
   }
 
@@ -218,10 +233,8 @@ async function callSuiteHook<T extends keyof SuiteHooks>(
   }
 
   if (name === 'afterEach' && parentSuite) {
-    callbacks.push(...(await callSuiteHook(parentSuite, currentTask, name, runner, args)))
+    await callSuiteHook(parentSuite, currentTask, name, runner, args)
   }
-
-  return callbacks
 }
 
 function getAroundEachHooks(suite: Suite): AroundEachListener[] {
@@ -603,7 +616,7 @@ async function runTest(test: Test, runner: VitestRunner): Promise<void> {
     const retry = getRetryCount(test.retry)
     for (let retryCount = 0; retryCount <= retry; retryCount++) {
       const attemptErrorsStart = test.result.errors?.length ?? 0
-      let beforeEachCleanups: unknown[] = []
+      const beforeEachCleanups: unknown[] = []
       // fixtureCheckpoint is passed by callAroundEachHooks - it represents the count
       // of fixture cleanup functions AFTER all aroundEach fixtures have been resolved
       // but BEFORE the test runs. This allows us to clean up only fixtures created
@@ -617,8 +630,15 @@ async function runTest(test: Test, runner: VitestRunner): Promise<void> {
 
           test.result!.repeatCount = repeatCount
 
-          beforeEachCleanups = await $('test.beforeEach', () =>
-            callSuiteHook(suite, test, 'beforeEach', runner, [test.context, suite]),
+          await $('test.beforeEach', () =>
+            callSuiteHook(
+              suite,
+              test,
+              'beforeEach',
+              runner,
+              [test.context, suite],
+              beforeEachCleanups,
+            ),
           )
 
           if (runner.runTask) {
@@ -840,7 +860,7 @@ async function runSuite(suite: Suite, runner: VitestRunner): Promise<void> {
 
   updateTask('suite-prepare', suite, runner)
 
-  let beforeAllCleanups: unknown[] = []
+  const beforeAllCleanups: unknown[] = []
 
   if (suite.mode === 'skip') {
     suite.result.state = 'skip'
@@ -859,8 +879,8 @@ async function runSuite(suite: Suite, runner: VitestRunner): Promise<void> {
         try {
           // beforeAll
           try {
-            beforeAllCleanups = await $('suite.beforeAll', () =>
-              callSuiteHook(suite, suite, 'beforeAll', runner, [suite]),
+            await $('suite.beforeAll', () =>
+              callSuiteHook(suite, suite, 'beforeAll', runner, [suite], beforeAllCleanups),
             )
           } catch (e) {
             failTask(suite.result!, e, runner.config._diffOptions)

test/e2e/test/concurrent.test.ts

@@ -440,6 +440,8 @@ test('beforeEach deadlocks with insufficient maxConcurrency', async () => {
         "wrapper": {
           "t": [
             "Hook timed out in 500ms.
+    If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout".",
+            "Hook timed out in 500ms.
     If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout".",
           ],
         },
@@ -522,6 +524,8 @@ test('afterEach deadlocks with insufficient maxConcurrency', async () => {
         "wrapper": {
           "t": [
             "Hook timed out in 500ms.
+    If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout".",
+            "Hook timed out in 500ms.
     If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout".",
           ],
         },