From 6c5a447203a29a0708fbc5de4fb91ea501bdc395 Mon Sep 17 00:00:00 2001 From: Vladimir Date: Thu, 15 Oct 2026 15:20:29 +0200 Subject: [PATCH] fix: run hook cleanups when a later hook fails (#11605) --- packages/vitest/src/runtime/runner/run.ts | 48 +++- test/e2e/test/concurrent.test.ts | 4 + test/e2e/test/hook-cleanup.test.ts | 310 ++++++++++++++++++++++ 3 files changed, 348 insertions(+), 14 deletions(-) create mode 100644 test/e2e/test/hook-cleanup.test.ts diff --git a/packages/vitest/src/runtime/runner/run.ts b/packages/vitest/src/runtime/runner/run.ts index 6e8299535..257c093b9 100644 --- a/packages/vitest/src/runtime/runner/run.ts +++ b/packages/vitest/src/runtime/runner/run.ts @@ -178,15 +178,15 @@ async function callSuiteHook( name: T, runner: VitestRunner, args: SuiteHooks[T][0] extends (...args: infer A) => Awaitable ? A : never, -): Promise { + cleanups: unknown[] = [], +): Promise { 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( } 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() + 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( } 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 { 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 { 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 { 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 { 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) diff --git a/test/e2e/test/concurrent.test.ts b/test/e2e/test/concurrent.test.ts index c3342b3a4..561ebcf53 100644 --- a/test/e2e/test/concurrent.test.ts +++ b/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".", ], }, diff --git a/test/e2e/test/hook-cleanup.test.ts b/test/e2e/test/hook-cleanup.test.ts new file mode 100644 index 000000000..918881070 --- /dev/null +++ b/test/e2e/test/hook-cleanup.test.ts @@ -0,0 +1,310 @@ +import { expect, test } from 'vitest' +import { runInlineTests } from '../../test-utils' + +function extractLogs(stdout: string): string { + return stdout + .split('\n') + .filter((l) => l.includes('>>')) + .join('\n') +} + +test.for(['stack', 'list', 'parallel'] as const)( + 'runs the cleanup of a resolved hook if a later hook fails with sequence.hooks: %s', + async (hooks) => { + const { stdout, errorTree } = await runInlineTests( + { + 'basic.test.js': ` + import { afterAll, afterEach, beforeAll, beforeEach, describe, test } from 'vitest' + + describe('beforeAll', () => { + beforeAll(async () => { + await new Promise((resolve) => setTimeout(resolve, 10)) + console.log('>> beforeAll resolved') + return () => console.log('>> beforeAll cleanup') + }) + beforeAll(() => { + throw new Error('beforeAll fails') + }) + afterAll(() => console.log('>> afterAll')) + test('skipped', () => {}) + }) + + describe('beforeEach', () => { + beforeEach(async () => { + await new Promise((resolve) => setTimeout(resolve, 10)) + console.log('>> beforeEach resolved') + return () => console.log('>> beforeEach cleanup') + }) + beforeEach(() => { + throw new Error('beforeEach fails') + }) + afterEach(() => console.log('>> afterEach')) + test('fails', () => {}) + }) + `, + }, + { sequence: { hooks } }, + ) + + expect(extractLogs(stdout)).toMatchInlineSnapshot(` + ">> beforeAll resolved + >> afterAll + >> beforeAll cleanup + >> beforeEach resolved + >> afterEach + >> beforeEach cleanup" + `) + expect(errorTree()).toMatchInlineSnapshot(` + { + "basic.test.js": { + "beforeAll": { + "__suite_errors__": [ + "beforeAll fails", + ], + "skipped": "skipped", + }, + "beforeEach": { + "fails": [ + "beforeEach fails", + ], + }, + }, + } + `) + }, +) + +test('reports every failed hook with sequence.hooks: parallel', async () => { + const { errorTree } = await runInlineTests( + { + 'basic.test.js': ` + import { beforeAll, beforeEach, describe, test } from 'vitest' + + describe('beforeAll', () => { + beforeAll(() => { + throw new Error('first beforeAll fails') + }) + beforeAll(() => { + throw new Error('second beforeAll fails') + }) + test('skipped', () => {}) + }) + + describe('beforeEach', () => { + beforeEach(() => { + throw new Error('first beforeEach fails') + }) + beforeEach(() => { + throw new Error('second beforeEach fails') + }) + test('fails', () => {}) + }) + + describe('skip', () => { + beforeEach(({ skip }) => skip()) + beforeEach(({ skip }) => skip()) + test('skipped', () => {}) + }) + `, + }, + { sequence: { hooks: 'parallel' } }, + ) + + expect(errorTree()).toMatchInlineSnapshot(` + { + "basic.test.js": { + "beforeAll": { + "__suite_errors__": [ + "first beforeAll fails", + "second beforeAll fails", + ], + "skipped": "skipped", + }, + "beforeEach": { + "fails": [ + "first beforeEach fails", + "second beforeEach fails", + ], + }, + "skip": { + "skipped": "skipped", + }, + }, + } + `) +}) + +test('reports the abort reason once if parallel hooks reject with it', async () => { + const { errorTree } = await runInlineTests( + { + 'basic.test.js': ` + import { beforeEach, test } from 'vitest' + + beforeEach(() => new Promise((resolve) => setTimeout(resolve, 1000)), 100) + beforeEach(async ({ signal }) => { + await new Promise((resolve) => signal.addEventListener('abort', resolve)) + signal.throwIfAborted() + }) + test('fails', () => {}) + `, + }, + { sequence: { hooks: 'parallel' } }, + ) + + expect(errorTree()).toMatchInlineSnapshot(` + { + "basic.test.js": { + "fails": [ + "Hook timed out in 100ms. + If this is a long-running hook, pass a timeout value as the last argument or configure it globally with "hookTimeout".", + ], + }, + } + `) +}) + +test('runs cleanups in sequence.hooks order if a later hook fails', async () => { + const logs: Record = {} + for (const hooks of ['stack', 'list'] as const) { + const { stdout } = await runInlineTests( + { + 'basic.test.js': ` + import { afterAll, afterEach, beforeAll, beforeEach, describe, test } from 'vitest' + + beforeEach(() => () => console.log('>> root beforeEach cleanup')) + + describe('beforeAll', () => { + beforeAll(() => () => console.log('>> first beforeAll cleanup')) + beforeAll(() => () => console.log('>> second beforeAll cleanup')) + beforeAll(() => { + throw new Error('beforeAll fails') + }) + afterAll(() => console.log('>> afterAll')) + test('skipped', () => {}) + }) + + describe('beforeEach', () => { + beforeEach(() => () => console.log('>> first beforeEach cleanup')) + beforeEach(() => () => console.log('>> second beforeEach cleanup')) + beforeEach(() => { + throw new Error('beforeEach fails') + }) + afterEach(() => console.log('>> afterEach')) + test('fails', () => {}) + }) + `, + }, + { sequence: { hooks } }, + ) + logs[hooks] = extractLogs(stdout) + } + + expect(logs).toMatchInlineSnapshot(` + { + "list": ">> afterAll + >> first beforeAll cleanup + >> second beforeAll cleanup + >> afterEach + >> root beforeEach cleanup + >> first beforeEach cleanup + >> second beforeEach cleanup", + "stack": ">> afterAll + >> second beforeAll cleanup + >> first beforeAll cleanup + >> afterEach + >> second beforeEach cleanup + >> first beforeEach cleanup + >> root beforeEach cleanup", + } + `) +}) + +test('runs beforeEach cleanups of parent suites on every retry', async () => { + const { stdout, stderr, errorTree } = await runInlineTests({ + 'basic.test.js': ` + import { afterEach, beforeEach, describe, test } from 'vitest' + + beforeEach(() => () => console.log('>> root cleanup')) + + describe('nested', () => { + let attempt = 0 + beforeEach(() => () => console.log('>> nested cleanup')) + beforeEach(() => { + if (attempt++ === 0) { + throw new Error('beforeEach fails') + } + }) + afterEach(() => console.log('>> afterEach')) + test('retried', { retry: 1 }, () => console.log('>> test')) + }) + `, + }) + + expect(stderr).toBe('') + expect(extractLogs(stdout)).toMatchInlineSnapshot(` + ">> afterEach + >> nested cleanup + >> root cleanup + >> test + >> afterEach + >> nested cleanup + >> root cleanup" + `) + expect(errorTree()).toMatchInlineSnapshot(` + { + "basic.test.js": { + "nested": { + "retried": "passed", + }, + }, + } + `) +}) + +test('reports an error thrown by the cleanup together with the hook error', async () => { + const { errorTree } = await runInlineTests({ + 'basic.test.js': ` + import { beforeAll, beforeEach, describe, test } from 'vitest' + + describe('beforeAll', () => { + beforeAll(() => () => { + throw new Error('beforeAll cleanup fails') + }) + beforeAll(() => { + throw new Error('beforeAll fails') + }) + test('skipped', () => {}) + }) + + describe('beforeEach', () => { + beforeEach(() => () => { + throw new Error('beforeEach cleanup fails') + }) + beforeEach(() => { + throw new Error('beforeEach fails') + }) + test('fails', () => {}) + }) + `, + }) + + expect(errorTree()).toMatchInlineSnapshot(` + { + "basic.test.js": { + "beforeAll": { + "__suite_errors__": [ + "beforeAll fails", + "beforeAll cleanup fails", + ], + "skipped": "skipped", + }, + "beforeEach": { + "fails": [ + "beforeEach fails", + "beforeEach cleanup fails", + ], + }, + }, + } + `) +}) -- 2.54.0