test: add tests for a some stream.Readable use cases #7260
Merged
Conversation
This file contains 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
|
Thx @addaleax, these are much needed! Would you mind adding the equivalent of test/parallel/test-stream-readable-invalid-chunk.js |
| for (const target of writables) { | ||
| assert.deepStrictEqual(target.output, [input]); | ||
| } | ||
| }); |
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.
There was a problem hiding this comment.
@mcollina Updated with readable.on('end')
|
@mcollina In |
|
@addaleax quite true :), forget my comment (not enough coffee this morning). |
| readable.unpipe(target); | ||
| } | ||
|
|
||
| readable.push('something else'); // This does not get through. |
There was a problem hiding this comment.
there are probably some extra spaces here
There was a problem hiding this comment.
@mcollina Fixed (if you’re talking about the spaces before the comment)
|
LGTM |
|
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 ? |
|
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. |
|
Rebased against the current master, one more CI just to be sure: https://ci.nodejs.org/job/node-test-commit/3759/ |
|
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 <matteo.collina@gmail.com>
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 <matteo.collina@gmail.com>
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 <matteo.collina@gmail.com>
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 <matteo.collina@gmail.com>
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 <matteo.collina@gmail.com>
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 <matteo.collina@gmail.com>
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 <matteo.collina@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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/