Skip to content

Commit

Permalink
fix(runner): fix fixture cleanup for concurrent tests (vitest-dev#4827)
Browse files Browse the repository at this point in the history
  • Loading branch information
hi-ogawa authored and zmullett committed Jan 4, 2024
1 parent 54dc582 commit 96bf8fc
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 4 deletions.
11 changes: 8 additions & 3 deletions packages/runner/src/fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,13 @@ export function mergeContextFixtures(fixtures: Record<string, any>, context: { f
}

const fixtureValueMaps = new Map<TestContext, Map<FixtureItem, any>>()
let cleanupFnArray = new Array<() => void | Promise<void>>()
const cleanupFnArrayMap = new Map<TestContext, Array<() => void | Promise<void>>>()

export async function callFixtureCleanup() {
export async function callFixtureCleanup(context: TestContext) {
const cleanupFnArray = cleanupFnArrayMap.get(context) ?? []
for (const cleanup of cleanupFnArray.reverse())
await cleanup()
cleanupFnArray = []
cleanupFnArrayMap.delete(context)
}

export function withFixtures(fn: Function, testContext?: TestContext) {
Expand All @@ -73,6 +74,10 @@ export function withFixtures(fn: Function, testContext?: TestContext) {
fixtureValueMaps.set(context, new Map<FixtureItem, any>())
const fixtureValueMap: Map<FixtureItem, any> = fixtureValueMaps.get(context)!

if (!cleanupFnArrayMap.has(context))
cleanupFnArrayMap.set(context, [])
const cleanupFnArray = cleanupFnArrayMap.get(context)!

const usedFixtures = fixtures.filter(({ prop }) => usedProps.includes(prop))
const pendingFixtures = resolveDeps(usedFixtures)

Expand Down
2 changes: 1 addition & 1 deletion packages/runner/src/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ export async function runTest(test: Test | Custom, runner: VitestRunner) {
try {
await callSuiteHook(test.suite, test, 'afterEach', runner, [test.context, test.suite])
await callCleanupHooks(beforeEachCleanups)
await callFixtureCleanup()
await callFixtureCleanup(test.context)
}
catch (e) {
failTask(test.result, e, runner.config.diffOptions)
Expand Down
45 changes: 45 additions & 0 deletions test/core/test/fixture-concurrent-beforeEach.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { afterAll, beforeEach, expect, test } from 'vitest'

// this test case might look exotic, but a few conditions were required to reproduce the reported bug.
// such particular conditions are marked with "[repro]" in the comments.

let globalA = 0
let globalB = 0

interface MyFixtures {
a: number
b: number
}

export const myTest = test.extend<MyFixtures>({
// [repro] fixture order must be { a, b } and not { b, a }
a: async ({}, use) => {
globalA++
await new Promise<void>(resolve => setTimeout(resolve, 200)) // [repro] async fixture
await use(globalA)
},
b: async ({}, use) => {
globalB++
await use(globalB)
},
})

// [repro] beforeEach uses only "b"
beforeEach<MyFixtures>(({ b }) => {
expect(b).toBeTypeOf('number')
})

afterAll(() => {
expect([globalA, globalB]).toEqual([2, 2])
})

// [repro] concurrent test uses both "a" and "b"
myTest.concurrent('test1', async ({ a, b }) => {
expect(a).toBeTypeOf('number')
expect(b).toBeTypeOf('number')
})

myTest.concurrent('test2', async ({ a, b }) => {
expect(a).toBeTypeOf('number')
expect(b).toBeTypeOf('number')
})

0 comments on commit 96bf8fc

Please sign in to comment.