Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 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
7 changes: 6 additions & 1 deletion packages/runner/src/fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ const contextHasFixturesCache = new WeakMap<TestContext, WeakSet<TestFixtureItem

export function withFixtures(fn: Function, options?: WithFixturesOptions) {
const collector = getCurrentSuite()
const suite = collector.suite || collector.file
const collectorSuite = collector.suite || collector.file
return async (hookContext?: TestContext): Promise<any> => {
const context: (TestContext & { [key: string]: any }) | undefined = hookContext || options?.context as TestContext

Expand All @@ -314,6 +314,11 @@ export function withFixtures(fn: Function, options?: WithFixturesOptions) {
return fn(context)
}

// For `xxxEach` hooks, use the test's parent suite to pick up fixture
// overrides registered in the test's describe block.
// For `test`, `collectorSuite` is already same as `context.task.suite`.
// For `xxxAll` hooks, there's no `context.task`.
const suite = context.task?.suite ?? collectorSuite

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to avoid using context.task anywhere in fixtures resolution, it's an implied value that makes this harder to track, in my opinion. Could we pass suite down as an option instead?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see it, but that indeed shows the flow better. I think this is what you mean. Updated.

const registrations = fixtures.get(suite)
if (!registrations.size) {
return fn(context)
Expand Down
133 changes: 133 additions & 0 deletions test/core/test/test-extend.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -640,3 +640,136 @@ describe('builder pattern with non-function values', () => {
expect(chainedSync).toBe('HELLO WORLD')
})
})

// https://github.com/vitest-dev/vitest/issues/9810
describe('override auto fixture with outer beforeEach', () => {
const myTest = test
.extend('base', { auto: true }, () => 'base:default')
.extend('derived', { auto: true }, ({ base }) => {
return `derived:${base}`
})

beforeEach(({ task }) => {
expect(task).toBeTruthy()
})

describe('with override', () => {
myTest.override('base', 'base:override')

myTest('auto fixture sees overridden dependency', ({ base, derived }) => {
expect(base).toBe('base:override')
expect(derived).toBe('derived:base:override')
})
})
})

describe('override auto fixture with co-located beforeEach', () => {
const myTest = test
.extend('base', { auto: true }, () => 'base:default')
.extend('derived', { auto: true }, ({ base }) => {
return `derived:${base}`
})

myTest.override('base', 'base:override')

beforeEach(({ task }) => {
expect(task).toBeTruthy()
})

myTest('override applies when beforeEach is co-located', ({ base, derived }) => {
expect(base).toBe('base:override')
expect(derived).toBe('derived:base:override')
})
})

describe('override non-auto fixture with outer beforeEach', () => {
const myTest = test
.extend('base', () => 'base:default')
.extend('derived', ({ base }) => `derived:${base}`)

beforeEach(({ task }) => {
expect(task).toBeTruthy()
})

describe('with override', () => {
myTest.override('base', 'base:override')

myTest('override applies to non-auto dependency', ({ base, derived }) => {
expect(base).toBe('base:override')
expect(derived).toBe('derived:base:override')
})
})
})

describe('override fixture accessed in outer beforeEach', () => {
const myTest = test
.extend('base', () => 'base:default')
.extend('derived', ({ base }) => `derived:${base}`)

const hookValues: string[] = []

myTest.beforeEach(({ base }) => {
hookValues.push(base)
})

describe('with override', () => {
myTest.override('base', 'base:override')

myTest('beforeEach sees overridden fixture', ({ derived }) => {
expect(hookValues).toEqual(['base:override'])
expect(derived).toBe('derived:base:override')
})
})
})
Comment on lines +704 to +723

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue wasn't only auto: true case. This was also failing with the same reason that beforeEach outside of describe aren't picking up inner override.


describe('nested overrides with outer beforeEach', () => {
const myTest = test
.extend('base', { auto: true }, () => 'base:default')
.extend('derived', { auto: true }, ({ base }) => {
return `derived:${base}`
})

beforeEach(({ task }) => {
expect(task).toBeTruthy()
})

describe('outer', () => {
myTest.override('base', 'base:outer')

myTest('outer override', ({ base, derived }) => {
expect(base).toBe('base:outer')
expect(derived).toBe('derived:base:outer')
})

describe('inner', () => {
myTest.override('base', 'base:inner')

myTest('inner override wins', ({ base, derived }) => {
expect(base).toBe('base:inner')
expect(derived).toBe('derived:base:inner')
})
})
})
})

describe('override fixture accessed in aroundEach', () => {
const myTest = test
.extend('base', () => 'base:default')
.extend('derived', ({ base }) => `derived:${base}`)

const hookValues: string[] = []

myTest.aroundEach(async (runTest, { base }) => {
hookValues.push(base)
await runTest()
})

describe('with override', () => {
myTest.override('base', 'base:override')

myTest('aroundEach sees overridden fixture', ({ derived }) => {
expect(hookValues).toEqual(['base:override'])
expect(derived).toBe('derived:base:override')
})
})
})
Loading