Skip to content

Added global cleanup #16

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

Merged
merged 2 commits into from
Mar 22, 2019
Merged

Conversation

marquesm91
Copy link
Contributor

What:

Added global cleanup

Why:

To ease the cleanup tests using a global setup file

How:

Added in root cleanup-after-each.js to require cleanup function and call inside afterEach

Checklist:

  • Documentation updated
  • Tests
  • Typescript definitions updated
  • Ready to be merged
  • Added myself to contributors table

I don't add anything to Documentation because react-testing-library don't. But if it is necessary I can complete cleanup() section with an example

Closes #13

@codecov-io
Copy link

codecov-io commented Mar 19, 2019

Codecov Report

Merging #16 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #16   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines          18     18           
=====================================
  Hits           18     18

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 595b7f8...90eb946. Read the comment docs.

Copy link
Member

@mpeyper mpeyper left a comment

Choose a reason for hiding this comment

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

Can you please add a section to the README about the usage of this cleanup helper?

Other than that this is great, thanks!

@mpeyper
Copy link
Member

mpeyper commented Mar 21, 2019

I should have read the description better as you explicitly mentioned documentation.

I think your suggestion of adding it to the cleanup() section is good.

@marquesm91
Copy link
Contributor Author

Added instructions in cleanup section. If you think it needs some more explatin let me know!

@mpeyper mpeyper merged commit 35199c9 into testing-library:master Mar 22, 2019
@mpeyper
Copy link
Member

mpeyper commented Mar 22, 2019

Thanks @marquesm91. I'll put together a release for this later (I really need to automate this).

@jljorgenson18
Copy link

It looks like all of the cleanup functionality was removed. Is there any particular reason why?

@mpeyper
Copy link
Member

mpeyper commented Apr 30, 2019

Hi @jljorgenson18,

Yes, we changed the renderer from react-testing-library to react-test-renderer. One of the consequences of this change was that the test component we wrap the hook in is not rendered into a shared DOM anymore, so there is nothing to actually clean up between tests.

@jljorgenson18
Copy link

Got it. Rendering though is only one facet to cleanup. If you have a side effect in useEffect (like a window event listener), would the cleanup function returned from useEffect get called?

@mpeyper
Copy link
Member

mpeyper commented May 1, 2019

We never handled that explicitly (perhaps we should?). Usually you would unmount the hook yourself, triggering the cleanup, or handle your own specific cleanup yourself in a beforeEach or afterEach block.

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.

Add global cleanup
4 participants