test(server): added unix socket helper tests

Author: knagaitsev

lib/Server.js

@@ -752,16 +752,20 @@ class Server {
       }
     };
 
-    const onListeningCallback = () => {
+    const listenCallback = () => {
       if (typeof this.options.onListening === 'function') {
         this.options.onListening(this);
       }
     };
 
-    const fullCallback = (err) => {
+    const setupAndListenCallback = () => {
       setupCallback();
+      listenCallback();
+    };
+
+    const fullCallback = (err) => {
+      setupAndListenCallback();
       userCallback(err);
-      onListeningCallback();
     };
 
     // try to follow the Node standard in terms of deciding
@@ -773,7 +777,12 @@ class Server {
       // set this so that status helper can identify how the project is being run correctly
       this.options.socket = socket;
 
-      startUnixSocket(this.listeningApp, socket, fullCallback, userCallback);
+      startUnixSocket(
+        this.listeningApp,
+        socket,
+        setupAndListenCallback,
+        userCallback
+      );
     } else {
       this.listeningApp.listen(port, hostname, fullCallback);
     }

lib/utils/startUnixSocket.js

@@ -3,12 +3,20 @@
 const fs = require('fs');
 const net = require('net');
 
+// userCallback should always be called to indicate that either the server has
+// started listening, or an error was thrown.
+// setupAndListenCallback should only be called if the server starts listening
 function startUnixSocket(
   listeningApp,
   socket,
-  onListeningCallback,
-  errorCallback
+  setupAndListenCallback,
+  userCallback
 ) {
+  const fullCallback = (err) => {
+    setupAndListenCallback();
+    userCallback(err);
+  };
+
   const chmodSocket = (done) => {
     // chmod 666 (rw rw rw) - octal
     const READ_WRITE = 438;
@@ -17,22 +25,22 @@ function startUnixSocket(
 
   const startSocket = () => {
     listeningApp.on('error', (err) => {
-      errorCallback(err);
+      userCallback(err);
     });
 
     // 511 is the default value for the server.listen backlog parameter
     // https://nodejs.org/api/net.html#net_server_listen
     listeningApp.listen(socket, 511, (err) => {
       if (err) {
-        onListeningCallback(err);
+        fullCallback(err);
       } else {
-        chmodSocket(onListeningCallback);
+        chmodSocket(fullCallback);
       }
     });
   };
 
-  fs.access(socket, fs.constants.F_OK, (err) => {
-    if (err) {
+  fs.access(socket, fs.constants.F_OK, (e) => {
+    if (e) {
       // file does not exist
       startSocket();
     } else {
@@ -56,7 +64,7 @@ function startUnixSocket(
         clientSocket.destroy();
         // do not call onListening or the setup method, since the server
         // cannot start listening on a used socket
-        errorCallback(err);
+        userCallback(err);
       });
     }
   });

test/helpers/test-unix-socket.js

@@ -0,0 +1,26 @@
+'use strict';
+
+const http = require('http');
+
+const TestUnixSocket = class TestUnixSocket {
+  constructor() {
+    this.server = http.createServer();
+    this.sockets = new Set();
+    this.server.on('connection', (socket) => {
+      this.sockets.add(socket);
+      socket.on('close', () => {
+        this.sockets.delete(socket);
+      });
+    });
+  }
+
+  close(done) {
+    // get rid of connected sockets
+    for (const socket of this.sockets.values()) {
+      socket.destroy();
+    }
+    this.server.close(done);
+  }
+};
+
+module.exports = TestUnixSocket;

test/server/socket-option.test.js

@@ -1,14 +1,14 @@
 'use strict';
 
-const http = require('http');
 const net = require('net');
 const fs = require('fs');
 const path = require('path');
 const webpack = require('webpack');
 const testServer = require('../helpers/test-server');
+const TestUnixSocket = require('../helpers/test-unix-socket');
+const { skipTestOnWindows } = require('../helpers/conditional-test');
 const config = require('../fixtures/simple-config/webpack.config');
 const Server = require('../../lib/Server');
-const { skipTestOnWindows } = require('../helpers/conditional-test');
 
 describe('socket', () => {
   const socketPath = path.join('.', 'socket-option.webpack.sock');
@@ -87,19 +87,12 @@ describe('socket', () => {
       return;
     }
 
-    let dummyServer;
-    const sockets = new Set();
+    let testUnixSocket;
     beforeAll((done) => {
-      dummyServer = http.createServer();
-      dummyServer.listen(socketPath, 511, () => {
+      testUnixSocket = new TestUnixSocket();
+      testUnixSocket.server.listen(socketPath, 511, () => {
         done();
       });
-      dummyServer.on('connection', (socket) => {
-        sockets.add(socket);
-        socket.on('close', () => {
-          sockets.delete(socket);
-        });
-      });
     });
 
     it('should work as Unix socket', (done) => {
@@ -118,21 +111,13 @@ describe('socket', () => {
     });
 
     afterAll((done) => {
-      // get rid of connected sockets on the dummyServer
-      for (const socket of sockets.values()) {
-        socket.destroy();
-      }
-      dummyServer.close(() => {
+      testUnixSocket.close(() => {
         fs.unlink(socketPath, done);
       });
     });
   });
 
   describe('path to existent, unused file', () => {
-    if (skipTestOnWindows('Unix sockets are not supported on Windows')) {
-      return;
-    }
-
     let devServer;
     const options = {
       socket: socketPath,

test/server/utils/startUnixSocket.test.js

@@ -0,0 +1,139 @@
+'use strict';
+
+const net = require('net');
+const fs = require('fs');
+const path = require('path');
+const TestUnixSocket = require('../../helpers/test-unix-socket');
+const { skipTestOnWindows } = require('../../helpers/conditional-test');
+const startUnixSocket = require('../../../lib/utils/startUnixSocket');
+
+describe('startUnixSocket', () => {
+  const socketPath = path.join('.', 'startUnixSocket.webpack.sock');
+  let testUnixSocket = null;
+
+  describe('path to a non-existent file', () => {
+    let err;
+    beforeAll((done) => {
+      testUnixSocket = new TestUnixSocket();
+      startUnixSocket(
+        testUnixSocket.server,
+        socketPath,
+        () => {},
+        (e) => {
+          err = e;
+          done();
+        }
+      );
+    });
+
+    it('should work as Unix socket or error on windows', (done) => {
+      if (process.platform === 'win32') {
+        expect(err.code).toEqual('EACCES');
+        done();
+      } else {
+        const clientSocket = new net.Socket();
+        clientSocket.connect({ path: socketPath }, () => {
+          // this means the connection was made successfully
+          expect(true).toBeTruthy();
+          done();
+        });
+      }
+    });
+
+    afterAll((done) => {
+      testUnixSocket.close(() => {
+        fs.unlink(socketPath, done);
+      });
+    });
+  });
+
+  describe('path to existent, unused file', () => {
+    if (skipTestOnWindows('Unix sockets are not supported on Windows')) {
+      return;
+    }
+
+    beforeAll((done) => {
+      fs.writeFileSync(socketPath, '');
+      testUnixSocket = new TestUnixSocket();
+      startUnixSocket(testUnixSocket.server, socketPath, () => {}, done);
+    });
+
+    it('should work as Unix socket', (done) => {
+      const clientSocket = new net.Socket();
+      clientSocket.connect({ path: socketPath }, () => {
+        // this means the connection was made successfully
+        expect(true).toBeTruthy();
+        done();
+      });
+    });
+
+    afterAll((done) => {
+      testUnixSocket.close(() => {
+        fs.unlink(socketPath, done);
+      });
+    });
+  });
+
+  describe('path to existent file with listening server', () => {
+    if (skipTestOnWindows('Unix sockets are not supported on Windows')) {
+      return;
+    }
+
+    let dummyUnixSocket;
+    beforeAll((done) => {
+      dummyUnixSocket = new TestUnixSocket();
+      dummyUnixSocket.server.listen(socketPath, 511, () => {
+        done();
+      });
+    });
+
+    it('should work as Unix socket', (done) => {
+      testUnixSocket = new TestUnixSocket();
+      startUnixSocket(
+        testUnixSocket.server,
+        socketPath,
+        () => {},
+        (err) => {
+          expect(err).not.toBeNull();
+          expect(err).not.toBeUndefined();
+          expect(err.message).toEqual('This socket is already used');
+          testUnixSocket.close(done);
+        }
+      );
+    });
+
+    afterAll((done) => {
+      dummyUnixSocket.close(() => {
+        fs.unlink(socketPath, done);
+      });
+    });
+  });
+
+  describe('path to existent, unused file', () => {
+    beforeAll(() => {
+      fs.writeFileSync(socketPath, '');
+      testUnixSocket = new TestUnixSocket();
+    });
+
+    // this test is significant because previously the callback was called
+    // twice if a file at the given socket path already existed, but
+    // could be removed
+    it('should only call server.listen callback once', (done) => {
+      const userCallback = jest.fn();
+      startUnixSocket(
+        testUnixSocket.server,
+        socketPath,
+        () => {},
+        userCallback
+      );
+      setTimeout(() => {
+        expect(userCallback).toBeCalledTimes(1);
+        done();
+      }, 10000);
+    });
+
+    afterAll((done) => {
+      testUnixSocket.close(done);
+    });
+  });
+});