test,http: fix http dump test

Make sure the dump test actually verify what is happening and it is
not flaky.

See: https://github.com/nodejs/node/issues/19139
PR-URL: https://github.com/nodejs/node/pull/19823
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

lib/_http_server.js

@@ -560,7 +560,7 @@ function resOnFinish(req, res, socket, state, server) {
   // if the user never called req.read(), and didn't pipe() or
   // .resume() or .on('data'), then we call req._dump() so that the
   // bytes will be pulled off the wire.
-  if (!req._consuming && !req._readableState.resumeScheduled)
+  if (!req._readableState.resumeScheduled)
     req._dump();
 
   res.detachSocket(socket);

test/sequential/test-http-dump-req-when-res-ends.js

@@ -2,53 +2,71 @@
 
 const common = require('../common');
 const http = require('http');
+const assert = require('assert');
 const fs = require('fs');
 
-const server = http.createServer(function(req, res) {
-  // this checks if the request gets dumped
+let resEnd = null;
+
+const server = http.createServer(common.mustCall(function(req, res) {
+  // This checks if the request gets dumped
+  // resume will be triggered by res.end().
   req.on('resume', common.mustCall(function() {
-    console.log('resume called');
+    // There is no 'data' event handler anymore
+    // it gets automatically removed when dumping the request.
+    assert.strictEqual(req.listenerCount('data'), 0);
 
     req.on('data', common.mustCallAtLeast(function(d) {
+      // Leaving the console.log explicitly, so that
+      // we can know how many chunks we have received.
       console.log('data', d);
     }, 1));
   }));
 
-  // end is not called as we are just exhausting
-  // the in-memory buffer
-  req.on('end', common.mustNotCall);
+  // We explicitly pause the stream
+  // so that the following on('data') does not cause
+  // a resume.
+  req.pause();
+  req.on('data', function() {});
 
-  // this 'data' handler will be removed when dumped
-  req.on('data', common.mustNotCall);
-
-  // start sending the response
+  // Start sending the response.
   res.flushHeaders();
 
-  setTimeout(function() {
-    res.end('hello world');
-  }, common.platformTimeout(100));
-});
+  resEnd = function() {
+    setImmediate(function() {
+      res.end('hello world');
+    });
+  };
+}));
 
-server.listen(0, function() {
+server.listen(0, common.mustCall(function() {
   const req = http.request({
     method: 'POST',
     port: server.address().port
   });
 
   // Send the http request without waiting
-  // for the body
+  // for the body.
   req.flushHeaders();
 
   req.on('response', common.mustCall(function(res) {
-    // pipe the body as soon as we get the headers of the
-    // response back
-    fs.createReadStream(__filename).pipe(req);
+    // Pipe the body as soon as we get the headers of the
+    // response back.
+    const readFileStream = fs.createReadStream(__filename);
+    readFileStream.on('end', resEnd);
+
+    readFileStream.pipe(req);
 
     res.resume();
 
-    // wait for the response
+    // Wait for the response.
     res.on('end', function() {
       server.close();
     });
   }));
-});
+
+  req.on('error', function() {
+    // An error can happen if there is some data still
+    // being sent, as the other side is calling .destroy()
+    // this is safe to ignore.
+  });
+}));