Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

stream, test: add tests for 'readable' state machine #8683

Closed
mcollina opened this issue Sep 21, 2016 · 2 comments
Closed

stream, test: add tests for 'readable' state machine #8683

mcollina opened this issue Sep 21, 2016 · 2 comments
Assignees
@italoacasas
Labels
good first issue Issues that are suitable for first-time contributors. stream Issues and PRs related to the stream subsystem. test Issues and PRs related to the tests.

Comments

@mcollina
Copy link
Member

Add test coverage for:

stream._readableState.needReadable
stream._readableState.emittedReadable
stream._readableState.readableListening
stream._readableState.resumeScheduled

node/lib/_stream_readable.js

Lines 74 to 77 in 774146d

this.needReadable = false;
this.emittedReadable = false;
this.readableListening = false;
this.resumeScheduled = false;

Those govern when and how we need to emit a 'readable' event and resume the flow.
The state machine is definitely complex, feel free to ask questions, and look hard at the source code. We can also add comments on the meaning of all those state variables.

Part of #8644.

cc @Fishrock123
@mcollina mcollina added stream Issues and PRs related to the stream subsystem. test Issues and PRs related to the tests. good first issue Issues that are suitable for first-time contributors. labels Sep 21, 2016
@mcollina mcollina mentioned this issue Sep 21, 2016
Closed
@sstern6
Copy link
Contributor

sstern6 commented Sep 24, 2016

@mcollina will get on this and let you know if I have any questions!!

Thanks

@italoacasas italoacasas self-assigned this Nov 21, 2016
@italoacasas italoacasas mentioned this issue Dec 1, 2016
Closed
3 tasks
This was referenced Dec 13, 2016
Closed
Closed
Fishrock123 pushed a commit that referenced this issue Dec 13, 2016
@italoacasas @Fishrock123
test: stream readableListening internal state
c792e2a 
PR-URL: #9864
Refs: #8683
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This was referenced Dec 13, 2016
Closed
Closed
italoacasas pushed a commit that referenced this issue Dec 16, 2016
@joyeecheung @italoacasas
test: stream readable needReadable state
3daa4b1 
PR-URL: #10241
Ref: #8683
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
mcollina pushed a commit that referenced this issue Dec 16, 2016
@joyeecheung @mcollina
stream, test: test _readableState.emittedReadable
8127501 
Part of #8683, increase coverage of the internal
state machine of streams.

PR-URL: #10249
See: #8683
See: #10230
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
italoacasas pushed a commit that referenced this issue Dec 17, 2016
@joyeecheung @italoacasas
stream, test: test _readableState.emittedReadable
28ffd59 
Part of #8683, increase coverage of the internal
state machine of streams.

PR-URL: #10249
See: #8683
See: #10230
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
italoacasas pushed a commit that referenced this issue Dec 17, 2016
@joyeecheung @italoacasas
test: stream readable needReadable state
ea16a2a 
PR-URL: #10241
Ref: #8683
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
@italoacasas
Copy link
Contributor

This is done, closing

cjihrig pushed a commit to cjihrig/node that referenced this issue Dec 20, 2016
@italoacasas @cjihrig
test: stream readable resumeScheduled state
cc9b4b7 
PR-URL: nodejs#10299
Ref: nodejs#8683
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
cjihrig pushed a commit that referenced this issue Dec 20, 2016
@joyeecheung @cjihrig
stream, test: test _readableState.emittedReadable
8ddbd53 
Part of #8683, increase coverage of the internal
state machine of streams.

PR-URL: #10249
See: #8683
See: #10230
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
cjihrig pushed a commit that referenced this issue Dec 20, 2016
@joyeecheung @cjihrig
test: stream readable needReadable state
8806342 
PR-URL: #10241
Ref: #8683
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
cjihrig pushed a commit that referenced this issue Dec 20, 2016
@italoacasas @cjihrig
test: stream readable resumeScheduled state
badb1dd 
PR-URL: #10299
Ref: #8683
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 16, 2017
@italoacasas @MylesBorins
test: stream readableListening internal state
3bd7ab1 
PR-URL: #9864
Refs: #8683
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 16, 2017
@italoacasas @MylesBorins
test: stream readableListening internal state
f355e20 
PR-URL: #9864
Refs: #8683
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 23, 2017
@joyeecheung @MylesBorins
stream, test: test _readableState.emittedReadable
f26213a 
Part of #8683, increase coverage of the internal
state machine of streams.

PR-URL: #10249
See: #8683
See: #10230
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 23, 2017
@joyeecheung @MylesBorins
stream, test: test _readableState.emittedReadable
9fa1e7d 
Part of #8683, increase coverage of the internal
state machine of streams.

PR-URL: #10249
See: #8683
See: #10230
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 23, 2017
@joyeecheung @MylesBorins
test: stream readable needReadable state
083ff5c 
PR-URL: #10241
Ref: #8683
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
MylesBorins pushed a commit that referenced this issue Jan 23, 2017
@joyeecheung @MylesBorins
test: stream readable needReadable state
ba36d55 
PR-URL: #10241
Ref: #8683
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
MylesBorins pushed a commit that referenced this issue Jan 23, 2017
@italoacasas @MylesBorins
test: stream readable resumeScheduled state
4d2e71b 
PR-URL: #10299
Ref: #8683
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 23, 2017
@italoacasas @MylesBorins
test: stream readable resumeScheduled state
6b8c4ab 
PR-URL: #10299
Ref: #8683
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 23, 2017
@italoacasas @MylesBorins
test: stream readable resumeScheduled state
cf1587a 
PR-URL: #10299
Ref: #8683
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 24, 2017
@italoacasas @MylesBorins
test: stream readableListening internal state
89bd0d3 
PR-URL: #9864
Refs: #8683
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 24, 2017
@joyeecheung @MylesBorins
stream, test: test _readableState.emittedReadable
1f6b10d 
Part of #8683, increase coverage of the internal
state machine of streams.

PR-URL: #10249
See: #8683
See: #10230
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 24, 2017
@joyeecheung @MylesBorins
test: stream readable needReadable state
2c7a59a 
PR-URL: #10241
Ref: #8683
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
MylesBorins pushed a commit that referenced this issue Jan 24, 2017
@italoacasas @MylesBorins
test: stream readable resumeScheduled state
93ed114 
PR-URL: #10299
Ref: #8683
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 24, 2017
@italoacasas @MylesBorins
test: stream readableListening internal state
0cc8d69 
PR-URL: #9864
Refs: #8683
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 24, 2017
@joyeecheung @MylesBorins
stream, test: test _readableState.emittedReadable
a023886 
Part of #8683, increase coverage of the internal
state machine of streams.

PR-URL: #10249
See: #8683
See: #10230
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 24, 2017
@joyeecheung @MylesBorins
test: stream readable needReadable state
76b034e 
PR-URL: #10241
Ref: #8683
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
MylesBorins pushed a commit that referenced this issue Jan 24, 2017
@italoacasas @MylesBorins
test: stream readable resumeScheduled state
d644821 
PR-URL: #10299
Ref: #8683
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 31, 2017
@italoacasas @MylesBorins
test: stream readableListening internal state
a75883e 
PR-URL: #9864
Refs: #8683
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 31, 2017
@joyeecheung @MylesBorins
stream, test: test _readableState.emittedReadable
9a84162 
Part of #8683, increase coverage of the internal
state machine of streams.

PR-URL: #10249
See: #8683
See: #10230
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 31, 2017
@joyeecheung @MylesBorins
test: stream readable needReadable state
a24a35f 
PR-URL: #10241
Ref: #8683
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
MylesBorins pushed a commit that referenced this issue Jan 31, 2017
@italoacasas @MylesBorins
test: stream readable resumeScheduled state
959860f 
PR-URL: #10299
Ref: #8683
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Feb 1, 2017
@italoacasas @MylesBorins
test: stream readableListening internal state
f12bab6 
PR-URL: #9864
Refs: #8683
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this issue Feb 1, 2017
@joyeecheung @MylesBorins
stream, test: test _readableState.emittedReadable
271f578 
Part of #8683, increase coverage of the internal
state machine of streams.

PR-URL: #10249
See: #8683
See: #10230
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
MylesBorins pushed a commit that referenced this issue Feb 1, 2017
@joyeecheung @MylesBorins
test: stream readable needReadable state
135a7c9 
PR-URL: #10241
Ref: #8683
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
MylesBorins pushed a commit that referenced this issue Feb 1, 2017
@italoacasas @MylesBorins
test: stream readable resumeScheduled state
fd195b4 
PR-URL: #10299
Ref: #8683
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@snyk-bot snyk-bot mentioned this issue Jul 17, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@italoacasas italoacasas

Labels
good first issue Issues that are suitable for first-time contributors. stream Issues and PRs related to the stream subsystem. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mcollina @italoacasas @sstern6