fix(runner): correctly call test hooks and teardown functions (#7775)

Author: sheremet-va

packages/runner/src/run.ts

@@ -221,15 +221,31 @@ export function updateTask(event: TaskUpdateEvent, task: Task, runner: VitestRun
   sendTasksUpdateThrottled(runner)
 }
 
-async function callCleanupHooks(cleanups: unknown[]) {
-  await Promise.all(
-    cleanups.map(async (fn) => {
+async function callCleanupHooks(runner: VitestRunner, cleanups: unknown[]) {
+  const sequence = runner.config.sequence.hooks
+
+  if (sequence === 'stack') {
+    cleanups = cleanups.slice().reverse()
+  }
+
+  if (sequence === 'parallel') {
+    await Promise.all(
+      cleanups.map(async (fn) => {
+        if (typeof fn !== 'function') {
+          return
+        }
+        await fn()
+      }),
+    )
+  }
+  else {
+    for (const fn of cleanups) {
       if (typeof fn !== 'function') {
-        return
+        continue
       }
       await fn()
-    }),
-  )
+    }
+  }
 }
 
 export async function runTest(test: Test, runner: VitestRunner): Promise<void> {
@@ -314,15 +330,6 @@ export async function runTest(test: Test, runner: VitestRunner): Promise<void> {
         failTask(test.result, e, runner.config.diffOptions)
       }
 
-      // skipped with new PendingError
-      if (test.result?.pending || test.result?.state === 'skip') {
-        test.mode = 'skip'
-        test.result = { state: 'skip', note: test.result?.note, pending: true }
-        updateTask('test-finished', test, runner)
-        setCurrentTest(undefined)
-        return
-      }
-
       try {
         await runner.onTaskFinished?.(test)
       }
@@ -335,7 +342,7 @@ export async function runTest(test: Test, runner: VitestRunner): Promise<void> {
           test.context,
           suite,
         ])
-        await callCleanupHooks(beforeEachCleanups)
+        await callCleanupHooks(runner, beforeEachCleanups)
         await callFixtureCleanup(test.context)
       }
       catch (e) {
@@ -356,6 +363,20 @@ export async function runTest(test: Test, runner: VitestRunner): Promise<void> {
       test.onFailed = undefined
       test.onFinished = undefined
 
+      // skipped with new PendingError
+      if (test.result?.pending || test.result?.state === 'skip') {
+        test.mode = 'skip'
+        test.result = {
+          state: 'skip',
+          note: test.result?.note,
+          pending: true,
+          duration: now() - start,
+        }
+        updateTask('test-finished', test, runner)
+        setCurrentTest(undefined)
+        return
+      }
+
       if (test.result.state === 'pass') {
         break
       }
@@ -504,7 +525,7 @@ export async function runSuite(suite: Suite, runner: VitestRunner): Promise<void
 
     try {
       await callSuiteHook(suite, suite, 'afterAll', runner, [suite])
-      await callCleanupHooks(beforeAllCleanups)
+      await callCleanupHooks(runner, beforeAllCleanups)
     }
     catch (e) {
       failTask(suite.result, e, runner.config.diffOptions)

test/cli/test/skip-note.test.ts

@@ -17,7 +17,7 @@ test.for([
   })
 
   expect(stderr).toBe('')
-  expect(stdout).toContain('my skipped test [custom message]')
+  expect(stdout).toMatch(/my skipped test (?:\d+ms )?\[custom message\]/)
 
   expect(ctx).toBeDefined()
   const testTask = ctx!.state.getFiles()[0].tasks[0]

test/core/test/hooks.test.ts

@@ -1,4 +1,4 @@
-import { afterAll, afterEach, beforeAll, beforeEach, expect, it, suite } from 'vitest'
+import { afterAll, afterEach, beforeAll, beforeEach, expect, it, onTestFinished, suite } from 'vitest'
 
 let count = -1
 
@@ -103,3 +103,83 @@ suite('hooks cleanup', () => {
     expect(cleanUpCount).toBe(0)
   })
 })
+
+suite('hooks cleanup order', () => {
+  const order: string[] = []
+
+  beforeEach(() => {
+    order.push('[a] beforeEach')
+    return () => {
+      order.push('[a] cleanup')
+    }
+  })
+
+  beforeEach(() => {
+    order.push('[b] beforeEach')
+    return () => {
+      order.push('[b] cleanup')
+    }
+  })
+
+  it('one', () => {
+    expect(order).toEqual([
+      '[a] beforeEach',
+      '[b] beforeEach',
+    ])
+  })
+
+  afterAll(() => {
+    expect(order).toEqual([
+      '[a] beforeEach',
+      '[b] beforeEach',
+      '[b] cleanup',
+      '[a] cleanup',
+    ])
+  })
+})
+
+suite('hooks are called for dynamically skipped tests', () => {
+  const order: string[] = []
+
+  suite('tests', () => {
+    beforeEach(() => {
+      order.push('beforeEach')
+      return () => {
+        order.push('beforeEach cleanup')
+      }
+    })
+    afterEach(() => {
+      order.push('afterEach')
+    })
+
+    beforeAll(() => {
+      order.push('beforeAll')
+      return () => {
+        order.push('beforeAll cleanup')
+      }
+    })
+
+    afterAll(() => {
+      order.push('afterAll')
+    })
+
+    it('skipped', (ctx) => {
+      onTestFinished(() => {
+        order.push('onTestFinished')
+      })
+      ctx.skip()
+    })
+  })
+
+  it('order is correct', () => {
+    expect(order).toEqual([
+      'beforeAll',
+      'beforeEach',
+      'afterEach',
+      'beforeEach cleanup',
+      'onTestFinished',
+      'afterAll',
+      'beforeAll cleanup',
+    ])
+  })
+})