-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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: add additional deprecation warning tests for rmdir recursive #35683
Conversation
1c6e32d
to
4ea6e4a
Compare
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.
This is looking good to me, it would be good to run your new tests with coverage.
I will add a section to the docs on how to do so.
@@ -6,7 +6,6 @@ const path = require('path'); | |||
|
|||
tmpdir.refresh(); | |||
|
|||
|
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.
nit: we usually try to avoid unrelated whitespace changes.
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.
Good to know. I'm pretty sure I accidentally added this extra whitespace in my previous PR so I was just cleaning it up.
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Antoine du Hamel <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #35683 +/- ##
=======================================
Coverage 96.40% 96.40%
=======================================
Files 220 220
Lines 73681 73675 -6
=======================================
- Hits 71031 71028 -3
+ Misses 2650 2647 -3
Continue to review full report at Codecov.
|
PR-URL: #35683 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Coe <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
Landed in c5b9b5b |
PR-URL: #35683 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Coe <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
The recently added code coverage in #35653 highlighted an uncovered case of the rmdir recursive deprecation warning. This PR adds two new tests to ensure we cover all the places where the deprecation warning can be shown in both the sync and async versions.
cc @bcoe @nodejs/tooling
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes