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

test: fix all warnings in unit tests #872

Closed
wants to merge 1 commit into from

Conversation

koba04
Copy link
Collaborator

@koba04 koba04 commented Jan 8, 2021

When we run unit testing, we see many warnings in the console output even all tests have been passed. This makes it difficult to find problems from the console output.

https://travis-ci.org/github/vercel/swr/builds/753389280

This PR fixed all warnings in unit testing.
I could do more refactoring, but I tried to keep the current implementation and changes minimum.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 8, 2021

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 523566c:

Sandbox Source
SWR-Basic Configuration
SWR-States Configuration
SWR-Infinite Configuration

Copy link
Member

@shuding shuding left a comment

Choose a reason for hiding this comment

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

Thank you so much for fixing the warnings! cc @promer94 what do you think?

@koba04
Copy link
Collaborator Author

koba04 commented Jan 8, 2021

@shuding This is off-topic, but what do you think about migrating CI from TravisCI to GitHub Actions? I think GitHub Actions is faster than TravisCI, so I'll send a PR for that if it could be accepted.

@shuding
Copy link
Member

shuding commented Jan 8, 2021

@koba04 That's a great idea 👍

@promer94
Copy link
Collaborator

promer94 commented Jan 9, 2021

@koba04 Thank you for fixing the warning. But I don't know if it should be addressed in this way.

await 0
expect(....)

This pattern here feels to be a poor man's version of async queries provided by testing-library. And i have already did a refactor on our next branch and fixed all the warnings by using async queries. #779

I highly recommend two articles to you which are common-mistakes-with-react-testing-library and fix-the-not-wrapped-in-act-warning.

@koba04
Copy link
Collaborator Author

koba04 commented Jan 9, 2021

@promer94 Thank you! I didn't know that you have already fixed this on the next branch, so I'll close this.
I hope your fixes are applied to the master branch too.

I'm not familiar with testing-library, so these resources are helpful for me, thank you!
Should I send a PR to the next branch rather than the master branch if there is an improvement point for the test code?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants