test: add tests for a some stream.Readable use cases#7260
Merged
Uh oh!
There was an error while loading. Please reload this page.
Conversation
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
addaleax added stream 
Member
mcollina commented Jun 10, 2016
Thx @addaleax, these are much needed! Would you mind adding the equivalent of test/parallel/test-stream-readable-invalid-chunk.js |
Member
There was a problem hiding this comment.
why on('exit')? I would prefer having those when the streams ends. I think it's cleaner to read.
MemberAuthor
addaleax commented Jun 10, 2016
@mcollina In |
Member
mcollina commented Jun 10, 2016
@addaleax quite true :), forget my comment (not enough coffee this morning). |
Member
There was a problem hiding this comment.
there are probably some extra spaces here
MemberAuthor
There was a problem hiding this comment.
@mcollina Fixed (if you’re talking about the spaces before the comment)
Member
mcollina commented Jun 10, 2016
LGTM |
Contributor
calvinmetcalf commented Jun 10, 2016
should we squash this or leave them as 4 separate commits as they do 4 separate things, my gut says leave them be but I'm not sure @Fishrock123 opinion ? |
MemberAuthor
addaleax commented Jun 13, 2016
I don’t care a lot about squashing these or not. When landing other PRs, I usually go by something like “bisectable logical units”, so I’d probably leave them as they are unless somebody objects to that. |
MemberAuthor
addaleax commented Jun 15, 2016
Rebased against the current master, one more CI just to be sure: https://ci.nodejs.org/job/node-test-commit/3759/ |
Member
mcollina commented Jun 16, 2016
We usually squash to edit/reconstruct the metadata easily. |
* test: check invalid chunk error for readable.push Test that passing invalid chunks to readable.push() in non-object mode throw errors. * test: add simple object mode + decoder stream test * test: add test for readable stream lacking _read Check that using a readable stream without a _read method will throw an error. * test: add basic test for piping to multiple dests Add a simple test for piping and unpiping from a readable stream to multiple writable streams. PR-URL: nodejs#7260 Reviewed-By: Matteo Collina <[email protected]>
evanlucas pushed a commit that referenced this pull request Jun 27, 2016
* test: check invalid chunk error for readable.push Test that passing invalid chunks to readable.push() in non-object mode throw errors. * test: add simple object mode + decoder stream test * test: add test for readable stream lacking _read Check that using a readable stream without a _read method will throw an error. * test: add basic test for piping to multiple dests Add a simple test for piping and unpiping from a readable stream to multiple writable streams. PR-URL: #7260 Reviewed-By: Matteo Collina <[email protected]>
Merged
Merged
MylesBorins pushed a commit that referenced this pull request Jul 11, 2016
* test: check invalid chunk error for readable.push Test that passing invalid chunks to readable.push() in non-object mode throw errors. * test: add simple object mode + decoder stream test * test: add test for readable stream lacking _read Check that using a readable stream without a _read method will throw an error. * test: add basic test for piping to multiple dests Add a simple test for piping and unpiping from a readable stream to multiple writable streams. PR-URL: #7260 Reviewed-By: Matteo Collina <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
* test: check invalid chunk error for readable.push Test that passing invalid chunks to readable.push() in non-object mode throw errors. * test: add simple object mode + decoder stream test * test: add test for readable stream lacking _read Check that using a readable stream without a _read method will throw an error. * test: add basic test for piping to multiple dests Add a simple test for piping and unpiping from a readable stream to multiple writable streams. PR-URL: #7260 Reviewed-By: Matteo Collina <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
* test: check invalid chunk error for readable.push Test that passing invalid chunks to readable.push() in non-object mode throw errors. * test: add simple object mode + decoder stream test * test: add test for readable stream lacking _read Check that using a readable stream without a _read method will throw an error. * test: add basic test for piping to multiple dests Add a simple test for piping and unpiping from a readable stream to multiple writable streams. PR-URL: #7260 Reviewed-By: Matteo Collina <[email protected]>
Merged
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
* test: check invalid chunk error for readable.push Test that passing invalid chunks to readable.push() in non-object mode throw errors. * test: add simple object mode + decoder stream test * test: add test for readable stream lacking _read Check that using a readable stream without a _read method will throw an error. * test: add basic test for piping to multiple dests Add a simple test for piping and unpiping from a readable stream to multiple writable streams. PR-URL: #7260 Reviewed-By: Matteo Collina <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
* test: check invalid chunk error for readable.push Test that passing invalid chunks to readable.push() in non-object mode throw errors. * test: add simple object mode + decoder stream test * test: add test for readable stream lacking _read Check that using a readable stream without a _read method will throw an error. * test: add basic test for piping to multiple dests Add a simple test for piping and unpiping from a readable stream to multiple writable streams. PR-URL: #7260 Reviewed-By: Matteo Collina <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
4 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.
Checklist
make -j4 test(UNIX) orvcbuild test nosign(Windows) passesAffected core subsystem(s)
test
Description of change
Adds tests for a couple of
stream.Readableuse cases that were previously not covered by tests./cc @nodejs/streams
CI: https://ci.nodejs.org/job/node-test-commit/3709/