No stdout from beforeEach/afterEach when failing test and `--no-isolate --no-file-parallelism`

Janpot commented 1 week ago

Describe the bug

Logs from beforeEach/afterEach are missing when I run my tests with --no-isolate --no-file-parallelism and one of the suites has a failing test

Reproduction

Open https://stackblitz.com/edit/vitest-dev-vitest-88szjo?file=README.md
Run pnpm test run
Notice twice printed
```
before
after
```
Run pnpm test run -- --no-isolate --no-file-parallelism
Notice only once printed
```
before
after
```

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.20.3 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.2.3 - /usr/local/bin/npm
    pnpm: 8.15.6 - /usr/local/bin/pnpm
  npmPackages:
    @vitest/ui: latest => 2.1.2 
    vite: latest => 5.4.8 
    vitest: latest => 2.1.2

Used Package Manager

pnpm

Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] The provided reproduction is a minimal reproducible example of the bug.

AriPerkkio commented 1 week ago

Setup file is run only once for the test runner due to --no-isolate. A single test runner is spawned due to --no-file-parallelism. The fact that there is a failing test case here doesn't affect anything - even when all tests pass, there is only a single log output.

But *each handlers should still be called 🤔

Janpot commented 1 week ago

Worth mentioning this mode also breaks @testing-library/dom, see https://github.com/testing-library/dom-testing-library/issues/1337.

AriPerkkio commented 1 week ago

How is that related? You need isolation for tests that are manipulating the DOM.

That setup is not using *each hooks at all. The problem in this issue to me seems to be that afterEach is treated as afterAll, same for before* hooks.

Janpot commented 1 week ago

That setup is not using *each hooks at all.

It's (should be) called internally by @testing-library/react.

You need isolation for tests that are manipulating the DOM.

Why? Shouldn't it work if you disable file parallelism? Isn't that essentially how e.g. mocha works?

AriPerkkio commented 1 week ago

this mode also breaks @testing-library/dom

It's (should be) called internally by @testing-library/react.

Oh right, you are using @testing-library/react, not @testing-library/dom. The issue is reported in wrong Github project.

See https://github.com/vitest-dev/vitest/issues/1430 for the root cause there.

Janpot commented 1 week ago

The issue is reported in wrong Github project.

They don't accept issues related to the dom query API in the @testing-library/react repo 🤷

hi-ogawa commented 2 days ago

Surprisingly, the behavior is different if you have setupFiles: ['./setup.ts'] instead of setupFiles: ['./setup'] https://stackblitz.com/edit/vitest-dev-vitest-hps2bc?file=vite.config.ts

This isn't probably intended. Module cache invalidation here needs to consider resolution https://github.com/vitest-dev/vitest/blob/5e6de2742f329b7e65fe3dff4827b5134d777009/packages/vitest/src/runtime/runners/test.ts#L35-L40

For the issue with testing-library's cleanup hooks https://github.com/vitest-dev/vitest/issues/1430, that one might be already fixed by https://github.com/vitest-dev/vitest/pull/6689

Janpot commented 2 days ago

btw, https://github.com/testing-library/dom-testing-library/issues/1337 is not caused by the @testing-library/react cleanup hooks, I'm seeing the same behavior without react. I updated the reproduction.

vitest-dev / vitest