mswjs / msw

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

No response when using `jest.useFakeTimers()` with axios and msw #966

Closed asode closed 2 years ago

asode commented 2 years ago

Describe the bug

My app uses Svelte with Axios to fetch data from a REST API endpoint. I noticed that Axios didn't sometimes wait long enough for the request and it timed out. Now I increased the Axios' timeout to 12000 ms and wanted to verify it with tests that it actually waits for long enough for the request.

So I mocked the API with msw for the basic test that it actually works. No problems there. And when I add the delay to my msw mocked API, it works too, I just need to increase the test's timeout value so it actually waits long enough.

Which brings me to my point: I don't really want to wait for that long to verify that the Axios' 12 second timeout actually works. So here's where I wanted to use the jest's useFaketimers method but I just can't get it to work. Even when trying to advance the mocked timer with every command I found in jest, the response never comes.

Environment

Please also provide your browser version.

Not really browser related but sure:

To Reproduce

Steps to reproduce the behavior:

  1. use axios to fetch data from a REST API endpoint
  2. mock the API with msw
  3. add a delay to the msw mocked API
  4. use jest.useFakeTimers() to mock timers
  5. use jest.advanceTimersByTime() to pass the time forward until there should be an answer from the delayed API request
  6. the test times out with an error like Unable to find an element with the text or similar

Expected behavior

jest successfully mocks timers and skips the time so the test doesn't timeout.

Reproduction repo

https://github.com/asode/msw-usefaketimers

Failing test code: https://github.com/asode/msw-usefaketimers/blob/main/src/App.test.ts#L43

Failing test run: https://github.com/asode/msw-usefaketimers/runs/4106445180#step:4:27

AhmedBHameed commented 2 years ago

I confirm the issue too. However, I'm not sure if jest.advanceTimersByTime() works with await promise. The issue here is that await will force the code to wait till we reach for an answer.

So the question, where should we put jest.advanceTimersByTime() to pass the time or even how to make a fake delay to make msw wait for 12 seconds but with a fake 1 second for instance.

Can you share the code of how you start testing axios with msw ?

asode commented 2 years ago

@AhmedBHameed Thank you for confirming the issue. I did share the reproduction repo but did you have something else in mind?

asode commented 2 years ago

Curious thing I noticed by accident. It seems that the axios timeout setting is completely ignored even when not using jest.useFakeTimers(). This is not the case in production, just with msw it seems.

I tried the following with my reproduction repo:

// axios settings
const myAPI = axios.create({
  baseURL: "http://localhost/api/",
  timeout: 1,
});
test("Should wait if the listing takes a while", async () => {
  serverDelay = 10000;
  render(App);
  await screen.findByText("Test Project 1", {}, { timeout: 11000 });
}, 11000);

And the test passes even though axios should timeout and throw an error after 1 ms (instantly).

AhmedBHameed commented 2 years ago

Hi @asode

I had some free time to help you with your issue. I did some changes in your code which make it works perfectly with delay() function of msw. However, I still confirm that mocking timers with jest.useFakeTimers() does not work which also the same case in my node.js application.

Regarding to your code, take a look to the followings:

App.svelte

<script lang="ts">
  import axios, { AxiosResponse } from "axios";

  const myAPI = axios.create({
    baseURL: "http://localhost/api/",
    timeout: 12000,
  });

  type axiosData = {
    result: [];
  };
  let resultList:  any;

  resultList = myAPI.get<axiosData>("projects");
  //   .then((res) => {
  //   console.log("result: ", res.data);
  //   if (res.data) resultList = res.data.result;
  // });
</script>

{#await resultList}
    <p>...waiting</p>
{:then data}
    <p>{data.data.result[0].id}</p>
{:catch error}
    <p style="color: red">{error.message}</p>
{/await}

<!-- {#each resultList as project}
  <dir>
    <h2>{project.name}</h2>
  </dir>
{/each} -->

App.test.ts

import App from "./App.svelte";
import "@testing-library/jest-dom";
import { render, screen } from "@testing-library/svelte";
import { setupServer } from "msw/node";
import { rest } from "msw";

const returnData = [
  {
    id: "testproject1",
    name: "Test Project 1",
  },
];

const server = setupServer(
  rest.get("*/projects", (req, res, ctx) => {
    return res(
      ctx.json({
        result: returnData,
      }),
      ctx.delay(4000)
    );
  })
);

beforeAll(() => {
  // jest.useFakeTimers();    // <<==== I confirm this still does not work or maybe I miss the right configuration?
  server.listen();
});
afterEach(() => {
  server.resetHandlers();
  jest.clearAllMocks(); // reset called count
});
afterAll(() => {
  // jest.useRealTimers();
  server.close();
});

describe("<App />", () => {
  test.only("Should render projects", async () => {
    render(App);
    // jest.advanceTimersByTime(2000);
    await screen.findByText("testproject1", undefined, {timeout: 4050});
  });

  test("Should wait if the listing takes a while", async () => {
    render(App);
    await screen.findByText("Test Project 1");
  });
});

I also added the following script just for easy testing

  "scripts": {
    "test": "jest --watchAll --detectOpenHandles"
  },

Please note if you use a delay more than 5 seconds, jest will throw the following error

thrown: "Exceeded timeout of 5000 ms for a test. Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

Hope that answer will help you. Good luck.

kettanaito commented 2 years ago

For posterity, the pull request that introduces the custom timers module instead of setTimeout: https://github.com/mswjs/msw/pull/243

kettanaito commented 2 years ago

As we're using timers instead, I'd expect that Jest's fake timers had no effect on the library. We need to take a deeper dive into how jest.useFakeTimers() actually works, what modules it stubs, etc.

I believe that historically, the main purpose to use the custom timers package was to opt-out from using setTimeout directly. MSW wraps all mocked responses in setTimeout and uses either the custom delay duration or 0 when no delay is intended. It's because of that zero that we still needed to allow immediate mocked responses when using fake times in Jest.

To give you a better perspective, this code will never resolve if we use setTimeout directly:

it('some test', async () => {
  server.use(
    rest.get('https://example.com', (req, res, ctx) => res(ctx.text('hello')))
  )

  jest.useFakeTimers()

  // This promise will never resolve, as nothing has advanced the fake timers,
  // so the "setTimeout(sendMockedResponse, 0)" is never called. 
  const res = await fetch('https://example.com')
})

We may omit the setTimeout usage for no-delay scenarios, which should allow us to both respect the fake timers (and demand you advance them to support delayed responses) and make immediate responses happen correctly while using fake timers.

I'd be thankful if somebody could give that approach a try. Here's the related logic that uses setTimeout:

https://github.com/mswjs/msw/blob/f6fbd6c94d86d54ee13d20f0128589332cbddcbf/src/utils/handleRequest.ts#L122-L130

  1. Remove the timers package from rollup.config.js.
  2. Use setTimeout only when the custom delay has been sent.
mc0 commented 2 years ago

@kettanaito

We may omit the setTimeout usage for no-delay scenarios, which should allow us to both respect the fake timers (and demand you advance them to support delayed responses) and make immediate responses happen correctly while using fake timers.

The only downside I see with this is that the response handling will get tied up into the same stack as the request. It could have unintended consequences (error handling, order of events).

If the goal is to remove the special case timers package, then perhaps documentation of the issue and a workaround could be sufficient to that end for users.

For example, here is how to do the example provided with Jest:

import * as timers from 'timers';
...
it('some test', async () => {
  server.use(
    rest.get('https://example.com', (req, res, ctx) => res(ctx.text('hello')))
  )

  jest.useFakeTimers()

  timers.setTimeout(() => jest.runOnlyPendingTimers(), 1)

  // This promise will be resolved by the runOnlyPendingTimers called in ~1ms.
  const res = await fetch('https://example.com')
})

This puts the onus on the test writer, but provides the flexibility to control when time passes:

  jest.useFakeTimers()
  const p = fetch('https://example.com')
  // rerender, click, etc.
  jest.advanceTimersByTime(1000)
  const res = await p
AhmedBHameed commented 2 years ago

@mc0

Your approach wrecked my testing time to the half. I confirm that your approach is working perfectly with faking msw setTimeout.

faking timers

Thanks for the tip.

mc0 commented 2 years ago

@kettanaito I had a different idea for solving this that aligns a bit with some of the feedback I've seen. Basically, allow setting a clock global variable name that will be used in place of window. While it'd be great to set an actual object, I think the way MSW works in the browser makes this difficult/impossible.

For example:

res(ctx.delay(100), ctx.clock('customClock'))
res(ctx.clock('customClock'))
res(ctx.clock())

This works with both the service worker and the node server. The global could be created like so:

import * as timers from 'timers'

global.customClock = timers
JBudny commented 2 years ago

Hi, I feel like my issue may be related to this so I post it here and maybe it wil help someone.

I had a problem while testing the React Native TextInput component with debouncing and redux-toolkit useQuery hook (for data fetching) and I've managed to test it this way. It seems kind of hacky though

I upgraded jest to the latest "^27.4.2" version and used it as below.

beforeEach(() => {
    jest.useFakeTimers('legacy')
})

afterEach(() => {
    jest.useRealTimers()
})

test('Search component should display fetched data', async () => {
    const { getByPlaceholderText, findByText } = renderWithProviders(
        <Search />
    )
    const textInput = getByPlaceholderText('Type to search')
    // I had to use act on this fireEvent because jest was asking for it
    // and jest.advanceTimersByTime and similar utilities were not helping.
    // Querying a fireEvent result with await waitFor was also not helping.
    act(() => {
        fireEvent.changeText(textInput, 'Some text')
    })
    // this expects works even without await, but not with 
    // jest.advanceTimersByTime and getByText
    expect(await findByText('some result')).toBeDefined()
})
snovos commented 2 years ago

So in my case setTimeout gets ignored and loadData never gets called Any ideas?

  const [loadData, { called, data }] = useLazyQuery(DATA_QUERY, {
    fetchPolicy: 'cache-and-network',
    pollInterval: 50
  });

  useEffect(() => {
    const loadingDataTimer:= setTimeout(
      () => {
        loadData();
      },
      1000,
    );

    // cleanup on unmount
    return () => {
      clearTimeout(loadingDataTimer);
    };
  }, []);

Test looks like this


it('should have valid readable location', async () => {
 jest.useFakeTimers();
    renderPage();

    act(() => {
     jest.advanceTimersByTime(2000);
     });
    const target = await screen.findByTestId('readableLocation', undefined, { timeout: 3000 });
    expect(target.textContent).toBe('San Francisco');
  });

Tried different options with jest.fakeTimers and jest.advanceTimersByTime but no luck loadData never gets called

kettanaito commented 2 years ago

@snovos from the looks of it, I don't see how MSW would control whether the loadData would be called. I suggest looking closely at what's happening, logging out things, put a debugger. I believe this issue is about responses not being received because MSW is wrapping every response resolver in setTimeout(stuff, 0).

kettanaito commented 2 years ago

Released: v0.42.3 πŸŽ‰

This has been released in v0.42.3!

Make sure to always update to the latest version (npm i msw@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.

kettanaito commented 2 years ago

Released: v0.43.0 πŸŽ‰

This has been released in v0.43.0!

Make sure to always update to the latest version (npm i msw@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.

karol1500 commented 1 year ago

snovos Did you manage to find a solution to the problem? I'm experiencing the same and no luck yet.

karin-on commented 11 months ago

@karol1500 I don't know, if you still need it, but I just fixed it with Selective Faking in Jest.

my test:

test('some title', async () => {
  jest.useFakeTimers({ now: new Date(2023, 9, 15), doNotFake: ['setTimeout'] });

  const user = userEvent.setup({
    advanceTimers: () => jest.runOnlyPendingTimers(),
  });

  // test logic and assertions

  jest.useRealTimers();
});

The key here is doNotFake: ['setTimeout'].

I'm using: jest: 29.7.0 jest-environment-jsdom: 29.7.0 msw: 1.3.2 @testing-library/jest-dom: 6.1.4 @testing-library/react: 14.0.0 @testing-library/user-event: 14.5.1