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

Documented API for MockedProvider #3309

Merged
merged 9 commits into from
Jun 14, 2018

Conversation

excitement-engineer
Copy link
Contributor

@excitement-engineer excitement-engineer commented Apr 12, 2018

I wrote some docs on how to test react-apollo components. This is not trivial to do so I decided to explain it step-by-step in a guide. Would love to get some feedback on this.

I am also curious if someone has found a better way to test react-apollo components that the method described in this guide. If so then I would be glad to update the docs.

Edit (jake): I took this PR and removed the testing guide from it and left the MockedProvider api docs

description: Testing react-apollo
---

React-apollo relies on [context](https://reactjs.org/docs/context.html) in order to pass the apollo-client instance through the react component tree. In addition, react-apollo makes network requests in order to fetch data. This behavior affects how you write tests for components that use react-apollo.
Copy link
Contributor

Choose a reason for hiding this comment

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

Stylistic thing: the docs refer to react-apollo as "React Apollo" and apollo-client as Apollo Client for the most part. This document should follow that.

"React" should be capitalized (i.e. not "react").

);
```

If we were to try and write a test for it then you will get an error that the `client` is missing in the context.
Copy link
Contributor

Choose a reason for hiding this comment

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

What "it" refers to is a bit unclear here; maybe "If we were to write a test for this component, we'd get an error that the client object is missing in the context."

});
```

In order to fix this we could wrap the component in an `<ApolloProvider />` and pass an instance of apollo-client to the `client` prop. However, this will cause our tests to run against an actual back-end which makes the tests very unpredictable for the following reasons:
Copy link
Contributor

Choose a reason for hiding this comment

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

Stylistic: replace "back-end" with "backend."

});
```

Despite having mocked the request the above test will still fail. Due to the asynchronous nature of react-apollo the loading state will be rendered instead of the dog name.
Copy link
Contributor

Choose a reason for hiding this comment

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

This part is a bit unclear to me. What is meant by "due to the asynchronous nature of React Apollo"?

@Poincare
Copy link
Contributor

Thanks so much for writing these docs. I had a few minor concerns that I outlined in the review above.

In addition, I'm unsure if the approach of separating presentational and data components is advisable in every situation.

@excitement-engineer
Copy link
Contributor Author

Hey @Poincare, thanks for the review. I really appreciate it!

I have been thinking for a while about how to test react components that use react-apollo and this guide sums up all the problems that I have run into and how I got to the final solution proposed in this PR. I removed the sentence that says: "we advise you separate the react-apollo components from presentational components" because this guide shows one way that tests can be written. There is probably no definitive way on how to best test the code, it all depends how far the developer wishes to go to ensure everything is tested. However, I do think that these principles of separating all the "noise" (i.e. react-apollo, network requests etc.) from the presentation part of react makes life a lot easier when testing and I hope that this is useful for many developers stuggling to write tests. If you have any other way of testing I would love to hear it. Always open to hearing suggestions;)

});
```

To test the component in isolation we need to mock out all the calls to the backend. This will make our tests predictable. React Apollo provides the `<MockedProvider />` component in order to do just that! `<MockedProvider />` allows you to specify the exact results that should be returned for a certain query using the `mocks` prop.

Choose a reason for hiding this comment

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

It would be good to mention where you import the MockedProvider from.

@excitement-engineer
Copy link
Contributor Author

I have found an easier way to test the components than is described in this guide. This new method will not require the developer to split the presentation and container components. This will address the rightful concerns you had about the current testing method @Poincare. I will update the docs soon and also add an example of testing mutations.

I have created two PRs in react-apollo that use this new technique of testing:

Testing queries
Testing mutations

@hwillson hwillson changed the title Wrote some docs on how to test react-apollo components [WIP] Wrote some docs on how to test react-apollo components Jun 1, 2018
@JakeDawkins
Copy link
Contributor

@excitement-engineer Great work! I'm going to close this PR and comment on the other 2 PRs you have open for queries and mutations!

@JakeDawkins JakeDawkins closed this Jun 5, 2018
@JakeDawkins JakeDawkins reopened this Jun 5, 2018
@JakeDawkins
Copy link
Contributor

@excitement-engineer I take it back. This still has some API docs that I think would be useful. Can you remove everything but the API documentation for MockedProvider

docs/_config.yml Outdated
@@ -41,6 +41,7 @@ sidebar_categories:
- advanced/subscriptions
- advanced/network-layer
- advanced/caching
- advanced/testing
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this :)

@@ -0,0 +1,310 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

This file can go away in favor of other docs, too

JakeDawkins added a commit to apollographql/apollo that referenced this pull request Jun 14, 2018
<!--**Pull Request Labels**

While not necessary, you can help organize our pull requests by labeling this issue when you open it.  To add a label automatically, simply [x] mark the appropriate box below:

- [ ] feature
- [ ] blocking
- [ ] docs

To add a label not listed above, simply place `/label another-label-name` on a line by itself.
-->

# Testing React Apollo 

This is a guide specifically for testing React Apollo. Based off of @excitement-engineer's (great) original work [in this PR](apollographql/apollo-client#3309).

**A few notes:**
- I'm keeping this separate from server testing, since combined I think they'll be too long. **I'm open to discussion on this, though**.
- I added the guide here rather than React Apollo's docs, since I feel like we have a good set of `guides` here, and would hate for this to be missing. I also feel like the view logic may transfer to other UI integrations, but I don't have a lot of insight into that.
- I went with using `wait(0)` instead of refactoring and counting renders like some other exmples do, for simplicity. I wanted to lower the barrier of entry for testing as much as possible.
- I used `react-test-renderer` instead of Enzyme or anything else, just to keep the toolset as simple as possible.
@JakeDawkins JakeDawkins changed the title [WIP] Wrote some docs on how to test react-apollo components Documented API for MockedProvider Jun 14, 2018
Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@hwillson hwillson merged commit 103e8f5 into apollographql:master Jun 14, 2018
@excitement-engineer
Copy link
Contributor Author

Thanks for ensuring that the MockedProvider docs got merged @JakeDawkins! I appreciate it:)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 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

5 participants