I don't like doing this for every test because the terminal seems bloated. We have a suite.result and it's enough. I think reporters should handle this case if they don't distinguish between suite fails and test fails

I don't like doing this for every test because the terminal seems bloated

That sounds fair. To explain my perspective, the reason I went with this is that I thought beforeAll/afterAll failures should be more aligned with beforeEach/afterEach failures.

I experimented with current behavior on stackblitz:

https://stackblitz.com/edit/vitest-dev-vitest-m7bw4w?file=test%2Frepro.test.ts

For example, currently afterAll failure would keep tests as success and I thought that's not an intended behavior.

To avoid cluttering terminal with a single error appearing in all tests, I think I could do something like this while still flagging tests as "fail":

      t.result = {
        ...t.result,
        // flag as "fail"
        state: 'fail',
        // but don't have to copy errors
        // errors: [...t.result?.errors ?? [], ...errors],
      }

What do you think?

EDIT: actually I did this first, then I realized currently junit reporter checks task.result.errors to generate <failure> tags. So, if we went with this, we still require modifying around this code:

Okay, I'll also experiment with approach to fix only on junit reporter side.

To explain my perspective, the reason I went with this is that I thought beforeAll/afterAll failures should be more aligned with beforeEach/afterEach failures.

afterAll/beforeAll are not bound to any test, they are suite hooks. beforeEach/afterEach are test hooks. What I would expect is for tests to have skip state if beforeAll failed (because we don't run any test), and any state if afterAll failed (only the suite is marked as failed)

For example, currently afterAll failure would keep tests as success and I thought that's not an intended behavior.

afterAll fails only a suite which seems correct to me.

afterAll fails only a suite which seems correct to me.

This is consistent with mocha for example:

import { test, after } from 'mocha'

after(() => {
  throw new Error('after')
})

test('test', () => {})

Although in mocha afterEach doesn't fail the test while in Vitest it does.

If we are making it consistent, then 1) this is definitely a breaking change, 2) we need to decide what is consistent

afterAll/beforeAll are not bound to any test, they are suite hooks. beforeEach/afterEach are test hooks.

Totally good point. I didn't have that understanding.

Okay, I'll see what I can do with junit reporter only first. Probably I'll close this PR and create a new one.

Thanks again for the review!