Skip to content
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

Updates testing docs to use Testing Library #9888

Merged

Conversation

alessbell
Copy link
Member

@alessbell alessbell commented Jul 7, 2022

This PR updates the Apollo Client testing docs by removing references to react-test-renderer in favor of React Testing Library, the UI testing library recommended in the official React docs. The goal is to improve the readability of our example tests while ensuring they pass, ideally without emitting any warnings.

These changes were the outcome of a quick look at #9887: copying + pasting one of the tests from our current docs results in a warning which may cause some confusion. Other tests outright fail when copy + pasted, like the one in Testing the Success State.

Let's look at the test from the "success state" section:

it('should render dog', async () => {
  const dogMock = {
    request: {
      query: GET_DOG_QUERY,
      variables: { name: 'Buck' },
    },
    result: {
      data: { dog: { id: 1, name: 'Buck', breed: 'poodle' } },
    },
  };

  const component = TestRenderer.create(
    <MockedProvider mocks={[dogMock]} addTypename={false}>
      <Dog name="Buck" />
    </MockedProvider>,
  );

  await new Promise(resolve => setTimeout(resolve, 0));

  const p = component.root.findByType('p');
  expect(p.children.join('')).toContain('Buck is a poodle');
});

When I run this test locally, I first see that the test failed:

Screen Shot 2022-07-06 at 9 35 36 PM

...followed by the act warning:

Screen Shot 2022-07-06 at 9 35 43 PM

The issue stems from having to await new Promise(resolve => setTimeout(resolve, 0));: since react-test-renderer is mostly used for snapshot testing and doesn't have async utilities for awaiting DOM changes, our docs state that you can await a zero-millisecond timeout before performing your checks. This delays the checks until the next "tick" of the event loop, which gives MockedProvider an opportunity to populate the mocked result.

I got the above test to pass by adding a second 0ms setTimeout immediately after the one in the test, and the act warning remained. Using promisified setTimeouts to manually cycle through ticks of the event loop is a fairly brittle and unintuitive way of testing dynamic/interactive behavior, especially given the async utilities available via React Testing Library.

The Testing Library Approach

The same test of the success state written with RTL looks like this:

it("should render dog", async () => {
  const dogMock = {
    request: {
      query: GET_DOG_QUERY,
      variables: { name: "Buck" },
    },
    result: {
      data: { dog: { id: 1, name: "Buck", breed: "poodle" } },
    },
  };
  render(
    <MockedProvider mocks={[dogMock]} addTypename={false}>
      <Dog name="Buck" />
    </MockedProvider>
  );
  expect(await screen.findByText("Loading...")).toBeInTheDocument();
  expect(await screen.findByText("Buck is a poodle")).toBeInTheDocument();
});

Instead of using setTimeout, Testing Library provides several findBy methods. From the Testing Library docs:

findBy queries work when you expect an element to appear but the change to the DOM might not happen immediately.

We can use the asynchronous screen.findByText to query the elements containing the loading message first, followed by the success message "Buck is a poodle" which appears after our query completes.

Under the hood, the findBy utils check for the element every N ms (default 50ms) up to a default timeout of 1000ms so the user doesn't have to be concerned with manually advancing the event loop.

Checklist:

  • Add a note about import '@testing-library/jest-dom'; (explain that it's usually present in a setup file, but include it in examples for copy-pastability)
  • Create an updated codesandbox at the bottom of testing.mdx (the current one crashes when you open it)
  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests

@alessbell alessbell force-pushed the issue-9887-example-tests-in-docs-result-in-act-warning branch from 2eb45bd to 2c80922 Compare July 7, 2022 22:07
Copy link
Contributor

@StephenBarlow StephenBarlow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good with small copy suggestions! I assume the pure removals of certain paragraphs are all appropriate (I'm not familiar enough with the content here to know which pieces might no longer be relevant due to the changes)

docs/source/development-testing/testing.mdx Outdated Show resolved Hide resolved
docs/source/development-testing/testing.mdx Outdated Show resolved Hide resolved
Co-authored-by: Stephen Barlow <stephen@apollographql.com>
<ExpansionPanel title="Click to expand 🐶">

```jsx
import TestRenderer from "react-test-renderer";
Copy link
Member Author

@alessbell alessbell Jul 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was combined with testing the loading state. If we don't, we'll see an act warning since the DOM updates again on success (it made sense to test these separately with react test renderer when we were manually advancing the event loop, much less so with RTL). The two tests are also mostly duplicates of each other with only a one line assertion difference.


Remember that the mock's value for `result` can also be a function, so you can perform arbitrary logic (like setting a boolean to indicate that the mutation completed) before returning its result.

[Testing error states](#testing-error-states) for mutations is identical to testing them for queries..
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix typo

await new Promise((resolve) => setTimeout(resolve, 0)); // wait for response

const tree = component.toJSON();
expect(tree.children).toContain("An error occurred");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before we were not rendering the error message in the component code so this test was also failing.

});
```

If your component performs complex calculations or includes delays in its render logic, you can increase the timeout's duration accordingly. You can also use a package like [`wait-for-expect`](https://npm.im/wait-for-expect) to delay until the render has occurred. The risk of using a package like this everywhere is that _every_ test might take up to five seconds to execute (or longer if the default timeout is increased).
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove all references to the setTimeout "trick" which is no longer necessary.

@alessbell alessbell force-pushed the issue-9887-example-tests-in-docs-result-in-act-warning branch from bd96aa5 to 9623ea3 Compare July 8, 2022 14:58
@alessbell alessbell force-pushed the issue-9887-example-tests-in-docs-result-in-act-warning branch from 9623ea3 to eac52bd Compare July 8, 2022 15:24
@alessbell alessbell merged commit 5a4b0ae into main Jul 8, 2022
@alessbell alessbell deleted the issue-9887-example-tests-in-docs-result-in-act-warning branch July 13, 2022 23:41
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants