-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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 tests for options.fs in fs streams #33185
test: add tests for options.fs in fs streams #33185
Conversation
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.
Just a suggestion but looks nice 🙂
6e16e76
to
e1e860a
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.
LGTM
e1e860a
to
c671654
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.
The only exception thrown is here https://github.com/nodejs/node/blob/master/lib/internal/fs/streams.js#L83, the rest of them are not being touched.
c671654
to
258a8ca
Compare
@juanarbol addressed your comments, all branches are being tested now. |
258a8ca
to
41415f4
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.
Looks amazing to me.
PR-URL: #33185 Reviewed-By: Adrian Estrada <edsadr@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Landed in 39ff647 🎉 |
PR-URL: #33185 Reviewed-By: Adrian Estrada <edsadr@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
PR-URL: #33185 Reviewed-By: Adrian Estrada <edsadr@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
This add tests for the new
options.fs
override methods in thefs
Stream operationsChecklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes