Skip to content

Commit

Permalink
feat!: remove the empty suite from the runner (#5435)
Browse files Browse the repository at this point in the history
  • Loading branch information
sheremet-va authored May 6, 2024
1 parent 2a80e95 commit dbbbe43
Show file tree
Hide file tree
Showing 27 changed files with 694 additions and 221 deletions.
42 changes: 31 additions & 11 deletions packages/runner/src/collect.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { relative } from 'pathe'
import { processError } from '@vitest/utils/error'
import type { File } from './types'
import type { File, SuiteHooks } from './types'
import type { VitestRunner } from './types/runner'
import { calculateSuiteHash, generateHash, interpretTaskModes, someTasksAreOnly } from './utils/collect'
import { clearCollectorContext, getDefaultSuite } from './suite'
import { clearCollectorContext, createSuiteHooks, getDefaultSuite } from './suite'
import { getHooks, setHooks } from './map'
import { collectorContext } from './context'
import { runSetupFiles } from './setup'
Expand All @@ -26,7 +26,9 @@ export async function collectTests(paths: string[], runner: VitestRunner): Promi
tasks: [],
meta: Object.create(null),
projectName: config.name,
file: undefined!,
}
file.file = file

clearCollectorContext(filepath, runner)

Expand All @@ -41,24 +43,27 @@ export async function collectTests(paths: string[], runner: VitestRunner): Promi

const defaultTasks = await getDefaultSuite().collect(file)

setHooks(file, getHooks(defaultTasks))
const fileHooks = createSuiteHooks()
mergeHooks(fileHooks, getHooks(defaultTasks))

for (const c of [...defaultTasks.tasks, ...collectorContext.tasks]) {
if (c.type === 'test') {
file.tasks.push(c)
}
else if (c.type === 'custom') {
file.tasks.push(c)
}
else if (c.type === 'suite') {
if (c.type === 'test' || c.type === 'custom' || c.type === 'suite') {
file.tasks.push(c)
}
else if (c.type === 'collector') {
const suite = await c.collect(file)
if (suite.name || suite.tasks.length)
if (suite.name || suite.tasks.length) {
mergeHooks(fileHooks, getHooks(suite))
file.tasks.push(suite)
}
}
else {
// check that types are exhausted
c satisfies never
}
}

setHooks(file, fileHooks)
file.collectDuration = now() - collectStart
}
catch (e) {
Expand All @@ -74,8 +79,23 @@ export async function collectTests(paths: string[], runner: VitestRunner): Promi
const hasOnlyTasks = someTasksAreOnly(file)
interpretTaskModes(file, config.testNamePattern, hasOnlyTasks, false, config.allowOnly)

file.tasks.forEach((task) => {
// task.suite refers to the internal default suite object
// it should not be reported
if (task.suite?.id === '')
delete task.suite
})
files.push(file)
}

return files
}

function mergeHooks(baseHooks: SuiteHooks, hooks: SuiteHooks): SuiteHooks {
for (const _key in hooks) {
const key = _key as keyof SuiteHooks
baseHooks[key].push(...hooks[key] as any)
}

return baseHooks
}
19 changes: 13 additions & 6 deletions packages/runner/src/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,14 @@ export async function callSuiteHook<T extends keyof SuiteHooks>(
const sequence = runner.config.sequence.hooks

const callbacks: HookCleanupCallback[] = []
if (name === 'beforeEach' && suite.suite) {
// stop at file level
const parentSuite: Suite | null = 'filepath' in suite
? null
: (suite.suite || suite.file)

if (name === 'beforeEach' && parentSuite) {
callbacks.push(
...await callSuiteHook(suite.suite, currentTask, name, runner, args),
...await callSuiteHook(parentSuite, currentTask, name, runner, args),
)
}

Expand All @@ -77,9 +82,9 @@ export async function callSuiteHook<T extends keyof SuiteHooks>(

updateSuiteHookState(currentTask, name, 'pass', runner)

if (name === 'afterEach' && suite.suite) {
if (name === 'afterEach' && parentSuite) {
callbacks.push(
...await callSuiteHook(suite.suite, currentTask, name, runner, args),
...await callSuiteHook(parentSuite, currentTask, name, runner, args),
)
}

Expand Down Expand Up @@ -150,6 +155,8 @@ export async function runTest(test: Test | Custom, runner: VitestRunner) {

setCurrentTest(test)

const suite = test.suite || test.file

const repeats = test.repeats ?? 0
for (let repeatCount = 0; repeatCount <= repeats; repeatCount++) {
const retry = test.retry ?? 0
Expand All @@ -160,7 +167,7 @@ export async function runTest(test: Test | Custom, runner: VitestRunner) {

test.result.repeatCount = repeatCount

beforeEachCleanups = await callSuiteHook(test.suite, test, 'beforeEach', runner, [test.context, test.suite])
beforeEachCleanups = await callSuiteHook(suite, test, 'beforeEach', runner, [test.context, suite])

if (runner.runTask) {
await runner.runTask(test)
Expand Down Expand Up @@ -202,7 +209,7 @@ export async function runTest(test: Test | Custom, runner: VitestRunner) {
}

try {
await callSuiteHook(test.suite, test, 'afterEach', runner, [test.context, test.suite])
await callSuiteHook(suite, test, 'afterEach', runner, [test.context, suite])
await callCleanupHooks(beforeEachCleanups)
await callFixtureCleanup(test.context)
}
Expand Down
19 changes: 14 additions & 5 deletions packages/runner/src/suite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,15 @@ export function getRunner() {
return runner
}

function createDefaultSuite(runner: VitestRunner) {
const config = runner.config.sequence
const api = config.shuffle ? suite.shuffle : suite
return api('', { concurrent: config.concurrent }, () => {})
}

export function clearCollectorContext(filepath: string, currentRunner: VitestRunner) {
if (!defaultSuite)
defaultSuite = currentRunner.config.sequence.shuffle ? suite.shuffle('') : currentRunner.config.sequence.concurrent ? suite.concurrent('') : suite('')
defaultSuite = createDefaultSuite(currentRunner)
runner = currentRunner
currentTestFilepath = filepath
collectorContext.tasks.length = 0
Expand Down Expand Up @@ -121,6 +127,7 @@ function createSuiteCollector(name: string, factory: SuiteFactory = () => { }, m
fails: options.fails,
context: undefined!,
type: 'custom',
file: undefined!,
retry: options.retry ?? runner.config.retry,
repeats: options.repeats,
mode: options.only ? 'only' : options.skip ? 'skip' : options.todo ? 'todo' : 'run',
Expand Down Expand Up @@ -211,10 +218,10 @@ function createSuiteCollector(name: string, factory: SuiteFactory = () => { }, m
name,
mode,
each,
file: undefined!,
shuffle,
tasks: [],
meta: Object.create(null),
projectName: '',
}

if (runner && includeLocation && runner.config.includeTaskLocation) {
Expand All @@ -236,7 +243,10 @@ function createSuiteCollector(name: string, factory: SuiteFactory = () => { }, m
initSuite(false)
}

async function collect(file?: File) {
async function collect(file: File) {
if (!file)
throw new TypeError('File is required to collect tasks.')

factoryQueue.length = 0
if (factory)
await runWithSuite(collector, () => factory(test))
Expand All @@ -251,8 +261,7 @@ function createSuiteCollector(name: string, factory: SuiteFactory = () => { }, m

allChildren.forEach((task) => {
task.suite = suite
if (file)
task.file = file
task.file = file
})

return suite
Expand Down
9 changes: 4 additions & 5 deletions packages/runner/src/types/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ export interface TaskBase {
concurrent?: boolean
shuffle?: boolean
suite?: Suite
file?: File
result?: TaskResult
retry?: number
repeats?: number
Expand All @@ -25,7 +24,7 @@ export interface TaskBase {
}

export interface TaskPopulated extends TaskBase {
suite: Suite
file: File
pending?: boolean
result?: TaskResult
fails?: boolean
Expand Down Expand Up @@ -54,14 +53,14 @@ export interface TaskResult {
export type TaskResultPack = [id: string, result: TaskResult | undefined, meta: TaskMeta]

export interface Suite extends TaskBase {
file: File
type: 'suite'
tasks: Task[]
filepath?: string
projectName: string
}

export interface File extends Suite {
filepath: string
projectName: string
collectDuration?: number
setupDuration?: number
}
Expand Down Expand Up @@ -301,7 +300,7 @@ export interface SuiteCollector<ExtraContext = {}> {
test: TestAPI<ExtraContext>
tasks: (Suite | Custom<ExtraContext> | Test<ExtraContext> | SuiteCollector<ExtraContext>)[]
task: (name: string, options?: TaskCustomOptions) => Custom<ExtraContext>
collect: (file?: File) => Promise<Suite>
collect: (file: File) => Promise<Suite>
clear: () => void
on: <T extends keyof SuiteHooks<ExtraContext>>(name: T, ...fn: SuiteHooks<ExtraContext>[T]) => void
}
Expand Down
7 changes: 5 additions & 2 deletions packages/runner/src/utils/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,14 @@ export function getNames(task: Task) {
const names = [task.name]
let current: Task | undefined = task

while (current?.suite || current?.file) {
current = current.suite || current.file
while (current?.suite) {
current = current.suite
if (current?.name)
names.unshift(current.name)
}

if (current !== task.file)
names.unshift(task.file.name)

return names
}
16 changes: 12 additions & 4 deletions packages/vitest/src/integrations/chai/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import { getCurrentTest } from '@vitest/runner'
import { ASYMMETRIC_MATCHERS_OBJECT, GLOBAL_EXPECT, addCustomEqualityTesters, getState, setState } from '@vitest/expect'
import type { Assertion, ExpectStatic } from '@vitest/expect'
import type { MatcherState } from '../../types/chai'
import { getFullName } from '../../utils/tasks'
import { getCurrentEnvironment } from '../../utils/global'
import { getTestName } from '../../utils/tasks'
import { getCurrentEnvironment, getWorkerState } from '../../utils/global'

export function createExpect(test?: TaskPopulated) {
const expect = ((value: any, message?: string): Assertion => {
Expand All @@ -31,6 +31,7 @@ export function createExpect(test?: TaskPopulated) {
// @ts-expect-error global is not typed
const globalState = getState(globalThis[GLOBAL_EXPECT]) || {}

const testPath = getTestFile(test)
setState<MatcherState>({
// this should also add "snapshotState" that is added conditionally
...globalState,
Expand All @@ -40,8 +41,8 @@ export function createExpect(test?: TaskPopulated) {
expectedAssertionsNumber: null,
expectedAssertionsNumberErrorGen: null,
environment: getCurrentEnvironment(),
testPath: test ? test.suite.file?.filepath : globalState.testPath,
currentTestName: test ? getFullName(test as Test) : globalState.currentTestName,
testPath,
currentTestName: test ? getTestName(test as Test) : globalState.currentTestName,
}, expect)

// @ts-expect-error untyped
Expand Down Expand Up @@ -89,6 +90,13 @@ export function createExpect(test?: TaskPopulated) {
return expect
}

function getTestFile(test?: TaskPopulated) {
if (test)
return test.file.filepath
const state = getWorkerState()
return state.filepath
}

const globalExpect = createExpect()

Object.defineProperty(globalThis, GLOBAL_EXPECT, {
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/integrations/snapshot/chai.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export const SnapshotPlugin: ChaiPlugin = (chai, utils) => {
if (!test)
return {}
return {
filepath: test.file?.filepath,
filepath: test.file.filepath,
name: getNames(test).slice(1).join(' > '),
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/node/reporters/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ export abstract class BaseReporter implements Reporter {

logger.log(`\n${c.cyan(c.inverse(c.bold(' BENCH ')))} ${c.cyan('Summary')}\n`)
for (const bench of topBenches) {
const group = bench.suite
const group = bench.suite || bench.file
if (!group)
continue
const groupName = getFullName(group, c.dim(' > '))
Expand Down
1 change: 1 addition & 0 deletions packages/vitest/src/node/reporters/junit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ export class JUnitReporter implements Reporter {
// NOTE: not used in JUnitReporter
context: null as any,
suite: null as any,
file: null as any,
} satisfies Task)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ function renderTree(tasks: Task[], options: ListRendererOptions, level = 0, maxR
let suffix = ''
let prefix = ` ${getStateSymbol(task)} `

if (level === 0 && task.type === 'suite' && task.projectName)
if (level === 0 && task.type === 'suite' && 'projectName' in task)
prefix += formatProjectName(task.projectName)

if (task.type === 'test' && task.result?.retryCount && task.result.retryCount > 0)
Expand Down
4 changes: 2 additions & 2 deletions packages/vitest/src/node/reporters/verbose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ export class VerboseReporter extends DefaultReporter {
const task = this.ctx.state.idMap.get(pack[0])
if (task && task.type === 'test' && task.result?.state && task.result?.state !== 'run') {
let title = ` ${getStateSymbol(task)} `
if (task.suite?.projectName)
title += formatProjectName(task.suite.projectName)
if (task.file.projectName)
title += formatProjectName(task.file.projectName)
title += getFullName(task, c.dim(' > '))
if (task.result.duration != null && task.result.duration > this.ctx.config.slowTestThreshold)
title += c.yellow(` ${Math.round(task.result.duration)}${c.dim('ms')}`)
Expand Down
33 changes: 19 additions & 14 deletions packages/vitest/src/node/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,19 +150,24 @@ export class StateManager {
}

cancelFiles(files: string[], root: string, projectName: string) {
this.collectFiles(files.map(filepath => ({
filepath,
name: relative(root, filepath),
id: filepath,
mode: 'skip',
type: 'suite',
result: {
state: 'skip',
},
meta: {},
// Cancelled files have not yet collected tests
tasks: [],
projectName,
})))
this.collectFiles(files.map((filepath) => {
const file: File = {
filepath,
name: relative(root, filepath),
id: filepath,
mode: 'skip',
type: 'suite',
result: {
state: 'skip',
},
meta: {},
// Cancelled files have not yet collected tests
tasks: [],
projectName,
file: null!,
}
file.file = file
return file
}))
}
}
Loading

0 comments on commit dbbbe43

Please sign in to comment.