Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fs: fix promisified fs.readdir withFileTypes #22832

Closed
wants to merge 1 commit into from
Closed

fs: fix promisified fs.readdir withFileTypes #22832

wants to merge 1 commit into from

Conversation

apapirovski
Copy link
Member

@apapirovski apapirovski commented Sep 13, 2018 •
edited
Loading

Fixes #22829

CI: https://ci.nodejs.org/job/node-test-pull-request/17164/

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@apapirovski build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/873/pipeline

@nodejs-github-bot nodejs-github-bot added the fs Issues and PRs related to the fs subsystem / file system. label Sep 13, 2018
@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 13, 2018
@bengl bengl mentioned this pull request Sep 13, 2018
Closed
2 tasks
BridgeAR
BridgeAR reviewed Sep 13, 2018
View reviewed changes
test/parallel/test-fs-readdir-types.js
@@ -57,6 +57,14 @@ fs.readdir(readdirDir, {
assertDirents(dirents);
}));

// Check the promisified version
assert.doesNotReject(async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doesNotReject should be obsolete. Any unhandled rejection is going to result in a test failure.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm OK with leaving assert.doesNotReject() in there anyway. In fact, I think I prefer it. It means that someone reading the test doesn't need to know that common performs the magic of adding a listener to the 'unhandledRejection' event.

@trivikr
Copy link
Member

trivikr commented Sep 15, 2018

Resumed build: https://ci.nodejs.org/job/node-test-pull-request/17201/

Trott pushed a commit to Trott/io.js that referenced this pull request Sep 15, 2018
@apapirovski @Trott
fs: fix promisified fs.readdir withFileTypes
16210ca 
PR-URL: nodejs#22832
Reviewed-By: Bryan English <bryan@bryanenglish.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@Trott
Copy link
Member

Trott commented Sep 15, 2018

Landed in 16210ca

@Trott Trott closed this Sep 15, 2018
targos pushed a commit that referenced this pull request Sep 16, 2018
@apapirovski @targos
fs: fix promisified fs.readdir withFileTypes
479e1ec 
PR-URL: #22832
Reviewed-By: Bryan English <bryan@bryanenglish.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@vsemozhetbyt vsemozhetbyt mentioned this pull request Sep 17, 2018
Closed
@vsemozhetbyt
Copy link
Contributor

Strangely, our automation did not close the fixed issue for this PR.

@apapirovski apapirovski deleted the fix-with-types branch September 17, 2018 15:01
@apapirovski
Copy link
Member Author

@vsemozhetbyt That one's one me. I used "fixes #issue" without a colon which I think NCU doesn't pick up.

@targos targos mentioned this pull request Sep 18, 2018
Merged
targos added a commit that referenced this pull request Sep 18, 2018
@targos
2018-09-19, Version 10.11.0 (Current)
4958ee3 
Notable changes:

* fs
  * Added a `recursive` option to `fs.mkdir` and `fs.mkdirSync`. If
    this option is set to `true`, non-existing parent folders will be
    automatically created.
    #21875
  * Fixed fsPromises.readdir `withFileTypes`.
    #22832
* http2
  * Added `http2stream.endAfterHeaders` property.
    #22843
* module
  * Added `module.createRequireFromPath(filename)`. This new method can
    be used to create a custom `require` function that will resolve
    modules relative to the `filename` path.
    #19360
* url
  * Added `url.fileURLToPath(url)` and `url.pathToFileURL(path)`. These
    methods can be used to correctly convert between `file:` URLs and
    absolute paths.
    #22506
* util
  * Added `util.types.isBoxedPrimitive(value)`.
    #22620
* Windows
  * The Windows msi installer now provides an option to automatically
    install the tools required to build native modules.
    #22645
* Added new collaborators:
  * boneskull (https://github.com/boneskull) - Christopher Hiller
* The Technical Steering Committee has new members:
  * apapirovski (https://github.com/apapirovski) - Anatoli Papirovski
  * gabrielschulhof (https://github.com/gabrielschulhof) - Gabriel Schulhof

PR-URL: #22932
targos pushed a commit that referenced this pull request Sep 19, 2018
@apapirovski @targos
fs: fix promisified fs.readdir withFileTypes
1cf6e3e 
PR-URL: #22832
Reviewed-By: Bryan English <bryan@bryanenglish.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos added a commit that referenced this pull request Sep 19, 2018
@targos
2018-09-19, Version 10.11.0 (Current)
960717b 
Notable changes:

* fs
  * Added a `recursive` option to `fs.mkdir` and `fs.mkdirSync`. If
    this option is set to `true`, non-existing parent folders will be
    automatically created.
    #21875
  * Fixed fsPromises.readdir `withFileTypes`.
    #22832
* http2
  * Added `http2stream.endAfterHeaders` property.
    #22843
* module
  * Added `module.createRequireFromPath(filename)`. This new method can
    be used to create a custom `require` function that will resolve
    modules relative to the `filename` path.
    #19360
* url
  * Added `url.fileURLToPath(url)` and `url.pathToFileURL(path)`. These
    methods can be used to correctly convert between `file:` URLs and
    absolute paths.
    #22506
* util
  * Added `util.types.isBoxedPrimitive(value)`.
    #22620
* Added new collaborators:
  * boneskull (https://github.com/boneskull) - Christopher Hiller
* The Technical Steering Committee has new members:
  * apapirovski (https://github.com/apapirovski) - Anatoli Papirovski
  * gabrielschulhof (https://github.com/gabrielschulhof) - Gabriel Schulhof

PR-URL: #22932
targos pushed a commit that referenced this pull request Sep 20, 2018
@apapirovski @targos
fs: fix promisified fs.readdir withFileTypes
24a35f9 
PR-URL: #22832
Reviewed-By: Bryan English <bryan@bryanenglish.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos added a commit that referenced this pull request Sep 20, 2018
@targos
2018-09-20, Version 10.11.0 (Current)
8c70b20 
Notable changes:

* fs
  * Fixed fsPromises.readdir `withFileTypes`.
    #22832
* http2
  * Added `http2stream.endAfterHeaders` property.
    #22843
* util
  * Added `util.types.isBoxedPrimitive(value)`.
    #22620
* Added new collaborators:
  * boneskull (https://github.com/boneskull) - Christopher Hiller
* The Technical Steering Committee has new members:
  * apapirovski (https://github.com/apapirovski) - Anatoli Papirovski
  * gabrielschulhof (https://github.com/gabrielschulhof) - Gabriel Schulhof

PR-URL: #22932
targos added a commit that referenced this pull request Sep 20, 2018
@targos
2018-09-20, Version 10.11.0 (Current)
df9abb6 
Notable changes:

* fs
  * Fixed fsPromises.readdir `withFileTypes`.
    #22832
* http2
  * Added `http2stream.endAfterHeaders` property.
    #22843
* util
  * Added `util.types.isBoxedPrimitive(value)`.
    #22620
* Added new collaborators:
  * boneskull (https://github.com/boneskull) - Christopher Hiller
* The Technical Steering Committee has new members:
  * apapirovski (https://github.com/apapirovski) - Anatoli Papirovski
  * gabrielschulhof (https://github.com/gabrielschulhof) - Gabriel Schulhof

PR-URL: #22932
@nellh nellh mentioned this pull request Aug 6, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BridgeAR BridgeAR BridgeAR approved these changes

@Trott Trott Trott approved these changes

@trivikr trivikr trivikr approved these changes

@richardlau richardlau richardlau approved these changes

@cjihrig cjihrig cjihrig approved these changes

@lpinca lpinca lpinca approved these changes

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

@bengl bengl bengl approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

fs.promises.readdir( path, { withFileTypes: true } ) error