About the challenges of afterEach in interactive test tools

[kasperpeulen]

Hello! Kasper from Storybook here.

At Storybook, we’re exploring adding an afterEach hook that runs after each story test execution, similar to how afterEach works in tools like Vitest. However, we’ve encountered some fundamental challenges with incorporating afterEach into an interactive testing tool.

I’m reaching out because I suspect you’ve faced similar issues and thought it would be valuable to align our approaches.

Using afterEach for Cleanup Impedes Interaction

In an interactive testing environment, performing cleanup immediately after test execution can render the test uninteractive. For example, Vitest’s fork of @testing-library/react avoids DOM cleanup in afterEach and instead performs it in beforeEach:
https://github.com/vitest-dev/vitest-browser-react

Unlike @testing-library/react, vitest-browser-react performs cleanup of the component before the test begins, allowing you to see the rendered result in your UI.

But this problem occurs every time cleanup is done in afterEach. For another example, MSW recommends resetting all handlers in afterEach.

import { beforeAll, afterEach, afterAll } from 'vitest'
import { server } from './src/mocks/node'
 
beforeAll(() => server.listen())
afterEach(() => server.resetHandlers())
afterAll(() => server.close())

In Vitest Browser UI, that would mean that interacting with the test won't hit the mocked endpoints anymore. This can be solved, by doing the reset handlers in a beforeEach.

beforeEach(() => server.resetHandlers())

Delaying the execution breaks if assertions are used in afterEach

At Storybook, we haven’t yet adopted afterEach but recently introduced beforeEach, which supports a return callback for cleanup:

const meta: Meta<typeof Page> = {
  component: Page,
  // 👇 Set the value of Date for every story in the file
  async beforeEach() {
    MockDate.set('2024-02-14');

    // 👇 Reset the Date after each story
    return () => {
      MockDate.reset();
    };
  },
};

Instead of running the cleanup callback immediately after the test, we defer it until switching stories. This avoids the interaction issue but creates a new problem: it prevents post-test assertions.

For example, a user trying to assert that console.error wasn’t called in any story wouldn’t see the assertion in Storybook due to the delayed execution:

// preview.ts is a place to add global beforeEach in storybook
const preview = {
  async beforeEach() {
    const consoleError = spyOn(console, 'error');    
    
    return () => {
      await expect(consoleError).not.toHaveBeenCalled();      
    }
  }
}

While assertions in afterEach are sometimes discouraged, they can be valuable in scenarios like the one above. Other examples include automatically checking for accessibility violations after each test or capturing a DOM/visual snapshot.

Proposed path forward

So concluding, we think that there is need for 2 distinct stages at the end of testing lifecycle:

1. one for post-test assertions, that happens after the test execution
2. one for cleanup which happens when switching test/stories.

To address this, we are considering the following approach:

1. Introduce afterEach to Storybook, but recommend its use for post-test assertions rather than cleanup.
2. Encourage people to do global cleanup in beforeEach, following the pattern used by vitest-browser-react.
3. Maintain the current behavior in storybook of using the return callback from beforeEach for story-specific cleanup, executed when switching stories.

Alternatively, we could introduce a dedicated hook for post-test assertions instead of adopting afterEach. This would avoid the confusion of adding afterEach while advising against its use for cleanup.

We’re curious to hear your thoughts on the best path forward and whether there’s an opportunity to align our approaches.

[sheremet-va]

Interesting point. I think it might be unique to projects that have a visual interface where components can be viewed after the fact and cannot be generalized into a standardized built-in hook in a test runner like Vitest unless we can define it in another way ("a hook that should be called to release resources", for example).

vitest-browser-react (and other vitest-browser packages) also defines a hidden cleanup function that we call before removing the iframe, so I can see how this potential hook is different from just beforeEach and can be used in different situations.

I think if we want to have a general solution, it's best to recommend beforeEach. On the other hand, I am also not against introducing a hook like onCleanup or onReleaseResources to have an easy to understand interface for any situation that requires release of resources (component cleanup, closing server, clearing timeouts, etc. By default, it's just an alias to afterEach), but I am a bit afraid of introducing too much mental complexity with it (or does it actually reduce it? I'm not sure yet) - you need to know why you call a specific function instead of when to call a function - I would love to know your take on that.