refactor(test): fix flakiness (#616)

Author: chimurai

package.json

@@ -69,6 +69,7 @@
     "eslint-config-prettier": "^8.3.0",
     "eslint-plugin-prettier": "^3.4.0",
     "express": "^4.17.1",
+    "get-port": "^5.1.1",
     "husky": "^6.0.0",
     "jest": "^26.6.3",
     "lint-staged": "^11.0.0",

test/e2e/express-router.spec.ts

@@ -1,20 +1,16 @@
 import * as express from 'express';
-import * as http from 'http';
+import * as request from 'supertest';
 import { createProxyMiddleware } from './test-kit';
 import { Options } from '../../src/index';
 
 describe('Usage in Express', () => {
   let app: express.Express;
-  let server: http.Server;
+  let agent: request.SuperTest<request.Test>;
 
   beforeEach(() => {
     app = express();
   });
 
-  afterEach(() => {
-    server?.close();
-  });
-
   // https://github.com/chimurai/http-proxy-middleware/issues/94
   describe('Express Sub Route', () => {
     beforeEach(() => {
@@ -43,19 +39,12 @@ describe('Usage in Express', () => {
       app.use('/sub', sub);
 
       // start server
-      server = app.listen(3000);
+      agent = request(app);
     });
 
-    // FIXME: flaky e2e test; caused by fixed port:3000
-    it('should still return a response when route does not match proxyConfig', (done) => {
-      let responseBody;
-      http.get('http://localhost:3000/sub/hello', (res) => {
-        res.on('data', (chunk) => {
-          responseBody = chunk.toString();
-          expect(responseBody).toBe('{"content":"foobar"}');
-          done();
-        });
-      });
+    it('should still return a response when route does not match proxyConfig', async () => {
+      const response = await agent.get('/sub/hello');
+      expect(response.body).toEqual({ content: 'foobar' });
     });
   });
 

test/e2e/router.spec.ts

@@ -2,6 +2,7 @@ import { createProxyMiddleware, createApp, createAppWithPath } from './test-kit'
 import { ErrorRequestHandler } from 'express';
 import * as request from 'supertest';
 import { getLocal, generateCACertificate, Mockttp } from 'mockttp';
+import * as getPort from 'get-port';
 
 const untrustedCACert = generateCACertificate({ bits: 1024 });
 
@@ -12,11 +13,19 @@ describe('E2E router', () => {
   let targetServerB: Mockttp;
   let targetServerC: Mockttp;
 
+  let targetPortA: number;
+  let targetPortB: number;
+  let targetPortC: number;
+
   beforeEach(async () => {
     targetServerA = getLocal({ https: await untrustedCACert });
     targetServerB = getLocal({ https: await untrustedCACert });
     targetServerC = getLocal({ https: await untrustedCACert });
 
+    targetPortA = await getPort();
+    targetPortB = await getPort();
+    targetPortC = await getPort();
+
     await targetServerA
       .anyRequest()
       .thenPassThrough({ ignoreHostCertificateErrors: ['localhost'] });
@@ -37,9 +46,9 @@ describe('E2E router', () => {
       .anyRequest()
       .thenCallback(({ protocol }) => ({ body: protocol === 'https' ? 'C' : 'NOT HTTPS C' }));
 
-    await targetServerA.start(6001);
-    await targetServerB.start(6002);
-    await targetServerC.start(6003);
+    await targetServerA.start(targetPortA);
+    await targetServerB.start(targetPortB);
+    await targetServerC.start(targetPortC);
   });
 
   afterEach(async () => {
@@ -52,11 +61,11 @@ describe('E2E router', () => {
     it('should work with a string', async () => {
       const app = createApp(
         createProxyMiddleware({
-          target: 'https://localhost:6001',
+          target: `https://localhost:${targetPortA}`,
           secure: false,
           changeOrigin: true,
           router(req) {
-            return 'https://localhost:6003';
+            return `https://localhost:${targetPortC}`;
           },
         })
       );
@@ -69,11 +78,11 @@ describe('E2E router', () => {
     it('should work with an object', async () => {
       const app = createApp(
         createProxyMiddleware({
-          target: 'https://localhost:6001',
+          target: `https://localhost:${targetPortA}`,
           secure: false,
           changeOrigin: true,
           router(req) {
-            return { host: 'localhost', port: 6003, protocol: 'https:' };
+            return { host: 'localhost', port: targetPortC, protocol: 'https:' };
           },
         })
       );
@@ -85,12 +94,12 @@ describe('E2E router', () => {
     it('should work with an async callback', async () => {
       const app = createApp(
         createProxyMiddleware({
-          target: 'https://localhost:6001',
+          target: `https://localhost:${targetPortA}`,
           secure: false,
           changeOrigin: true,
           router: async (req) => {
             return new Promise((resolve) =>
-              resolve({ host: 'localhost', port: 6003, protocol: 'https:' })
+              resolve({ host: 'localhost', port: targetPortC, protocol: 'https:' })
             );
           },
         })
@@ -104,7 +113,7 @@ describe('E2E router', () => {
     it('should handle promise rejection in router', async () => {
       const app = createApp(
         createProxyMiddleware({
-          target: 'https://localhost:6001',
+          target: `https://localhost:${targetPortA}`,
           secure: false,
           changeOrigin: true,
           router: async (req) => {
@@ -125,12 +134,12 @@ describe('E2E router', () => {
     it('missing a : will cause it to use http', async () => {
       const app = createApp(
         createProxyMiddleware({
-          target: 'https://localhost:6001',
+          target: `https://localhost:${targetPortA}`,
           secure: false,
           changeOrigin: true,
           router: async (req) => {
             return new Promise((resolve) =>
-              resolve({ host: 'localhost', port: 6003, protocol: 'https' })
+              resolve({ host: 'localhost', port: targetPortC, protocol: 'https' })
             );
           },
         })
@@ -149,13 +158,13 @@ describe('E2E router', () => {
       const app = createAppWithPath(
         '/',
         createProxyMiddleware({
-          target: 'https://localhost:6001',
+          target: `https://localhost:${targetPortA}`,
           secure: false,
           changeOrigin: true,
           router: {
-            'alpha.localhost:6000': 'https://localhost:6001',
-            'beta.localhost:6000': 'https://localhost:6002',
-            'localhost:6000/api': 'https://localhost:6003',
+            'alpha.localhost:6000': `https://localhost:${targetPortA}`,
+            'beta.localhost:6000': `https://localhost:${targetPortB}`,
+            'localhost:6000/api': `https://localhost:${targetPortC}`,
           },
         })
       );

test/e2e/websocket.spec.ts

@@ -1,6 +1,7 @@
 import * as http from 'http';
 import * as WebSocket from 'ws';
 import { Server as WebSocketServer } from 'ws';
+import * as getPort from 'get-port';
 import { createProxyMiddleware, createApp } from './test-kit';
 import type { RequestHandler } from '../../src/types';
 
@@ -14,9 +15,13 @@ describe('E2E WebSocket proxy', () => {
   let ws: WebSocket;
   let wss: WebSocketServer;
   let proxyMiddleware: RequestHandler;
-  const WS_SERVER_PORT = 9000;
+  let WS_SERVER_PORT: number;
+  let SERVER_PORT: number;
+
+  beforeEach(async () => {
+    WS_SERVER_PORT = await getPort();
+    SERVER_PORT = await getPort();
 
-  beforeEach(() => {
     wss = new WebSocketServer({ port: WS_SERVER_PORT });
 
     wss.on('connection', (websocket) => {
@@ -44,7 +49,6 @@ describe('E2E WebSocket proxy', () => {
 
   describe('option.ws', () => {
     beforeEach(async (done) => {
-      const SERVER_PORT = 31000;
       proxyServer = createApp(proxyMiddleware).listen(SERVER_PORT);
 
       // quick & dirty Promise version of http.get (don't care about correctness)
@@ -70,7 +74,6 @@ describe('E2E WebSocket proxy', () => {
 
   describe('option.ws with external server "upgrade"', () => {
     beforeEach((done) => {
-      const SERVER_PORT = 32000;
       proxyServer = createApp(proxyMiddleware).listen(SERVER_PORT);
       proxyServer.on('upgrade', proxyMiddleware.upgrade);
 
@@ -88,8 +91,6 @@ describe('E2E WebSocket proxy', () => {
   });
 
   describe('option.ws with external server "upgrade" and shorthand usage', () => {
-    const SERVER_PORT = 33000;
-
     beforeEach(() => {
       proxyServer = createApp(
         createProxyMiddleware(`ws://localhost:${WS_SERVER_PORT}`, {
@@ -115,8 +116,6 @@ describe('E2E WebSocket proxy', () => {
   });
 
   describe('with router and pathRewrite', () => {
-    const SERVER_PORT = 34000;
-
     beforeEach(() => {
       // override
       proxyServer = createApp(

yarn.lock

@@ -3040,6 +3040,11 @@ get-own-enumerable-property-symbols@^3.0.0:
   resolved "https://registry.yarnpkg.com/get-own-enumerable-property-symbols/-/get-own-enumerable-property-symbols-3.0.2.tgz#b5fde77f22cbe35f390b4e089922c50bce6ef664"
   integrity sha512-I0UBV/XOz1XkIJHEUDMZAbzCThU/H8DxmSfmdGcKPnVhu2VfFqr34jr9777IyaTYvxjedWhqVIilEDsCdP5G6g==
 
+get-port@^5.1.1:
+  version "5.1.1"
+  resolved "https://registry.yarnpkg.com/get-port/-/get-port-5.1.1.tgz#0469ed07563479de6efb986baf053dcd7d4e3193"
+  integrity sha512-g/Q1aTSDOxFpchXC4i8ZWvxA1lnPqx/JHqcpIw0/LX9T8x/GBbi6YnlN5nhaKIFkT8oFsscUKgDJYxfwfS6QsQ==
+
 [email protected]:
   version "8.0.0"
   resolved "https://registry.yarnpkg.com/get-stdin/-/get-stdin-8.0.0.tgz#cbad6a73feb75f6eeb22ba9e01f89aa28aa97a53"