fix(runner): fix fixture cleanup for concurrent tests (#4827)

Author: hi-ogawa

packages/runner/src/fixture.ts

@@ -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) {
@@ -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)
 

packages/runner/src/run.ts

@@ -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)

test/core/test/fixture-concurrent-beforeEach.test.ts

@@ -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')
+})