Global cleanup #199
Global cleanup #199
Conversation
Codecov Report
@@ Coverage Diff @@
## master #199 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 1 2 +1
Lines 45 56 +11
Branches 4 5 +1
=====================================
+ Hits 45 56 +11
Continue to review full report at Codecov.
|
| } | ||
|
|
||
| // Automatically registers cleanup in supported testing frameworks | ||
| if (typeof afterEach === 'function' && !process.env.RHTL_SKIP_AUTO_CLEANUP) { |
There was a problem hiding this comment.
📓 process.env.RHTL_SKIP_AUTO_CLEANUP only needs to be "truthy" and not true to skip.
Do we want this to explicitly be the string "true"?
There was a problem hiding this comment.
Again, this is implemented as per @testing-library/react-testing-library
There was a problem hiding this comment.
Cool. Nice to see sharing the pattern from RTL for dev familiarity.
| > Please note that this is done automatically if the testing framework you're using supports the | ||
| > `afterEach` global (like mocha, Jest, and Jasmine). If not, you will need to do manual cleanups | ||
| > after each test. |
There was a problem hiding this comment.
❓ Do you happen to have any reference or example of a testing library auto adding a global afterEach?
I think I like this, i'm just curious about community best practices and such.
There was a problem hiding this comment.
The registration of this is lifted almost verbatim from @testing-library/react-testing-library#430
I don't think this is a commonly used pattern (yet).
| }) | ||
|
|
||
| test('second', () => { | ||
| expect(cleanupCalled).toBe(false) |
There was a problem hiding this comment.
❓ Is this test supposed to check true, expect(cleanupCalled).toBe(true)?
It appears to be a copy of the above test, autoCleanup.disabled.test.js, but without the process.env setup.
You may need to clear the require cache and reset the process.env before running?
There was a problem hiding this comment.
No. In the beforeAll am overriding afterEach to not be a function to test the case where the testing framework does not support it. I was very worried about this, but I tested thoroughly and jest brings it back for the next test.
Happy to hear ideas for better ways to test this case.
|
Yeah, will have to be a major, even though I doubt anyone will actually be affected by the break. |
|
Ok, I'm going to merge this now and get a major release out. @hartzis, if you would like to, you are welcome to submit a PR adding yourself as a contributor for your efforts reviewing this PR. |
What:
Added cleanup functionality to unmount any rendered test components.
Resolved #76
Why:
Currently, the underlying rendered test component is not necessarily unmounted at the end of a test, resulting in effects from
useEffectanduseLayoutEffecthanging around between tests.How:
The library now exports a global
cleanupfunction that will unmount any rendered hooks. As per testing-library/react-testing-library#430 we also auto regester anafterEachhandler in supported test frameworks (with an environment variable opt out).Checklist: