-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat: add warnings when globals are missing #1244
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 39fdc6c:
|
Codecov Report
@@ Coverage Diff @@
## main #1244 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 4 4
Lines 192 192
Branches 38 40 +2
=========================================
Hits 192 192
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea!
I'm probably "stealing" this for Angular Testing Library :)
Co-authored-by: Tim Deschryver <[email protected]>
@MatanBobi (and @timdeschryver, if you're still interested in Angular's implementation), it seems like this change could potentially run the risk of cluttering the logs for developers who intentionally aren't using the automatic cleanup functions. (For example, I'm using Is it possible to only display this warning once per call to |
Thanks @ITenthusiasm that's a valid point, we'll look into this. |
@@ -20,6 +20,10 @@ if (typeof process === 'undefined' || !process.env?.RTL_SKIP_AUTO_CLEANUP) { | |||
teardown(() => { | |||
cleanup() | |||
}) | |||
} else { | |||
console.warn( | |||
`The current test runner does not support afterEach/teardown hooks. This means we won't be able to run automatic cleanup and you should be calling cleanup() manually.`, |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
What: Adds a warning when we weren't able to setup logic through test runner globals (
afterEach
,beforeAll
,afterAll
).Why: This was raised in #1240 and I believe it's worth adding because some stuff won't work for users so they should have a warning stating why.
How:
Adding
else
blocks in the relevant places.Checklist:
[ ] Documentation added to thedocs site
TestsTypeScript definitions updated