-
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 buffer to fs_permission tests #55734
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #55734 +/- ##
=======================================
Coverage 88.40% 88.40%
=======================================
Files 654 654
Lines 187655 187655
Branches 36111 36112 +1
=======================================
+ Hits 165889 165896 +7
- Misses 14996 14997 +1
+ Partials 6770 6762 -8 |
@@ -21,6 +21,11 @@ const regularFile = __filename; | |||
permission: 'FileSystemRead', | |||
resource: path.toNamespacedPath(blockedFile), | |||
})); | |||
fs.readFile(bufferBlockedFile, common.expectsError({ |
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.
non-blocking nit... this particular test is rather dense. While we're in here making changes it would be nice to fixup the formatting just a bit so that it's easier to read and see what is going on. Comments explaining what is being tested would be helpful also.
Landed in c8e1298 |
PR-URL: #55734 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#55734 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Increasing the permission model coverage by including Buffer in fs tests.