Skip to content

test: writable.write arg type - #29746

Closed
ronag wants to merge 1 commit into
nodejs:masterfrom
nxtedition:test-writable-write-invalid-chunk
Closed

test: writable.write arg type#29746
ronag wants to merge 1 commit into
nodejs:masterfrom
nxtedition:test-writable-write-invalid-chunk

Conversation

@ronag

@ronagronag commented Sep 28, 2019•
edited
Loading

Copy link
Copy Markdown
Member

Adds some test for passing invalid args to Writable.write

This basically ports some test from Readable.push.

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

@ronag
ronagforce-pushed the test-writable-write-invalid-chunk branch from 65df100 to af366a5CompareSeptember 28, 2019 07:49
ronag
ronag commented Sep 28, 2019
View reviewed changes
Comment threadtest/parallel/test-stream-writable-invalid-chunk.js Outdated
Comment threadtest/parallel/test-stream-writable-invalid-chunk.js Outdated
@ronag

ronag commented Sep 28, 2019

Copy link
Copy Markdown
MemberAuthor

There is a slight discrepancy between null and undefined both inside and outside objectMode which I'd hope someone could clarify the rationale behind.

@ronag
ronagforce-pushed the test-writable-write-invalid-chunk branch from af366a5 to 780f62dCompareSeptember 28, 2019 07:53
@Trott

Trott commented Oct 1, 2019

Copy link
Copy Markdown
Member

@nodejs/testing

@nodejs-github-bot

nodejs-github-bot commented Oct 1, 2019•
edited by Trott
Loading

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/25780/ ✔️

@Trott

Trott commented Oct 2, 2019

Copy link
Copy Markdown
Member

@nodejs/streams (particularly on the "is undefined being handled correctly?" question)

mcollina
mcollina approved these changes Oct 3, 2019
View reviewed changes

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Comment threadtest/parallel/test-stream-writable-invalid-chunk.js Outdated
Comment threadtest/parallel/test-stream-writable-invalid-chunk.js Outdated
@TrottTrott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 3, 2019
@Trott

Trott commented Oct 3, 2019

Copy link
Copy Markdown
Member

Landed in 6ea51bc

@TrottTrott closed this Oct 3, 2019
Trott pushed a commit that referenced this pull request Oct 3, 2019
@ronag@Trott
test: add test for writable.write() argument types
6ea51bc
PR-URL: #29746
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BridgeAR pushed a commit that referenced this pull request Oct 9, 2019
@ronag@BridgeAR
test: add test for writable.write() argument types
bf1727a
PR-URL: #29746
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@BridgeARBridgeAR mentioned this pull request Oct 10, 2019
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollinamcollinamcollina approved these changes
@TrottTrottTrott approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ronag@Trott@nodejs-github-bot@mcollina