Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: don't use Custom type and pass down test context to test hooks instead of a test result #7028

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/api/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -1113,7 +1113,7 @@ These hooks will throw an error if they are called outside of the test body.

### onTestFinished {#ontestfinished}

This hook is always called after the test has finished running. It is called after `afterEach` hooks since they can influence the test result. It receives a `TaskResult` object with the current test result.
This hook is always called after the test has finished running. It is called after `afterEach` hooks since they can influence the test result. It receives a `TestContext` object.

```ts {1,5}
import { onTestFinished, test } from 'vitest'
Expand Down Expand Up @@ -1170,7 +1170,7 @@ This hook is always called in reverse order and is not affected by [`sequence.ho

### onTestFailed

This hook is called only after the test has failed. It is called after `afterEach` hooks since they can influence the test result. It receives a `TaskResult` object with the current test result. This hook is useful for debugging.
This hook is called only after the test has failed. It is called after `afterEach` hooks since they can influence the test result. It receives a `TestContext` object. This hook is useful for debugging.

```ts {1,5-7}
import { onTestFailed, test } from 'vitest'
Expand Down
2 changes: 1 addition & 1 deletion packages/runner/src/collect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export async function collectTests(
mergeHooks(fileHooks, getHooks(defaultTasks))

for (const c of [...defaultTasks.tasks, ...collectorContext.tasks]) {
if (c.type === 'test' || c.type === 'custom' || c.type === 'suite') {
if (c.type === 'test' || c.type === 'suite') {
file.tasks.push(c)
}
else if (c.type === 'collector') {
Expand Down
34 changes: 25 additions & 9 deletions packages/runner/src/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { Awaitable } from '@vitest/utils'
import type { DiffOptions } from '@vitest/utils/diff'
import type { FileSpec, VitestRunner } from './types/runner'
import type {
Custom,
ExtendedContext,
File,
HookCleanupCallback,
HookListener,
Expand Down Expand Up @@ -63,32 +63,48 @@ function getSuiteHooks(

async function callTestHooks(
runner: VitestRunner,
task: Task,
hooks: ((result: TaskResult) => Awaitable<void>)[],
test: Test,
hooks: ((result: ExtendedContext<Test>) => Awaitable<void>)[],
sequence: SequenceHooks,
) {
if (sequence === 'stack') {
hooks = hooks.slice().reverse()
}

if (!hooks.length) {
return
}

const onTestFailed = test.context.onTestFailed
const onTestFinished = test.context.onTestFinished
test.context.onTestFailed = () => {
throw new Error(`Cannot call "onTestFailed" inside a test hook.`)
}
test.context.onTestFinished = () => {
throw new Error(`Cannot call "onTestFinished" inside a test hook.`)
}

if (sequence === 'parallel') {
try {
await Promise.all(hooks.map(fn => fn(task.result!)))
await Promise.all(hooks.map(fn => fn(test.context)))
}
catch (e) {
failTask(task.result!, e, runner.config.diffOptions)
failTask(test.result!, e, runner.config.diffOptions)
}
}
else {
for (const fn of hooks) {
try {
await fn(task.result!)
await fn(test.context)
}
catch (e) {
failTask(task.result!, e, runner.config.diffOptions)
failTask(test.result!, e, runner.config.diffOptions)
}
}
}

test.context.onTestFailed = onTestFailed
test.context.onTestFinished = onTestFinished
}

export async function callSuiteHook<T extends keyof SuiteHooks>(
Expand Down Expand Up @@ -177,7 +193,7 @@ async function callCleanupHooks(cleanups: HookCleanupCallback[]) {
)
}

export async function runTest(test: Test | Custom, runner: VitestRunner): Promise<void> {
export async function runTest(test: Test, runner: VitestRunner): Promise<void> {
await runner.onBeforeRunTask?.(test)

if (test.mode !== 'run') {
Expand Down Expand Up @@ -471,7 +487,7 @@ export async function runSuite(suite: Suite, runner: VitestRunner): Promise<void
let limitMaxConcurrency: ReturnType<typeof limitConcurrency>

async function runSuiteChild(c: Task, runner: VitestRunner) {
if (c.type === 'test' || c.type === 'custom') {
if (c.type === 'test') {
return limitMaxConcurrency(() => runTest(c, runner))
}
else if (c.type === 'suite') {
Expand Down
17 changes: 5 additions & 12 deletions packages/runner/src/suite.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import type { FixtureItem } from './fixture'
import type { VitestRunner } from './types/runner'
import type {
Custom,
CustomAPI,
File,
Fixtures,
Expand Down Expand Up @@ -250,15 +249,9 @@ function parseArguments<T extends (...args: any[]) => any>(

// it('', () => {}, { retry: 2 })
if (typeof optionsOrTest === 'object') {
// it('', { retry: 2 }, { retry: 3 })
if (typeof optionsOrFn === 'object') {
throw new TypeError(
'Cannot use two objects as arguments. Please provide options and a function callback in that order.',
)
}
// TODO: more info, add a name
// console.warn('The third argument is deprecated. Please use the second argument for options.')
options = optionsOrTest
throw new TypeError(
'Expected the third argument to be a function or a timeout number, instead got an object. If you need to configure options, use the second argument.',
)
}
// it('', () => {}, 1000)
else if (typeof optionsOrTest === 'number') {
Expand Down Expand Up @@ -296,7 +289,7 @@ function createSuiteCollector(
each?: boolean,
suiteOptions?: TestOptions,
) {
const tasks: (Test | Custom | Suite | SuiteCollector)[] = []
const tasks: (Test | Suite | SuiteCollector)[] = []
const factoryQueue: (Test | Suite | SuiteCollector)[] = []

let suite: Suite
Expand Down Expand Up @@ -503,7 +496,7 @@ function createSuite() {
this: Record<string, boolean | undefined>,
name: string | Function,
factoryOrOptions?: SuiteFactory | TestOptions,
optionsOrFactory: number | TestOptions | SuiteFactory = {},
optionsOrFactory?: number | TestOptions | SuiteFactory,
) {
const mode: RunMode = this.only
? 'only'
Expand Down
8 changes: 3 additions & 5 deletions packages/runner/src/types/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ export interface Custom<ExtraContext = object> extends TaskPopulated {
context: TaskContext<Test> & ExtraContext & TestContext
}

export type Task = Test | Suite | Custom | File
export type Task = Test | Suite | File

/**
* @deprecated Vitest doesn't provide `done()` anymore
Expand Down Expand Up @@ -572,8 +572,6 @@ export interface SuiteCollector<ExtraContext = object> {
test: TestAPI<ExtraContext>
tasks: (
| Suite
// TODO: remove in Vitest 3
| Custom<ExtraContext>
| Test<ExtraContext>
| SuiteCollector<ExtraContext>
)[]
Expand Down Expand Up @@ -629,8 +627,8 @@ export interface TaskContext<Task extends Test = Test> {
export type ExtendedContext<T extends Test> = TaskContext<T> &
TestContext

export type OnTestFailedHandler = (result: TaskResult) => Awaitable<void>
export type OnTestFinishedHandler = (result: TaskResult) => Awaitable<void>
export type OnTestFailedHandler = (result: ExtendedContext<Test>) => Awaitable<void>
export type OnTestFinishedHandler = (result: ExtendedContext<Test>) => Awaitable<void>

export interface TaskHook<HookListener> {
(fn: HookListener, timeout?: number): void
Expand Down
12 changes: 6 additions & 6 deletions packages/runner/src/utils/tasks.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
import type { Custom, Suite, Task, Test } from '../types/tasks'
import type { Suite, Task, Test } from '../types/tasks'
import { type Arrayable, toArray } from '@vitest/utils'

/**
* @deprecated use `isTestCase` instead
*/
export function isAtomTest(s: Task): s is Test | Custom {
export function isAtomTest(s: Task): s is Test {
return isTestCase(s)
}

export function isTestCase(s: Task): s is Test | Custom {
return s.type === 'test' || s.type === 'custom'
export function isTestCase(s: Task): s is Test {
return s.type === 'test'
}

export function getTests(suite: Arrayable<Task>): (Test | Custom)[] {
const tests: (Test | Custom)[] = []
export function getTests(suite: Arrayable<Task>): (Test)[] {
const tests: Test[] = []
const arraySuites = toArray(suite)
for (const s of arraySuites) {
if (isTestCase(s)) {
Expand Down
7 changes: 3 additions & 4 deletions packages/vitest/src/node/reporters/reported-tasks.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type {
Custom as RunnerCustomCase,
Task as RunnerTask,
Test as RunnerTestCase,
File as RunnerTestFile,
Expand Down Expand Up @@ -56,7 +55,7 @@ class ReportedTaskImplementation {
export class TestCase extends ReportedTaskImplementation {
#fullName: string | undefined

declare public readonly task: RunnerTestCase | RunnerCustomCase
declare public readonly task: RunnerTestCase
public readonly type = 'test'

/**
Expand All @@ -79,7 +78,7 @@ export class TestCase extends ReportedTaskImplementation {
*/
public readonly parent: TestSuite | TestModule

protected constructor(task: RunnerTestCase | RunnerCustomCase, project: TestProject) {
protected constructor(task: RunnerTestCase, project: TestProject) {
super(task, project)

this.name = task.name
Expand Down Expand Up @@ -405,7 +404,7 @@ export interface TaskOptions {
}

function buildOptions(
task: RunnerTestCase | RunnerCustomCase | RunnerTestFile | RunnerTestSuite,
task: RunnerTestCase | RunnerTestFile | RunnerTestSuite,
): TaskOptions {
return {
each: task.each,
Expand Down
Loading