Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Tests for joyent/node#5141 not working as advertised #5183

Closed
arhart opened this Issue · 2 comments

2 participants

@arhart

The tests for #5141 do not agree with the test comments. Of particular concern, first and second both test the !state.readign case, meaning second misses the state.reading && state.length case.

In other words,

} else if (state.length) {
  emitReadable(this, state);

can be removed from Readable.prototype.on and the tests would not fail.

I'll try to work up a pull request, but this'll be my first time so it might be awhile.
I've not yet submitted the CLA.

@arhart arhart referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@arhart arhart referenced this issue from a commit in arhart/node
@arhart arhart test: test intended code-paths
The tests did not agree with the test comments. Tests first and second
were both testing the !state.reading case. Now second tests the
state.reading && state.length case.

Fixes joyent/node#5183
933f1f1
@arhart

CLA submitted

@isaacs isaacs referenced this issue from a commit in isaacs/node
@arhart arhart test: test intended code-paths
The tests did not agree with the test comments. Tests first and second
were both testing the !state.reading case. Now second tests the
state.reading && state.length case.

Fixes joyent/node#5183
db8ce89
@isaacs
Owner

Landed on db8ce89. Thanks!

@isaacs isaacs closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.