fixtures are not preserved between beforeEach and test

[Trinovantes]

Describe the bug

Fixture instances are not preserved between beforeEach and test. This is counterintuitive because it defeats the purpose of having fixtures in beforeEach (I'm a bit biased because this is opposite to how playwright behaves)

Reproduction

class ApiMocker {
    items = new Array<string>()

    mockItems(n: number) {
        for (let i = 0; i < n; i++) {
            this.items.push('test')
        }
    }
}

type Fixtures = { api: ApiMocker }

const apiTest = test.extend<Fixtures>({
    api: async({}, use) => {
        console.log('new instance')
        const apiMocker = new ApiMocker()
        await use(apiMocker)
    },
})

beforeEach<Fixtures>(({ api }) => {
    // prints 'new instance'
    api.mockItems(100)
})

apiTest('have 100 items', ({ api }) => {
    // prints 'new instance' again
    expect(api.items.length).toBe(100) // fails
})

System Info

System:
    OS: Linux 5.15 Ubuntu 20.04.6 LTS (Focal Fossa)
    CPU: (12) x64 AMD Ryzen 5 2600 Six-Core Processor
    Memory: 6.85 GB / 15.59 GB
    Container: Yes
    Shell: 5.0.17 - /bin/bash
  Binaries:
    Node: 20.6.1 - /usr/local/bin/node
    Yarn: 1.22.19 - ~/.yarn/bin/yarn
    npm: 10.1.0 - /usr/local/bin/npm
    pnpm: 7.9.0 - ~/.yarn/bin/pnpm
  npmPackages:
    @vitejs/plugin-vue: ^4.3.4 => 4.3.4
    vite: ^4.4.9 => 4.4.9
    vitest: ^0.34.3 => 0.34.5

### Used Package Manager

yarn

### Validations

- [X] Follow our [Code of Conduct](https://github.com/vitest-dev/vitest/blob/main/CODE_OF_CONDUCT.md)
- [X] Read the [Contributing Guidelines](https://github.com/vitest-dev/vitest/blob/main/CONTRIBUTING.md).
- [X] Read the [docs](https://vitest.dev/guide/).
- [X] Check that there isn't [already an issue](https://github.com/vitest-dev/vitest/issues) that reports the same bug to avoid creating a duplicate.
- [X] Check that this is a concrete bug. For Q&A open a [GitHub Discussion](https://github.com/vitest-dev/vitest/discussions) or join our [Discord Chat Server](https://chat.vitest.dev).
- [X] The provided reproduction is a [minimal reproducible example](https://stackoverflow.com/help/minimal-reproducible-example) of the bug.

[sheremet-va]

It sounds like a bug 🤔 We should have a test that tests this specifically 🤔

[sheremet-va]

@Dunqing what do you think?

[Dunqing]

I think this is a bug. Does this only happen in the latest version?

[sheremet-va]

I think this is a bug. Does this only happen in the latest version?

Yes, it was not possible to access these fixtures in previous versions.

[Trinovantes]

Sorry to bother you again but I don't think #4168 resolves my issue

The new behavior in v0.34.6 (with #4168) is:

init apiMocker
beforEach
test 1

beforEach
test 2

Whereas my desired behavior (similar to playwright) is:

init apiMocker
beforEach
test 1

init apiMocker (missing in 0.34.6)
beforEach
test 2

Although I suppose this could be a philosophical question on whether or not fixtures should only be initialized once. Personally since vitest isn't v1 yet, I'd like to change it to be as consistent with playwright as possible.

---

Just for reference, in v0.34.5 when I originally opened this issue:

init apiMocker
beforEach
init apiMocker (don't want)
test 1

init apiMocker
beforEach
init apiMocker (don't want)
test 2

[sheremet-va]

Personally since vitest isn't v1 yet, I'd like to change it to be as consistent with playwright as possible

Agreed 👍🏻

@Dunqing what do you think?

[Dunqing]

While I think it's better to be consistent with playwright's behaviour, the current behaviour makes more sense.

In general, we don't need to initialize Fixtures for every test, which is a big performance overhead. However, you may need to re-initialize Fixtures for some test cases, so you can write those test cases in different describe since the same describe scope is only initialized once.

You can see this example.

describe('case1', () => {
 test('xxx', () => {})
})

describe('case2', () => {
 test('xxx', () => {})
})

This way we will initialize the fixtures once in both describe.

[Trinovantes]

Performance issues aside, I think it makes more sense for fixtures to be scoped to individual tests. I use test fixtures for mocking my API and provisioning databases. Since they have internal states, I want to be able to access a fresh instance for every test.

Essentially I'm replacing this pattern:

let db
beforeEach(() => {
  db = createSqliteDb(':memory:')
  seedData(db)
})
afterEach(() => {
  db.close()
})
test('test 1', () => {
  doSomething(db)
  expect(db.query()).toBe(...)
})
test('test 2', () => {
  doSomethingElse(db)
  expect(db.query()).toBe(...)
})

With:

export const dbTest = test.extend({
  db: async({ use }) => {
    const db = createSqliteDb(':memory:')
    seedData(db)
    await use(db)
    db.close()
  }
})

dbTest('test 1', ({ db }) => {
  doSomething(db)
  expect(db.query()).toBe(...)
})
dbTest('test 2', ({ db }) => {
  doSomethingElse(db)
  expect(db.query()).toBe(...)
})

I'm not sure if this pattern is specific to playwright but I find it to be cleaner and allow reusability.

Not having access to a fresh instance (for this use case) would defeat the purpose of fixtures for me since I would have to write the beforeEach/afterEach every time I want to access db:

export const dbTest = test.extend({
  db: async({ use }) => {
    const db = createSqliteDb(':memory:')
    await use(db)
    db.close()
  }
})

// have to be rewritten in every file that wants to use dbTest
beforeEach(({ db }) => {
  seedData(db)
})
afterEach(({ db }) => {
  dropTables(db)
})

[Dunqing]

Yes, In your case, It is more useful to initialize fixtures for each test. Does writing your need to re-initialise fixtures of test cases in describe get complicated?

[sheremet-va]

While I think it's better to be consistent with playwright's behaviour, the current behaviour makes more sense.

I would disagree. As a user, I would expect init apiMocker to be called before each test and passed it down to beforeEach (so, new value each time). Performance considerations shouldn't be taken into account yet.

[Dunqing]

I would disagree. As a user, I would expect init apiMocker to be called before each test and passed it down to beforeEach (so, new value each time). Performance considerations shouldn't be taken into account yet.

Okay, I will handle this soon