mswjs / msw

Industry standard API mocking for JavaScript.
https://mswjs.io
MIT License
15.97k stars 519 forks source link

Component with fetch call during onMount fails due some race condition where server.listen() doesn't kick in time #2253

Closed sombriks closed 2 months ago

sombriks commented 2 months ago

Prerequisites

Environment check

Node.js version

v20.12.2

Reproduction repository

https://github.com/sombriks/simple-vitest/blob/main/my-vue-app/src/App.spec.ts

Reproduction steps

npm test

Current behavior

https://github.com/sombriks/simple-vitest/actions/runs/10548917976/job/29223355169#step:4:76

Expected behavior

unmount call should be fine, but due the mock issue it fails https://github.com/sombriks/simple-vitest/blob/main/my-vue-app/src/App.spec.ts#L22

if i add a waitFor() everything goes well:

    // Start server before all tests
    beforeAll(async () => {
        server.listen({onUnhandledRequest: 'error'})

        // give the mock server a time, without the wait it ends up in network error
        await vi.waitFor(()=> {
            render(App)
        })
    })
kettanaito commented 2 months ago

Hi, @sombriks.

Your test setup doesn't seem to be correct.

  1. You are missing server.listen()—the method that enables the request interception.
  2. If you inspect how your tests fail, they are all green, with the error happening after the test run is over. This is an indication that you are not awaiting something asynchronous in your test.

How to solve this

Here's a diff that provides correct MSW setup in your test:

--- a/my-vue-app/src/App.spec.ts
+++ b/my-vue-app/src/App.spec.ts
@@ -2,19 +2,20 @@ import {afterAll, beforeAll, describe, expect, it} from "vitest";
 import {render, RenderResult} from "@testing-library/vue";
 import userEvent from "@testing-library/user-event";
 import {http, HttpResponse} from 'msw'
-import {setupServer, SetupServerApi} from 'msw/node'
+import {setupServer} from 'msw/node'

 import App from "./App.vue";

 describe('app tests', () => {

+    const server = setupServer(
+        http.get('http://mock-url:3000/todos', () => HttpResponse.json([]))
+    )
+
     let component: RenderResult
-    let server: SetupServerApi

     beforeAll(() => {
-        server = setupServer(
-            http.get('http://mock-url:3000/todos', () => HttpResponse.json([]))
-        )
+        server.listen()
         component = render(App)
     })

Note that there's no need to keep the intermediary server and reassign it in beforeAll. In fact, that's a harmful practice. setupServer() is a setup function, like the name implies. It prepares the mocking without enabling it. You then use server.listen() and server.close() in the beforeAll() and afterAll() hooks respectively.

sombriks commented 2 months ago

Thanks a lot @kettanaito , i did the proposed changes and got able to narrow down the issue.

mswjs is working just prefect, the network error in previous builds where indeed the lack of a server.listen() call.

and the error in the afterAll hook is related to something in testing-library.

the working test case ended up like this:

import {afterAll, beforeAll, describe, expect, it, vi} from "vitest";
import {render, RenderResult} from "@testing-library/vue";
import userEvent from "@testing-library/user-event";
import {http, HttpResponse} from 'msw'
import {setupServer} from 'msw/node'

import App from "./App.vue";

describe('app tests', () => {

    let component: RenderResult
    const server = setupServer(
        http.get('http://mock-url:3000/todos', () => HttpResponse.json([
            {id: 777, description:'Walk the dog', done: true }
        ]))
    )

    beforeAll(() => {
        server.listen()
        component = render(App)
    })

    afterAll(() => {
        vi.waitFor(() => component.unmount())

        server.close()
    })

    it('should have import.meta.env.MODE to be "test"', () => {
        expect(import.meta.env.MODE).eq('test')
    })

    it('should have VITE_API_URL set to mock url', () => {
        expect(import.meta.env.VITE_API_URL).eq('http://mock-url:3000')
    })

    it('should click the count button', async () => {
        const button = component.getByText("count is 0")
        expect(button).toBeTruthy()
        const user = userEvent.setup()
        await user.click(button)
        expect(button.innerText).eq("count is 1")
    })

    it('should search todos', async () => {
        const search = component.getByPlaceholderText('Search')
        expect(search).toBeTruthy()
    })

    it('should have one todo - check id', async () => {
        const id = component.getByText('#777')
        expect(id).toBeTruthy()
    })

    it('should have one todo - check input', async () => {
        const field = component.getByDisplayValue('Walk the dog')
        expect(field).toBeTruthy()
    })
})