Skip to content

test: add tests for a some stream.Readable use cases#7260

Merged
addaleax merged 1 commit intonodejs:masterfrom
addaleax:stream-testing
Jun 18, 2016
Merged

test: add tests for a some stream.Readable use cases#7260
addaleax merged 1 commit intonodejs:masterfrom
addaleax:stream-testing

Conversation

@addaleax
Copy link
Member

@addaleax addaleax commented Jun 10, 2016 •
edited
Loading

Checklist
  • make -j4 test (UNIX) or vcbuild test nosign (Windows) passes
  • a test and/or benchmark is included
  • the commit message follows commit guidelines
Affected core subsystem(s)

test

Description of change

Adds tests for a couple of stream.Readable use cases that were previously not covered by tests.

/cc @nodejs/streams

CI: https://ci.nodejs.org/job/node-test-commit/3709/

@addaleax addaleax added stream Issues and PRs related to the stream subsystem. test Issues and PRs related to the tests. labels Jun 10, 2016
@mcollina
Copy link
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
for objectMode streams?

mcollina
mcollina reviewed Jun 10, 2016
View reviewed changes
test/parallel/test-stream-pipe-multiple-pipes.js Outdated
Copy link
Member

@mcollina mcollina Jun 10, 2016

Choose a reason for hiding this comment

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

why on('exit')? I would prefer having those when the streams ends. I think it's cleaner to read.

Copy link
Member Author

@addaleax addaleax Jun 10, 2016

Choose a reason for hiding this comment

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

@mcollina Updated with readable.on('end')

@addaleax
Copy link
Member Author

addaleax commented Jun 10, 2016

@mcollina In chunkInvalid() there’s an explicit test for !state.objectMode, so… what exactly would an invalid chunk in object mode look like?

@mcollina
Copy link
Member

mcollina commented Jun 10, 2016

@addaleax quite true :), forget my comment (not enough coffee this morning).

mcollina
mcollina reviewed Jun 10, 2016
View reviewed changes
test/parallel/test-stream-pipe-multiple-pipes.js Outdated
Copy link
Member

@mcollina mcollina Jun 10, 2016

Choose a reason for hiding this comment

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

there are probably some extra spaces here

Copy link
Member Author

@addaleax addaleax Jun 10, 2016

Choose a reason for hiding this comment

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

@mcollina Fixed (if you’re talking about the spaces before the comment)

@mcollina
Copy link
Member

mcollina commented Jun 10, 2016

LGTM

@calvinmetcalf
Copy link
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 ?

@addaleax
Copy link
Member Author

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.

@addaleax
Copy link
Member Author

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/

@mcollina
Copy link
Member

mcollina commented Jun 16, 2016

We usually squash to edit/reconstruct the metadata easily.

@addaleax
test: add tests for some stream.Readable uses
3fe3b41 
* 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>
@addaleax addaleax merged commit 3fe3b41 into nodejs:master Jun 18, 2016
evanlucas pushed a commit that referenced this pull request Jun 27, 2016
@addaleax @evanlucas
test: add tests for some stream.Readable uses
e2bf250 
* 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>
@Fishrock123 Fishrock123 mentioned this pull request Jun 27, 2016
Merged
@Fishrock123 Fishrock123 mentioned this pull request Jul 5, 2016
Merged
@MylesBorins MylesBorins mentioned this pull request Jul 11, 2016
Closed
MylesBorins pushed a commit that referenced this pull request Jul 11, 2016
@addaleax
test: add tests for some stream.Readable uses
9c28be2 
* 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
@addaleax @MylesBorins
test: add tests for some stream.Readable uses
a852a44 
* 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
@addaleax
test: add tests for some stream.Readable uses
d27eb23 
* 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 MylesBorins mentioned this pull request Jul 12, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
@addaleax
test: add tests for some stream.Readable uses
6ec05db 
* 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
@addaleax
test: add tests for some stream.Readable uses
79b4588 
* 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

Reviewers

No reviews

Assignees

No one assigned

Labels

stream Issues and PRs related to the stream subsystem. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@addaleax @mcollina @calvinmetcalf @MylesBorins