GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — adding test for _stream_writable.js needDrain state by italoacasas · Pull Request #8799 · nodejs/node

Conversation

italoacasas commented Sep 27, 2016

Copy link

Copy Markdown

Checklist

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

Affected core subsystem(s)

test

Description of change

Adding tests for stream._writableState.needDrain

Issue related: #8686

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

nodejs-github-bot added the test Issues and PRs related to Node.js core tests and test infrastructure. label

Sep 27, 2016

lpinca added the stream Issues and PRs related to Node.js streams. label

Sep 27, 2016

jasnell reviewed

Sep 27, 2016

View reviewed changes

Comment thread

test/parallel/test-stream-writable-needdrain-state.js Outdated

jasnell Sep 27, 2016

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

this callback should be wrapped in a common.mustCall() to ensure that it's actually called.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

italoacasas reacted with thumbs up emoji

All reactions

👍 1 reaction

addaleax approved these changes

Sep 30, 2016

View reviewed changes

addaleax left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

jasnell approved these changes

Sep 30, 2016

View reviewed changes

jasnell left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

jasnell commented Sep 30, 2016

Copy link

Copy Markdown

Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          test: writable stream needDrain state

4b9c503

rvagg force-pushed the master branch 2 times, most recently from c133999 to 83c7a88 Compare

October 18, 2016 17:02

italoacasas commented Oct 25, 2016

Copy link

Copy Markdown

Author

@mcollina ping

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mcollina approved these changes

Oct 26, 2016

View reviewed changes

mcollina left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

mcollina commented Oct 26, 2016

Copy link

Copy Markdown

Member

Ci: https://ci.nodejs.org/job/node-test-pull-request/4681/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mcollina commented Oct 26, 2016

Copy link

Copy Markdown

Member

Failures unrelated. This is good to go, I'm planning on merging tomorrow or Friday, if no one else has objections.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cjihrig approved these changes

Oct 26, 2016

View reviewed changes

mcollina commented Oct 28, 2016 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

Merged in 9ed7261

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mcollina closed this

Oct 28, 2016

mcollina pushed a commit that referenced this pull request

Oct 28, 2016


          test: writable stream needDrain state

9ed7261

Add a test for _writableState.needDrain.

PR-URL: #8799
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Related: #8686

evanlucas pushed a commit that referenced this pull request

Nov 3, 2016


          test: writable stream needDrain state

c9b67c6

Add a test for _writableState.needDrain.

PR-URL: #8799
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Related: #8686

evanlucas mentioned this pull request

Nov 3, 2016

v7.1.0 proposal - 2016-11-08 #9438

Merged

MylesBorins pushed a commit that referenced this pull request

Nov 18, 2016


          test: writable stream needDrain state

4b50419

Add a test for _writableState.needDrain.

PR-URL: #8799
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Related: #8686

MylesBorins pushed a commit that referenced this pull request

Nov 18, 2016


          test: writable stream needDrain state

40ef239

Add a test for _writableState.needDrain.

PR-URL: #8799
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Related: #8686

MylesBorins added land-on-v4.x labels

Nov 18, 2016

MylesBorins pushed a commit that referenced this pull request

Nov 19, 2016


          test: writable stream needDrain state

1fee6c1

Add a test for _writableState.needDrain.

PR-URL: #8799
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Related: #8686

This was referenced Nov 22, 2016

v6.9.2 proposal #9735

Merged

v4.7.0 proposal #9736

Merged

Reviewers

mcollina

mcollina approved these changes

jasnell

jasnell approved these changes

addaleax

addaleax approved these changes

cjihrig

cjihrig approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

stream

Issues and PRs related to Node.js streams.

test

Issues and PRs related to Node.js core tests and test infrastructure.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

8 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.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

italoacasas commented Sep 27, 2016

Checklist

Affected core subsystem(s)

Description of change

Uh oh!

jasnell Sep 27, 2016

Choose a reason for hiding this comment

Uh oh!

addaleax left a comment

Choose a reason for hiding this comment

Uh oh!

jasnell left a comment

Choose a reason for hiding this comment

Uh oh!

jasnell commented Sep 30, 2016

Uh oh!

italoacasas commented Oct 25, 2016

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

mcollina commented Oct 26, 2016

Uh oh!

mcollina commented Oct 26, 2016

Uh oh!

mcollina commented Oct 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

mcollina commented Oct 28, 2016 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.