test: stream._writableState.ending

[italoacasas]

Checklist

• make -j4 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.ending
• Adding comment about needDrain flag

Issue related: #8686

[mcollina]

Edit: Not LGTM yet.

Can you please adjust the commit message?

We need to account for this behavior here: mainly we need to check that ending might be true while the other state variables (ended, finished) are false.
https://github.com/nodejs/node/blob/master/lib/_stream_writable.js#L496-L507

[italoacasas]

@mcollina I made some changes in the test, but It was not possible for me to test the fallowing behavior.

• ending = true while ended = false

The way that functionality is implemented right now is hard to test because they change to true at the "same time" endWritable.

This is my first time reading the stream codebase and maybe I'm not seeing something.

[mcollina]

@italoacasas you will need to listen to the 'finish' event, which will be emitted in https://github.com/nodejs/node/blob/master/lib/_stream_writable.js#L488 if there are no writes in flight (for which the callback has not be called yet).

[italoacasas]

@mcollina ping

[mcollina]

LGTM

[mcollina]

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

[mcollina]

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

[cjihrig]

Could you capitalize and punctuate the comments in this PR. You also have some typos: 'endeding' should be 'ending' in a few places I think. Other than that LGTM.

[mcollina]

Merged in fd16eed.

[gibfahn]

@mcollina Looks like this test is causing a linter error:

/Users/gib/wrk/com/DANGER/node/test/parallel/test-stream-writableState-ending.js3:7error'common'isassignedavaluebutneverusedno-unused-vars

Fix: #9335