React Apollo Testing #164
React Apollo Testing #164
Conversation
There was a problem hiding this comment.
This is awesome @JakeDawkins! 🎉 I've added a few small comments inline - I can't wait to get this in place!
| }); | ||
| ``` | ||
|
|
||
| This example shows how to define the mocked response for a query. The `mocks` array takes objects with specific `request`s and their associated `result`s. In this example, when the provider receives a `GET_DOG_QUERY` witht the specified variables, it returns the object under the `result` key. |
|
|
||
| ### addTypename | ||
|
|
||
| You may notice the prop being passed to the MockedProvider called `addTypename`. The reason this is here is because of how Apollo Client normally works. When a request is made with Apollo Client normally, it adds a `__typename` field to every object type requested. This is to make sure that Apollo Client's cache knows how to normalize and store the response. When we're making our mocks, though, we're importing the raw queries _without typenames_ from the component files. |
There was a problem hiding this comment.
Maybe backtick MockedProvider here to be consistent.
| </MockedProvider>, | ||
| ); | ||
|
|
||
| await wait(0); // wait for response |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Just to add - I saw your comment about ignoring it for now, but I was too excited! 🙂 |
There was a problem hiding this comment.
Awesome work @JakeDawkins, thanks for writing this! I added some comments on what I have run into when testing components, hopefully this will help out.
Due to time constraints I have not been able to go through it thoroughly but I will do so soon!
|
|
||
| If we don't disable the adding of typenames to queries, our imported query won't match the query actually being run by the component during our tests. | ||
|
|
||
| > In short, If you don't have `__typename`s in your queries, pass `addTypename={false}` to your `MockedResolver`s. |
There was a problem hiding this comment.
I am wondering what the addTypename prop is actually used for. When testing I simply want to link some data to the imported query without having to deal with typenames like so:
import { Query } from “./some_component”;
const mocks = [{
request: {
query: Query
},
result: someData
}]
renderer.create(
<MockedProvider mocks={mocks}>
<Component />
</MockedProvider>
);I actually created a PR in the past where I simplified the MockedProvider (see #1882) and removed the addTypename prop. Back then I created this PR because I was constantly running into the issue that the MockedProvider was not returning data for my query (this was because addTypename was true by default).
Furthermore, I have a [PR]](apollographql/react-apollo#1995) open that sets the addTypename prop to false by default because this will allow developers to simply create mock data for their queries without having to worry about typenames.
Is there a specific use case that the addTypename prop enables? I have personally never had to use it in my projects and it has actually gotten in my way when testing apollo components. So I am wondering if more users will run into this.
Given that the MockedProvider has not been documented yet, I think that this may be a good moment to flesh out this API before it becomes public knowledge. Thoughts?
There was a problem hiding this comment.
I think I agree that false is a more sensible default.
I think we left it to true by default to make sure that it wouldn't be a breaking change to anyone using it currently. We hadn't documented it, but I know there are some people that we had pointed to this code initially, and may be using it in their projects.
Thoughts @jbaxleyiii?
There was a problem hiding this comment.
Oh ok, if people are actively using it then a breaking change may not be such a good idea. With the future release of react-apollo v3 then perhaps this behavior can be changed. Thanks for clarifying:)
@jbaxleyiii Do you know by any chance why the addTypename prop was actually added in the first place? I'm wondering what use cases it enables
There was a problem hiding this comment.
Yeah, v3 may be a good place to reconsider this.
Originally, the mocked provider acted like the normal provider did, and added typename by default. We added this prop to disable that
| </MockedProvider>, | ||
| ); | ||
|
|
||
| await wait(0); // wait for response |
There was a problem hiding this comment.
Does this always work consistently? As I remember correctly adding a timeout of 0 doesn't always guarantee that the loading state is gone. Therefore in the react-apollo examples I used wait-for-expect to test the components and this proved to work great:) What do you think?
import wait from "wait-for-expect";
it("tests", () => {
// This statement will wait until loading UI disappears before continuing
await wait(() => {
isNotLoading();
})
// Data is present
});There was a problem hiding this comment.
Oh, interesting. I'll have to check out those examples and see. I never saw the loading state not be gone (although these examples are very simple).
Otherwise, I like wait-for-expect. I'll check it out and see if it's a simple enough solution!
There was a problem hiding this comment.
Great, thanks a lot for taking it into consideration:)
| }); | ||
| ``` | ||
|
|
||
| Testing UI components isn't a simple issue, but hopefully with these tools, you can feel more confident when testing components dependent on data. |
There was a problem hiding this comment.
Should we mention that the error state can be tested by passing an error to the mock similar to the Query example?
There was a problem hiding this comment.
Ideally, yeah! I wrote a test for doing that in the codesandbox example I linked to at the bottom.
I commented it out because for some reason when testing, it would throw the error on the preview window (even though the component is never mounted).
I didn't have the time to figure out what was going on there, unfortunately
|
@excitement-engineer @hwillson I've moved this PR to apollographql/apollo-client#3572 and added notes for the things still left to fix/discuss. Also, sorry @excitement-engineer was right in opening the PR there originally 🙈 |
|
I realize there has been some punting back and forth going on here, and it's partially my fault for not acting on this a bit sooner (also, not at all @JakeDawkins' fault), but after following some other conversations, and with the future of the more-general (as contrasted with the separate server and client docs) That's not to say that the back-and-forth isn't understandable though since we haven't historically had a home for content which "spans the stack" and this PR's content (testing) has two faces: a client and a server side. While it might make sense to co-locate the respective sections within the (separate) server and client sections of the documentation, this separation creates three different entry-points for subjective content which would be more easily found within the (new) Guides section we now have on If we don't put this opinionated content into a single place, we'll also make it difficult (for ourselves and others) to decide where new content should land since a guide might start as client-only and graduate into a more full-stack recommendation. This would necessitate the relocation of this content somewhat organically and unexpectedly, which would be less than ideal. We've already planted the seeds for having implementation guides outside of documentation, and by putting this testing content in the new "Guides" section of Of course, we can (and should!) provide links to this content from the server and client guides, but having the content live there doesn't seem manageable long-term. As we get more and more content which has similar client/server division that "Testing" has, perhaps we can accommodate it better with some navigational element (Server/Client tabs? 🤷♂️ ), but for now, I think we should be comfortable having a single "Testing" guide with "Client" and "Server" sections, or if the content is prohibitively long, separate "Testing: Client" and "Testing: Server" guides. I think I've outlined some very compelling reasons above, but of course, I'm happy to discuss this further. Ultimately, my goal here is to optimize and streamline the documentation experience. I can't wait to get this excellent content into a new, highly-visible home, and I'm convinced that the Guide section on |
|
Totally agree with you @abernix 👍 |
|
@JakeDawkins @hwillson @peggyrayzis @abernix Are we considering any other change or addition to this content. If not, then we can merge it in ASAP! And then continue iteration on it as new recommendations or examples or updates come in. |
|
Added a note about potentially needing to wait longer than |
| @@ -0,0 +1,340 @@ | |||
| --- | |||
| title: Testing React Apollo | |||
There was a problem hiding this comment.
We don't currently use the words "React Apollo" as a brand name in that order. Maybe "testing react with apollo" or "testing your frontend"?
| @@ -0,0 +1,340 @@ | |||
| --- | |||
| title: Testing React with Apollo | |||
There was a problem hiding this comment.
This makes it sound like Apollo is a testing tool.
This is editorial work for #164. The changes I've introduced here try to remove the subject of "you". This is a pattern which we've tried to perpetuate in other portions of the documentation. In my experience this has generally made the content more succinct (i.e. shorter), but also simplifies translation in some languages and makes the content more easily digestible for those who might be less proficient by removing the extra, unnecessary subject. I have some concerns about using `awwait`, since it's basically a full npm module that does: await (new Promise(r => setTimeout(r, 0))) I think we can probably accomplish the same testing without this module by using `Promise`-aware test tooling which waits for fulfillment of the `Promise`s before testing. We can iterate on this later though, and this is a great start. Oh, I also made a number of changes of the term "resolves" to "is fulfilled" since the term "resolves" would indicate that the execution of the `Promise` didn't chain didn't result in an error - which is entirely possible.
There was a problem hiding this comment.
My requested editorial changes are in #169, which will merge into this if accepted.
I've also renamed the name of the markdown file (which represents the html filename in the URL) to match the new title of the page.
Overall though, this looks great!
This is editorial work for #164. It also targets that PR and will merge into it. The changes I've introduced here try to remove the subject of "you". This is a pattern which we've tried to perpetuate in other portions of the documentation. In my experience this has generally made the content more succinct (i.e. shorter), but also simplifies translation in some languages and makes the content more easily digestible for those who might be less proficient by removing the extra, unnecessary subject. I have some concerns about using the [`awwait`](https://npm.im/awwait), since it's basically a full npm module that does: await (new Promise(r => setTimeout(r, 0))) I think we can probably accomplish the same testing without this module by using `Promise`-aware test tooling which waits for fulfillment of the `Promise`s before testing. We can iterate on this later though, and this is a great start. Oh, I also made a number of changes of the term "resolves" to "is fulfilled" since the term "resolves" would indicate that the execution of the `Promise` didn't chain didn't result in an error - which is entirely possible.
The name was changed in 676f65b.
|
Please merge @abernix |
Testing React Apollo
This is a guide specifically for testing React Apollo. Based off of @excitement-engineer's (great) original work in this PR.
A few notes:
guideshere, 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.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.react-test-rendererinstead of Enzyme or anything else, just to keep the toolset as simple as possible.