Uh oh!
There was an error while loading. Please reload this page.
doc,test: widen fsPromises.appendFile()'s data type, add missing tests - #64279
Conversation
0b04ed5 to
62f79deCompare| @@ -1241,7 +1241,7 @@ changes: | |||
| --> | |||
| * `path` {string|Buffer|URL|FileHandle} filename or {FileHandle} | |||
| * `data` {string|Buffer} | |||
| * `data` {string|Buffer|TypedArray|DataView|AsyncIterable|Iterable|Stream} | |||
There was a problem hiding this comment.
| *`data` {string|Buffer|TypedArray|DataView|AsyncIterable|Iterable|Stream} | |
| *`data` {string|Buffer|TypedArray|DataView|AsyncIterable|Iterable} |
We should get rid of this from writeFile as well. There is no special stream logic for these methods, they are just treated as async-iterables and consumed as such.
Otherwise, LGTM 👍
From lib/internal/fs/promises.js, one can see fsPromises.appendFile() is simply a wrapper on fsPromises.writeFile(). Hence if writeFile() can take non-buffer data argument, so can appendFile() too. To justify the doc change, I have also restuctured the relevant tests, to enhance the test coverage. They now includes the below test cases consistently. - await do(Append|Write)Buffer(); - await do(Append|Write)BufferAndCancel(); - await do(Append|Write)String(); - await do(Append|Write)Stream(); - await do(Append|Write)StreamWithCancel(); - await do(Append|Write)Iterable(); - await do(Append|Write)InvalidIterable(); - await do(Append|Write)IterableWithEncoding(); - await do(Append|Write)BufferIterable(); - await do(Append|Write)AsyncIterable(); - await do(Append|Write)LargeIterable(); - await do(Append|Write)InvalidValues(); - await do(Append|Write)TypedArrays(); Note that test/parallel/test-fs-promises-writefile-typedarray.js is subsumed by test/parallel/test-fs-promises-file-handle-writeFile.js, and test/parallel/test-fs-promises-writefile-with-fd.js becomes a subcase in test/parallel/test-fs-promises-file-handle-writeFile.js. Refs: nodejs#41677 Signed-off-by: Jimmy Leung <mingfamily@hotmail.com.hk>
62f79de to
275b809CompareCodecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## main #64279 +/- ##
=======================================
Coverage 90.25% 90.25% =======================================
Files 741 741 Lines 241207 241207 Branches 45430 45431 +1 =======================================
Hits 217698 217698 - Misses 15084 15085 +1 + Partials 8425 8424 -1 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
This is fine, but for running a batch of async tests, you may wish to consider utilising the node:test runner which has intrinsic async test support (example: https://github.com/nodejs/node/blob/main/test/parallel/test-aborted-util.js)
There was a problem hiding this comment.
I have not read too deep on how nodejs worked with async test batch in the latest practice.
I suppose I may come back and open another PR to address things in later free time :)
nodejs-github-bot
commented
Jul 9, 2026
Uh oh!
There was an error while loading. Please reload this page.
nodejs-github-bot
commented
Jul 14, 2026
Landed in 3ac95f2 |
From lib/internal/fs/promises.js, one can see fsPromises.appendFile() is simply a wrapper on fsPromises.writeFile(). Hence if writeFile() can take non-buffer data argument, so can appendFile() too. To justify the doc change, I have also restuctured the relevant tests, to enhance the test coverage. They now includes the below test cases consistently. - await do(Append|Write)Buffer(); - await do(Append|Write)BufferAndCancel(); - await do(Append|Write)String(); - await do(Append|Write)Stream(); - await do(Append|Write)StreamWithCancel(); - await do(Append|Write)Iterable(); - await do(Append|Write)InvalidIterable(); - await do(Append|Write)IterableWithEncoding(); - await do(Append|Write)BufferIterable(); - await do(Append|Write)AsyncIterable(); - await do(Append|Write)LargeIterable(); - await do(Append|Write)InvalidValues(); - await do(Append|Write)TypedArrays(); Note that test/parallel/test-fs-promises-writefile-typedarray.js is subsumed by test/parallel/test-fs-promises-file-handle-writeFile.js, and test/parallel/test-fs-promises-writefile-with-fd.js becomes a subcase in test/parallel/test-fs-promises-file-handle-writeFile.js. Refs: #41677 Signed-off-by: Jimmy Leung <mingfamily@hotmail.com.hk> PR-URL: #64279 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
From lib/internal/fs/promises.js, one can see fsPromises.appendFile() is simply a wrapper on fsPromises.writeFile(). Hence if writeFile() can take non-buffer data argument, so can appendFile() too. To justify the doc change, I have also restuctured the relevant tests, to enhance the test coverage. They now includes the below test cases consistently. - await do(Append|Write)Buffer(); - await do(Append|Write)BufferAndCancel(); - await do(Append|Write)String(); - await do(Append|Write)Stream(); - await do(Append|Write)StreamWithCancel(); - await do(Append|Write)Iterable(); - await do(Append|Write)InvalidIterable(); - await do(Append|Write)IterableWithEncoding(); - await do(Append|Write)BufferIterable(); - await do(Append|Write)AsyncIterable(); - await do(Append|Write)LargeIterable(); - await do(Append|Write)InvalidValues(); - await do(Append|Write)TypedArrays(); Note that test/parallel/test-fs-promises-writefile-typedarray.js is subsumed by test/parallel/test-fs-promises-file-handle-writeFile.js, and test/parallel/test-fs-promises-writefile-with-fd.js becomes a subcase in test/parallel/test-fs-promises-file-handle-writeFile.js. Refs: #41677 Signed-off-by: Jimmy Leung <mingfamily@hotmail.com.hk> PR-URL: #64279 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
From lib/internal/fs/promises.js, one can see fsPromises.appendFile() is simply a wrapper on fsPromises.writeFile(). Hence if writeFile() can take non-buffer data argument, so can appendFile() too. To justify the doc change, I have also restuctured the relevant tests, to enhance the test coverage. They now includes the below test cases consistently. - await do(Append|Write)Buffer(); - await do(Append|Write)BufferAndCancel(); - await do(Append|Write)String(); - await do(Append|Write)Stream(); - await do(Append|Write)StreamWithCancel(); - await do(Append|Write)Iterable(); - await do(Append|Write)InvalidIterable(); - await do(Append|Write)IterableWithEncoding(); - await do(Append|Write)BufferIterable(); - await do(Append|Write)AsyncIterable(); - await do(Append|Write)LargeIterable(); - await do(Append|Write)InvalidValues(); - await do(Append|Write)TypedArrays(); Note that test/parallel/test-fs-promises-writefile-typedarray.js is subsumed by test/parallel/test-fs-promises-file-handle-writeFile.js, and test/parallel/test-fs-promises-writefile-with-fd.js becomes a subcase in test/parallel/test-fs-promises-file-handle-writeFile.js. Refs: #41677 Signed-off-by: Jimmy Leung <mingfamily@hotmail.com.hk> PR-URL: #64279 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
From lib/internal/fs/promises.js, one can see fsPromises.appendFile() is simply a wrapper on fsPromises.writeFile(). Hence if writeFile() can take non-buffer data argument, so can appendFile() too. To justify the doc change, I have also restuctured the relevant tests, to enhance the test coverage. They now includes the below test cases consistently. - await do(Append|Write)Buffer(); - await do(Append|Write)BufferAndCancel(); - await do(Append|Write)String(); - await do(Append|Write)Stream(); - await do(Append|Write)StreamWithCancel(); - await do(Append|Write)Iterable(); - await do(Append|Write)InvalidIterable(); - await do(Append|Write)IterableWithEncoding(); - await do(Append|Write)BufferIterable(); - await do(Append|Write)AsyncIterable(); - await do(Append|Write)LargeIterable(); - await do(Append|Write)InvalidValues(); - await do(Append|Write)TypedArrays(); Note that test/parallel/test-fs-promises-writefile-typedarray.js is subsumed by test/parallel/test-fs-promises-file-handle-writeFile.js, and test/parallel/test-fs-promises-writefile-with-fd.js becomes a subcase in test/parallel/test-fs-promises-file-handle-writeFile.js. Refs: #41677 Signed-off-by: Jimmy Leung <mingfamily@hotmail.com.hk> PR-URL: #64279 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
From lib/internal/fs/promises.js, one can see fsPromises.appendFile() is
simply a wrapper on fsPromises.writeFile(). Hence if writeFile() can
take non-buffer data argument, so can appendFile() too.
To justify the doc change, I have also restuctured the relevant tests,
to enhance the test coverage. They now includes the below test cases
consistently.
Note that test/parallel/test-fs-promises-writefile-typedarray.js is
subsumed by test/parallel/test-fs-promises-file-handle-writeFile.js, and
test/parallel/test-fs-promises-writefile-with-fd.js becomes a subcase in
test/parallel/test-fs-promises-file-handle-writeFile.js.
Refs: #41677
Signed-off-by: Jimmy Leung mingfamily@hotmail.com.hk