refactor: use TestSWRConfig for error-test #1404
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 15490e8:
|
| createResponse(new Error('error!')) | ||
| ) | ||
| if (error) return <div>{error.message}</div> | ||
| return <div>hello, {data}</div> | ||
| } | ||
|
|
||
| render(<Page />) | ||
| render( | ||
| <TestSWRConfig> |
There was a problem hiding this comment.
I dont think we should replace this with wrapping context provider. Most of the tests are also to make sure they're good to run without context provider. We shouldn't replace unique key with a standalone cache in context.
There was a problem hiding this comment.
Thank you for your feedback! That's a good point.
Is the point solved by adding tests with a global cache or should we confirm to pass existing test cases with a global cache?
There was a problem hiding this comment.
Is the point solved by adding tests with a global cache or should we confirm to pass existing test cases with a global cache?
Yeah that's better 👍
|
Another alternative way is to clear the global cache in the jest's setup file. |
clearing cache for each test case sgtm. |
|
Updating branch with latest change should be able to unblock the checks 🙏 |
|
I like the idea of creating a wrapper/helper util to make writing tests easier! |
|
Makes sense to merge this for now! Thank you! |
|
Thank you! |
This is a PR to propose to make unit tests more reliable.
Currently, most of the tests depend on a global cache store, which means that those tests share the same cache. This is fragile because the tests might start failing when we use the same key in between them.
SWR has
createKeyas a utility function for testing to generate a random key, which would solve the problem, but I think running tests on independent environments is important and makes tests more reliable.To fix this, I've added
<TestSWRConfig />, which creates its own cache provider, and wrapped target components in it. We don't have to care about duplicating keys while using<TestSWRConfig />.I used the name because I expect to be added more configurations for testing in the component. More explicit names like
<SWRCacheBoundary />might be better.What do you think? If this looks good to you, I'll refactor other tests as well.
I've also come up with wrapping the render function that testing-library provides, but I didn't adopt the approach because it's less clear what the function does and introduces additional abstraction. It's also hard to cover tests for concurrent rendering because it doesn't use the
testing-library's render function.