stream: inline needMoreData function

Inline the needMoreData function since it has only one call place.
Update the related comment.
Add a test for the edge case where HWM=0 and state.length=0.
Add a test for ReadableStream.read(n) method's edge case where
n, HWM and state.length are all zero.
This proves that there is no easy way to simplify the check at
https://github.com/nodejs/node/blob/master/lib/_stream_readable.js#L440

Fixes: https://github.com/nodejs/node/issues/19893
Refs: https://github.com/nodejs/node/pull/19896

PR-URL: https://github.com/nodejs/node/pull/21009
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

1 files changed, 50 insertions, 23 deletions

diff --git a/test/parallel/test-streams-highwatermark.js b/test/parallel/test-streams-highwatermark.js
index 377fe08e90..4dd9694a46 100644
--- a/test/parallel/test-streams-highwatermark.js
+++ b/test/parallel/test-streams-highwatermark.js
@@ -1,31 +1,58 @@
 'use strict';
 const common = require('../common');
 
-// This test ensures that the stream implementation correctly handles values
-// for highWaterMark which exceed the range of signed 32 bit integers and
-// rejects invalid values.
-
 const assert = require('assert');
 const stream = require('stream');
 
-// This number exceeds the range of 32 bit integer arithmetic but should still
-// be handled correctly.
-const ovfl = Number.MAX_SAFE_INTEGER;
-
-const readable = stream.Readable({ highWaterMark: ovfl });
-assert.strictEqual(readable._readableState.highWaterMark, ovfl);
-
-const writable = stream.Writable({ highWaterMark: ovfl });
-assert.strictEqual(writable._writableState.highWaterMark, ovfl);
-
-for (const invalidHwm of [true, false, '5', {}, -5, NaN]) {
-  for (const type of [stream.Readable, stream.Writable]) {
-    common.expectsError(() => {
-      type({ highWaterMark: invalidHwm });
-    }, {
-      type: TypeError,
-      code: 'ERR_INVALID_OPT_VALUE',
-      message: `The value "${invalidHwm}" is invalid for option "highWaterMark"`
-    });
+{
+  // This test ensures that the stream implementation correctly handles values
+  // for highWaterMark which exceed the range of signed 32 bit integers and
+  // rejects invalid values.
+
+  // This number exceeds the range of 32 bit integer arithmetic but should still
+  // be handled correctly.
+  const ovfl = Number.MAX_SAFE_INTEGER;
+
+  const readable = stream.Readable({ highWaterMark: ovfl });
+  assert.strictEqual(readable._readableState.highWaterMark, ovfl);
+
+  const writable = stream.Writable({ highWaterMark: ovfl });
+  assert.strictEqual(writable._writableState.highWaterMark, ovfl);
+
+  for (const invalidHwm of [true, false, '5', {}, -5, NaN]) {
+    for (const type of [stream.Readable, stream.Writable]) {
+      common.expectsError(() => {
+        type({ highWaterMark: invalidHwm });
+      }, {
+        type: TypeError,
+        code: 'ERR_INVALID_OPT_VALUE',
+        message:
+          `The value "${invalidHwm}" is invalid for option "highWaterMark"`
+      });
+    }
+  }
+}
+
+{
+  // This test ensures that the push method's implementation
+  // correctly handles the edge case where the highWaterMark and
+  // the state.length are both zero
+
+  const readable = stream.Readable({ highWaterMark: 0 });
+
+  for (let i = 0; i < 3; i++) {
+    const needMoreData = readable.push();
+    assert.strictEqual(needMoreData, true);
   }
 }
+
+{
+  // This test ensures that the read(n) method's implementation
+  // correctly handles the edge case where the highWaterMark, state.length
+  // and n are all zero
+
+  const readable = stream.Readable({ highWaterMark: 0 });
+
+  readable._read = common.mustCall();
+  readable.read(0);
+}