Skip to content

Async hook testing #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ab18556 opened this issue Mar 9, 2019 · 14 comments · Fixed by #11
Closed

Async hook testing #10

ab18556 opened this issue Mar 9, 2019 · 14 comments · Fixed by #11
Labels
enhancement New feature or request question Further information is requested

Comments

@ab18556
Copy link
Contributor

ab18556 commented Mar 9, 2019

Hi, I am wondering how I can test hooks with async behavior. I would like to create hooks that send a request to my api to rerender my component with some fresh data.

I tryied a couple of things and I couldn't find a way to test it. Is it supported by this library?

@ab18556 ab18556 added the question Further information is requested label Mar 9, 2019
@mpeyper
Copy link
Member

mpeyper commented Mar 9, 2019

We don't actively prevent testing async behaviour, but your hook would need to make the promise/callback/whatever available so that your test runner can be aware of it and so you can delay your assertions.

Can you please share an example of the hook you are trying to test and the rest that is failing and I'll see if I can help get it running green for you.

@pgangwani
Copy link

@ab18556
Copy link
Contributor Author

ab18556 commented Mar 10, 2019

useFetch is actually a good example of what I am trying to achieve. I don't have an example to share yet, but I will try to provide one as soon as I can.

@mpeyper
Copy link
Member

mpeyper commented Mar 11, 2019

So using useFetch as an example, I came up with:

import fetch from 'node-fetch'
import { useFetch } from 'react-async'
import { renderHook, cleanup } from 'react-hooks-testing-library'

window.fetch = fetch

describe('useFetch tests', () => {
  afterEach(cleanup)

  test('should use fetch', (done) => {
    const { result, rerender } = renderHook(() =>
      useFetch(
        'https://swapi.co/api/people/1',
        { Accept: 'application/json' },
        {
          onResolve: () => {
            rerender()
            expect(result.current.isLoading).toBe(false)
            done()
          },
          onReject: () => {
            done.fail(new Error('request failed'))
          }
        }
      )
    )

    expect(result.current.isLoading).toBe(true)
  })
})

This test passes, however, the "not wrapped in act" warning is displayed:

 PASS  test/useFetch.test.js
  ● Console

    console.error node_modules/react-dom/cjs/react-dom.development.js:506
      Warning: An update to TestHook inside a test was not wrapped in act(...).
      
      When testing, code that causes React state updates should be wrapped into act(...):
      
      act(() => {
        /* fire events that update state */
      });
      /* assert on the output */
      
      This ensures that you're testing the behavior the user would see in the browser. Learn more at https://fb.me/react-wrap-tests-with-act
          in TestHook

I believe I have tracked the warning down to the setState that eventually gets called when the promise resolves.

To be honest, I'm not sure how we are supposed to wrap the promise resolution in an act call. I tried a few of the suggestions in @threepointone's examples, but could not prevent the message from showing, even using fetch-mock to mock the API. Perhaps a different API on the hook would allow some of them to work, but I'm not sure.

@mpeyper
Copy link
Member

mpeyper commented Mar 11, 2019

I'm just wondering if this could be improved with a result.onUpdate callback or something?

import fetch from 'node-fetch'
import { useFetch } from 'react-async'
import { renderHook, cleanup } from 'react-hooks-testing-library'

window.fetch = fetch

describe('useFetch tests', () => {
  afterEach(cleanup)

  test('should use fetch', (done) => {
    const { result, rerender } = renderHook(() =>
      useFetch('https://swapi.co/api/people/1', { Accept: 'application/json' })
    )

    expect(result.current.isLoading).toBe(true)

    result.onUpdate(() => {
      expect(result.current.isLoading).toBe(false)
      done()
    })
  })
})

or perhaps Promise based?

import fetch from 'node-fetch'
import { useFetch } from 'react-async'
import { renderHook, cleanup } from 'react-hooks-testing-library'

window.fetch = fetch

describe('useFetch tests', () => {
  afterEach(cleanup)

  test('should use fetch', () => {
    const { result, rerender } = renderHook(() =>
      useFetch('https://swapi.co/api/people/1', { Accept: 'application/json' })
    )

    expect(result.current.isLoading).toBe(true)

    return result.waitForUpdate().then(() => {
      expect(result.current.isLoading).toBe(false)
    })
  })
})

Thoughts?

@ab18556
Copy link
Contributor Author

ab18556 commented Mar 11, 2019

The example with the callbacks, even if I hate callbacks, would actually allow me to use this library, so I am happy with it.

It would be great to have a way to watchForUpdate. This would allow us to use async/await and we wouldn't be forced to use callbacks. I prefer this method, but it might be harder to implement.

About the warning that is triggered, there is already a PR allowing to pass async stuff to act that should fix this issue on react so I wouldn't worry about it too much. facebook/react#14853

@ab18556
Copy link
Contributor Author

ab18556 commented Mar 11, 2019

An example that should be harder to test:
https://codesandbox.io/s/7ok779l6n6?fontsize=14

@mpeyper
Copy link
Member

mpeyper commented Mar 11, 2019

I'm also in favour of Promise over callbacks... if nothing else, it removes the ambiguity if the value changes twice.

Ok, let's see if we cant get something like this working

test('should use fetch', async () => {
  const { result } = renderHook(() =>
    useFetch('https://swapi.co/api/people/1', { Accept: 'application/json' })
  )

  expect(result.current.isLoading).toBe(true)

  await result.waitForUpdate()

  expect(result.current.isLoading).toBe(false)
})

I'm also thought might read a bit better if it was like

test('should use fetch', async () => {
  const { result, nextUpdate } = renderHook(() =>
    useFetch('https://swapi.co/api/people/1', { Accept: 'application/json' })
  )

  expect(result.current.isLoading).toBe(true)

  await nextUpdate()

  // may need to call `rerender` before `result.current` can updates, or perhaps `nextUpdate` can take care of that?
  expect(result.current.isLoading).toBe(false)
})

I've got no idea how much effort this is, but if anyone want to try to implement it, I'm happy to help, otherwise I'll take a look myself next time I have a free evening.

@mpeyper mpeyper added the enhancement New feature or request label Mar 11, 2019
@mpeyper
Copy link
Member

mpeyper commented Mar 12, 2019

Anyone interested should jump on that PR and give me some feedback.

@ab18556
Copy link
Contributor Author

ab18556 commented Mar 12, 2019

I will not be able to test it until a couple of days, but it seems to be exactly what I needed ;)

@ab18556
Copy link
Contributor Author

ab18556 commented Mar 12, 2019

Actually I suggested some changes

@ab18556 ab18556 closed this as completed Mar 12, 2019
@ab18556 ab18556 reopened this Mar 12, 2019
@ab18556
Copy link
Contributor Author

ab18556 commented Mar 12, 2019

Sorry I didn't meant to close the issue, but to cancel my comment. I'm a noob ;)

mpeyper added a commit that referenced this issue Mar 14, 2019
Add `waitForNextUpdate` promise for testing async updates

Resolves #10
@jnelken
Copy link

jnelken commented Apr 12, 2021

For anyone else stumbling across this issue: https://testing-library.com/docs/dom-testing-library/api-async/#waitfor

@raaaahman
Copy link

For anyone else stumbling across this issue: https://testing-library.com/docs/dom-testing-library/api-async/#waitfor

This solved my issue (mind that this is a custom implementation of useFetch):

import "@testing-library/jest-dom"
import { renderHook, waitFor } from "@testing-library/react"
import { rest } from "msw"
import { setupServer } from "msw/node"

import useFetch from "./useFetch"

const pokemonEndpoint = jest.fn()

const handlers = [
    rest.get("https://pokeapi.co/api/v2/pokemon", pokemonEndpoint)
]

const server = setupServer(...handlers)

describe("The useFetch hook", () => {
    beforeAll(() => server.listen())

    afterAll(() => server.close())

    it("automatically fetches the given URL when called", async () => {
        const { result } = renderHook(() => useFetch("https://pokeapi.co/api/v2/pokemon"))
    
        await waitFor(() => expect(result.current.status).toBe("fetching"))
        expect(pokemonEndpoint).toHaveBeenCalled()
    })
})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants