test: coverage for util.promisify#17591
Closed
mithunsasidharan wants to merge 1 commit intonodejs:masterfrom
mithunsasidharan:pr_coverage_promisify
Closed
test: coverage for util.promisify#17591mithunsasidharan wants to merge 1 commit intonodejs:masterfrom mithunsasidharan:pr_coverage_promisify
mithunsasidharan wants to merge 1 commit intonodejs:masterfrom
mithunsasidharan:pr_coverage_promisify
Conversation
addaleax
approved these changes
Dec 10, 2017
Member
Trott
reviewed
Dec 10, 2017
test/parallel/test-util-promisify.js
Outdated
Member
There was a problem hiding this comment.
Please use TypeError here instead of Error.
Member
There was a problem hiding this comment.
I am going to open a PR to make that stricter.
Contributor
Author
There was a problem hiding this comment.
@Trott : I've updated that. Kindly review now. Thanks !
cjihrig
approved these changes
Dec 11, 2017
BridgeAR
approved these changes
Dec 11, 2017
Trott
approved these changes
Dec 11, 2017
4 tasks
4 tasks
maclover7
approved these changes
Dec 11, 2017
Contributor
maclover7
left a comment
There was a problem hiding this comment.
LGTM, but can you please lowercase util.promisify in the commit message
Contributor
Author
|
@maclover7 : Thanks for the feedback. I've updated the commit message. Kindly review now! |
lpinca
approved these changes
Dec 12, 2017
Member
Member
|
@mithunsasidharan since you open quite a few PRs - would you be so kind and tick off the checkboxes? It just makes it easier to have a good overview of what is going on 😃 |
jasnell
approved these changes
Dec 12, 2017
Contributor
Author
|
@BridgeAR : I'm sorry I missed to update the checklist before. I've updated it now. Kindly review. Thanks ! |
gireeshpunathil
approved these changes
Dec 13, 2017
apapirovski
suggested changes
Dec 14, 2017
Contributor
apapirovski
left a comment
There was a problem hiding this comment.
Can this be updated to take all the various types to confirm it's not just a falsy check or something similar?
[undefined, null, true, 0, 'str', {}, [], Symbol()].forEach(/* test */)
Contributor
Author
|
@apapirovski : Thanks for your feedback. I've updated the PR. Kindly review now! |
apapirovski
approved these changes
Dec 14, 2017
Contributor
Contributor
|
Landed in f86427e Thanks @mithunsasidharan! |
apapirovski
pushed a commit
that referenced
this pull request
Dec 14, 2017
Add a test that confirms that non-function arguments passed to util.promisify throw an ERR_INVALID_ARG_TYPE error. PR-URL: #17591 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Jon Moss <me@jonathanmoss.me> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MylesBorins
pushed a commit
that referenced
this pull request
Jan 8, 2018
Add a test that confirms that non-function arguments passed to util.promisify throw an ERR_INVALID_ARG_TYPE error. PR-URL: #17591 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Jon Moss <me@jonathanmoss.me> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Merged
MylesBorins
pushed a commit
that referenced
this pull request
Jan 22, 2018
Add a test that confirms that non-function arguments passed to util.promisify throw an ERR_INVALID_ARG_TYPE error. PR-URL: #17591 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Jon Moss <me@jonathanmoss.me> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
14 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added missing coverage for scenario where it throws when a non function is passed as argument to
Util.PromisifyChecklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test