addin test for _stream.writable finished state#8791
addin test for _stream.writable finished state#8791italoacasas wants to merge 1 commit intonodejs:masterfrom italoacasas:test/stream-writable-finished-state
Conversation
There was a problem hiding this comment.
The callback here should be wrapped in a common.mustCall() in order to validate that it really does get called.
There was a problem hiding this comment.
Can you please add an assert also here that finished is false?
|
Good job! cc @nodejs/streams |
|
Can you please update the commit message to follow our standards? |
|
The commit style guidelines are detailed here: https://github.com/nodejs/node/blob/master/CONTRIBUTING.md#step-3-commit :-) |
c133999 to
83c7a88
Compare
|
@mcollina ping |
|
Failures unrelated. This is good to go, I'm planning on merging tomorrow or Friday, if no one else has objections. |
There was a problem hiding this comment.
Could you capitalize and punctuate the comment please.
|
Merged in f12338d |
Add a test for _writableState.finished. PR-URL: #8791 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Related: #8686
Add a test for _writableState.finished. PR-URL: #8791 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Related: #8686
Add a test for _writableState.finished. PR-URL: #8791 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Related: #8686
Add a test for _writableState.finished. PR-URL: #8791 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Related: #8686
Add a test for _writableState.finished. PR-URL: #8791 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Related: #8686
Checklist
make -j8 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
test
Description of change
Issue related: #8686