fix: move findPort as static method on Server (#3296)

Author: anshumanv

lib/Server.js

@@ -761,8 +761,6 @@ class Server {
   }
 
   listen(port, hostname, fn) {
-    const findPort = require('./utils/findPort');
-
     if (hostname === 'local-ip') {
       this.hostname = internalIp.v4.sync() || internalIp.v6.sync() || '0.0.0.0';
     } else if (hostname === 'local-ipv4') {
@@ -780,7 +778,7 @@ class Server {
     }
 
     return (
-      findPort(port || this.options.port)
+      Server.getFreePort(port || this.options.port)
         // eslint-disable-next-line no-shadow
         .then((port) => {
           this.options.port = port;
@@ -846,6 +844,35 @@ class Server {
     };
   }
 
+  static getFreePort(port) {
+    const pRetry = require('p-retry');
+    const portfinder = require('portfinder');
+
+    if (port && port !== 'auto') {
+      return Promise.resolve(port);
+    }
+
+    function runPortFinder() {
+      return new Promise((resolve, reject) => {
+        // default port
+        portfinder.basePort = 8080;
+        portfinder.getPort((error, foundPort) => {
+          if (error) {
+            return reject(error);
+          }
+
+          return resolve(foundPort);
+        });
+      });
+    }
+
+    // Try to find unused port and listen on it for 3 times,
+    // if port is not specified in options.
+    const defaultPortRetry = parseInt(process.env.DEFAULT_PORT_RETRY, 10) || 3;
+
+    return pRetry(runPortFinder, { retries: defaultPortRetry });
+  }
+
   getStats(statsObj) {
     const getStatsOption = require('./utils/getStatsOption');
 

lib/utils/findPort.js

@@ -1,32 +1,6 @@
 'use strict';
 
-const pRetry = require('p-retry');
-const portfinder = require('portfinder');
+// TODO(@anshumanv) - Drop in next major release
+const getFreePort = require('../Server').getFreePort;
 
-function runPortFinder() {
-  return new Promise((resolve, reject) => {
-    // default port
-    portfinder.basePort = 8080;
-    portfinder.getPort((error, port) => {
-      if (error) {
-        return reject(error);
-      }
-
-      return resolve(port);
-    });
-  });
-}
-
-function findPort(port) {
-  if (port && port !== 'auto') {
-    return Promise.resolve(port);
-  }
-
-  // Try to find unused port and listen on it for 3 times,
-  // if port is not specified in options.
-  const defaultPortRetry = parseInt(process.env.DEFAULT_PORT_RETRY, 10) || 3;
-
-  return pRetry(runPortFinder, { retries: defaultPortRetry });
-}
-
-module.exports = findPort;
+module.exports = getFreePort;

test/server/Server.test.js

@@ -1,13 +1,17 @@
 'use strict';
 
 const { relative, sep } = require('path');
+const http = require('http');
 const webpack = require('webpack');
 const sockjs = require('sockjs/lib/transport');
+// TODO(@anshumanv) - Remove this test in next major
+const findPort = require('../../lib/utils/findPort');
 const Server = require('../../lib/Server');
 const config = require('../fixtures/simple-config/webpack.config');
 const port = require('../ports-map').Server;
 const isWebpack5 = require('../helpers/isWebpack5');
 
+const getFreePort = Server.getFreePort;
 jest.mock('sockjs/lib/transport');
 
 const baseDevConfig = {
@@ -363,4 +367,150 @@ describe('Server', () => {
       expect(process.env.WEBPACK_SERVE).toBe(true);
     });
   });
+
+  describe('getFreePort', () => {
+    let dummyServers = [];
+
+    afterEach(() => {
+      delete process.env.DEFAULT_PORT_RETRY;
+
+      return dummyServers
+        .reduce(
+          (p, server) =>
+            p.then(
+              () =>
+                new Promise((resolve) => {
+                  server.close(resolve);
+                })
+            ),
+          Promise.resolve()
+        )
+        .then(() => {
+          dummyServers = [];
+        });
+    });
+
+    function createDummyServers(n) {
+      return (Array.isArray(n) ? n : [...new Array(n)]).reduce(
+        (p, _, i) =>
+          p.then(
+            () =>
+              new Promise((resolve, reject) => {
+                const server = http.createServer();
+
+                dummyServers.push(server);
+
+                server.listen(8080 + i, () => {
+                  resolve();
+                });
+
+                server.on('error', (error) => {
+                  reject(error);
+                });
+              })
+          ),
+        Promise.resolve()
+      );
+    }
+
+    it('should returns the port when the port is specified', () => {
+      process.env.DEFAULT_PORT_RETRY = 5;
+
+      return getFreePort(8082).then((freePort) => {
+        expect(freePort).toEqual(8082);
+      });
+    });
+
+    it('should returns the port when the port is null', () => {
+      const retryCount = 2;
+
+      process.env.DEFAULT_PORT_RETRY = 2;
+
+      return createDummyServers(retryCount)
+        .then(() => getFreePort(null))
+        .then((freePort) => {
+          expect(freePort).toEqual(8080 + retryCount);
+        });
+    });
+
+    it('should returns the port when the port is undefined', () => {
+      const retryCount = 2;
+
+      process.env.DEFAULT_PORT_RETRY = 2;
+
+      return (
+        createDummyServers(retryCount)
+          // eslint-disable-next-line no-undefined
+          .then(() => getFreePort(undefined))
+          .then((freePort) => {
+            expect(freePort).toEqual(8080 + retryCount);
+          })
+      );
+    });
+
+    it('should retry finding the port for up to defaultPortRetry times (number)', () => {
+      const retryCount = 3;
+
+      process.env.DEFAULT_PORT_RETRY = retryCount;
+
+      return createDummyServers(retryCount)
+        .then(() => getFreePort())
+        .then((freePort) => {
+          expect(freePort).toEqual(8080 + retryCount);
+        });
+    });
+
+    it('should retry finding the port for up to defaultPortRetry times (string)', () => {
+      const retryCount = 3;
+
+      process.env.DEFAULT_PORT_RETRY = `${retryCount}`;
+
+      return createDummyServers(retryCount)
+        .then(() => getFreePort())
+        .then((freePort) => {
+          expect(freePort).toEqual(8080 + retryCount);
+        });
+    });
+
+    // TODO: fix me, Flaky on CI
+    it.skip('should retry finding the port when serial ports are busy', () => {
+      const busyPorts = [8080, 8081, 8082, 8083];
+
+      process.env.DEFAULT_PORT_RETRY = 3;
+
+      return createDummyServers(busyPorts)
+        .then(() => getFreePort())
+        .then((freePort) => {
+          expect(freePort).toEqual(8080 + busyPorts.length);
+        });
+    });
+
+    it("should throws the error when the port isn't found", () => {
+      expect.assertions(1);
+
+      jest.mock('portfinder', () => {
+        return {
+          getPort: (callback) => callback(new Error('busy')),
+        };
+      });
+
+      const retryCount = 1;
+
+      process.env.DEFAULT_PORT_RETRY = 0;
+
+      return createDummyServers(retryCount)
+        .then(() => getFreePort())
+        .catch((err) => {
+          expect(err.message).toMatchSnapshot();
+        });
+    });
+
+    it('should work with findPort util', () => {
+      process.env.DEFAULT_PORT_RETRY = 5;
+
+      return findPort(8082).then((freePort) => {
+        expect(freePort).toEqual(8082);
+      });
+    });
+  });
 });

test/server/__snapshots__/Server.test.js.snap.webpack4

@@ -59,3 +59,5 @@ Array [
   ],
 ]
 `;
+
+exports[`Server getFreePort should throws the error when the port isn't found 1`] = `"busy"`;

test/server/__snapshots__/Server.test.js.snap.webpack5

@@ -59,3 +59,5 @@ Array [
   ],
 ]
 `;
+
+exports[`Server getFreePort should throws the error when the port isn't found 1`] = `"busy"`;