Skip to content

Conversation

shisama
Copy link
Contributor

@shisama shisama commented Oct 3, 2018

Added tests to occur error when empty buffer is passed to filehandle.read()
to increase coverage.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 3, 2018
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: if we could write this with await it would look better.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, now that you are doing it, please cover the fs promises read with a file descriptor.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I think this needs to be aligned with 'The argument...

@shisama shisama force-pushed the test-fsPromises-readfile-empty-buffer branch from 61d6bef to a93f6e0 Compare October 6, 2018 15:17
@danbev
Copy link
Contributor

danbev commented Oct 10, 2018

@shisama Could you update the commit message and correct the length as reported by travis?

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

@shisama shisama force-pushed the test-fsPromises-readfile-empty-buffer branch from a93f6e0 to 3b01a5e Compare October 10, 2018 07:34
@shisama
Copy link
Contributor Author

shisama commented Oct 10, 2018

@danbev Thank you for your comment. I updated the commit message.

@danbev
Copy link
Contributor

danbev commented Oct 10, 2018

@shisama Sorry about this, but could I also ask you to rebase this to fix this travis CI failure? (this is related to #23397).

Masashi Hirano added 3 commits October 10, 2018 18:10
@shisama shisama force-pushed the test-fsPromises-readfile-empty-buffer branch from 3b01a5e to 41afe00 Compare October 10, 2018 09:10
@danbev
Copy link
Contributor

danbev commented Oct 10, 2018

Re-run of failing node-test-commit-linux-containered ✔️

@shisama
Copy link
Contributor Author

shisama commented Oct 10, 2018

@danbev Thank you to tell me that this should be rebased and to run CI.

@danbev
Copy link
Contributor

danbev commented Oct 10, 2018

Landed in a8530bc.

@danbev danbev closed this Oct 10, 2018
danbev pushed a commit that referenced this pull request Oct 10, 2018
Added tests to occur error when empty buffer is passed to
filehandle.read() to increase coverage.

PR-URL: #23250
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
jasnell pushed a commit that referenced this pull request Oct 17, 2018
Added tests to occur error when empty buffer is passed to
filehandle.read() to increase coverage.

PR-URL: #23250
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants