Merge pull request #25 from NimaSoroush/getport

Add dynamic port allocation for chrome instances

Author: NimaSoroush

CHANGELOG.md

@@ -1,3 +1,7 @@
+## [0.0.14] - 2017-08-01
+### Added
+- Assign free ports to run chrome instances
+
 ## [0.0.13] - 2017-07-25
 ### Added
 - Updating readme file

package.json

@@ -1,6 +1,6 @@
 {
   "name": "differencify",
-  "version": "0.0.13",
+  "version": "0.0.14",
   "description": "Perceptual diffing tool",
   "main": "dist/index.js",
   "scripts": {
@@ -50,6 +50,7 @@
     "check-types": "^7.2.0",
     "chromy": "^0.3.6",
     "fs": "0.0.1-security",
+    "get-port": "^3.1.0",
     "jimp": "^0.2.28"
   },
   "jest": {

src/index.js

@@ -1,13 +1,13 @@
 import 'babel-polyfill';
 import fs from 'fs';
 import Chromy from 'chromy';
+import getPort from 'get-port';
 import { sanitiseGlobalConfiguration, sanitiseTestConfiguration } from './sanitiser';
 import run from './chromyRunner';
 import logger from './logger';
 import { configTypes } from './defaultConfig';
 import actions from './actions';
 
-const CHROME_PORT = 9222;
 const CHROME_WIDTH = 800;
 const CHROME_HEIGHT = 600;
 
@@ -18,73 +18,84 @@ const createDir = (path) => {
   }
 };
 
+const getFreePort = async () => {
+  try {
+    return await getPort();
+  } catch (error) {
+    logger.error('Failed to get a free port', error);
+    return null;
+  }
+};
+
 export default class Differencify {
   constructor(conf) {
     this.configuration = sanitiseGlobalConfiguration(conf);
     this.chromeInstances = {};
-    this.chromeInstancesId = CHROME_PORT;
     if (this.configuration.debug === true) {
       logger.enable();
     }
     createDir(this.configuration.screenshots);
     createDir(this.configuration.testReportPath);
   }
 
-  _createChromeInstance(testConfig) {
+  async _createChromeInstance(testConfig) {
     const width = testConfig.resolution.width || CHROME_WIDTH;
     const height = testConfig.resolution.height || CHROME_HEIGHT;
     const flags = [`--window-size=${width},${height}`];
+    const port = await getFreePort();
+    if (!port) {
+      return null;
+    }
     const chromy = new Chromy({
+      port,
       chromeFlags: flags,
-      port: this.chromeInstancesId,
       waitTimeout: this.configuration.timeout,
       visible: this.configuration.visible,
     });
     return chromy;
   }
 
-  _updateChromeInstances(id, chromy) {
-    this.chromeInstances[id] = chromy;
-    this.chromeInstancesId += 1;
+  _updateChromeInstances(chromy) {
+    this.chromeInstances[chromy.options.port] = chromy;
   }
 
-  async _closeChrome(id, chromy) {
+  async _closeChrome(chromy, testName) {
     try {
-      logger.log('closing browser');
+      logger.prefix(testName).log('closing browser');
       await chromy.close();
-      delete this.chromeInstances[id];
+      delete this.chromeInstances[chromy.options.port];
     } catch (error) {
-      logger.error(error);
+      logger.prefix(testName).log(error);
     }
   }
 
-  async _run(config, type, step) {
+  async _run(config, type) {
     const testConfig = sanitiseTestConfiguration(config);
-    const chromy = this._createChromeInstance(testConfig);
-    const testId = this.chromeInstancesId;
-    this._updateChromeInstances(testId, chromy);
-    testConfig.type = type;
-    if (step) {
-      testConfig.steps.push(step);
+    const chromy = await this._createChromeInstance(testConfig);
+    if (!chromy) {
+      return false;
     }
+    this._updateChromeInstances(chromy);
+    testConfig.type = type;
     const result = await run(chromy, this.configuration, testConfig);
-    await this._closeChrome(testId, chromy);
+    await this._closeChrome(chromy, testConfig.name);
     return result;
   }
 
   async update(config) {
-    return await this._run(config, configTypes.update, null);
+    return await this._run(config, configTypes.update);
   }
 
   async test(config) {
-    const testStep = { name: actions.test, value: this.configuration.testReportPath };
-    return await this._run(config, configTypes.test, testStep);
+    config.steps.push({ name: actions.test, value: this.configuration.testReportPath });
+    return await this._run(config, configTypes.test);
   }
 
   async cleanup() {
     await Promise.all(
       Object.values(this.chromeInstances).map(chromeInstance => chromeInstance.close()),
     );
+    this.chromeInstances = {};
     logger.log('All browsers been closed');
   }
 }

src/index.test.js

@@ -1,40 +1,33 @@
 import fs from 'fs';
 import Chromy from 'chromy';
+import getPort from 'get-port';
 import Differencify from './index';
 import logger from './logger';
+import run from './chromyRunner';
 
-let chromyCloseCallsCounter = 0;
-jest.mock('chromy', () => jest.fn(() =>
-    ({
-      goto: jest.fn(),
-      close: jest.fn(() => { chromyCloseCallsCounter += 1; }),
-      screenshotDocument: jest.fn(() => 'png file'),
-      screenshotSelector: jest.fn(() => 'png file'),
-    }),
-  ));
+jest.mock('get-port', () => jest.fn(() => 3000));
+const mockClose = jest.fn();
+jest.mock('chromy', () => () =>
+  ({
+    close: mockClose,
+    options: { port: 3000 },
+  }));
+jest.mock('./chromyRunner', () => jest.fn(() => true));
 
-jest.mock('./compareImage', () => jest.fn(arg =>
-    new Promise((resolve, reject) => {
-      if (arg.screenshots === 'screenshots') {
-        return resolve('Saving the diff image to disk');
-      }
-      return reject('error');
-    }),
-  ));
+jest.mock('fs', () => ({
+  mkdirSync: jest.fn(),
+  existsSync: jest.fn(),
+}));
 
-let writeFileSyncCalls = [];
-fs.writeFileSync = (...args) => {
-  writeFileSyncCalls.push(...args);
-};
-fs.mkdirSync = (...args) => {
-  writeFileSyncCalls.push(...args);
-};
-
-let loggerCalls = [];
-logger.prefix = () => logger;
-logger.log = (...args) => {
-  loggerCalls.push(...args);
-};
+const mockLog = jest.fn();
+jest.mock('./logger', () => ({
+  prefix: jest.fn(() => ({
+    log: mockLog,
+  })),
+  log: jest.fn(),
+  error: jest.fn(),
+  enable: jest.fn(),
+}));
 
 const globalConfig = {
   screenshots: 'screenshots',
@@ -59,35 +52,53 @@ const differencify = new Differencify(globalConfig);
 
 describe('Differencify', () => {
   afterEach(() => {
-    loggerCalls = [];
-    writeFileSyncCalls = [];
-    chromyCloseCallsCounter = 0;
+    mockLog.mockClear();
+    logger.log.mockClear();
+    run.mockClear();
+    mockClose.mockClear();
+  });
+  it('constructor fn', async () => {
+    expect(fs.mkdirSync).toHaveBeenCalledWith('screenshots');
+    expect(fs.mkdirSync).toHaveBeenCalledWith('./differencify_report');
   });
   it('update fn', async () => {
     const result = await differencify.update(testConfig);
     expect(result).toEqual(true);
-    expect(differencify.chromeInstancesId).toEqual(9223);
-    expect(loggerCalls[0]).toEqual('goto -> www.example.com');
-    expect(loggerCalls[1]).toEqual('capturing screenshot of whole DOM');
-    expect(loggerCalls[2]).toEqual('screenshot saved in -> screenshots/default.png');
-    expect(writeFileSyncCalls).toEqual(['screenshots', './differencify_report', 'screenshots/default.png', 'png file']);
+    expect(differencify.chromeInstances[3000]).toEqual(undefined);
+    expect(mockClose).toHaveBeenCalledTimes(1);
+    expect(mockLog).toHaveBeenCalledWith('closing browser');
+    expect(run).toHaveBeenCalledTimes(1);
   });
   it('test fn', async () => {
     const result = await differencify.test(testConfig);
     expect(result).toEqual(true);
-    expect(differencify.chromeInstancesId).toEqual(9224);
-    expect(loggerCalls[0]).toEqual('goto -> www.example.com');
-    expect(loggerCalls[1]).toEqual('capturing screenshot of whole DOM');
-    expect(loggerCalls[2]).toEqual('screenshot saved in -> ./differencify_report/default.png');
-    expect(writeFileSyncCalls).toEqual(['./differencify_report/default.png', 'png file']);
+    expect(differencify.chromeInstances[3000]).toEqual(undefined);
+    expect(mockClose).toHaveBeenCalledTimes(1);
+    expect(mockLog).toHaveBeenCalledWith('closing browser');
+    expect(run).toHaveBeenCalledTimes(1);
+  });
+  it('chromyRunner will fail test fn', async () => {
+    run.mockReturnValueOnce(Promise.resolve(false));
+    const result = await differencify.test(testConfig);
+    expect(result).toEqual(false);
+    expect(differencify.chromeInstances[3000]).toEqual(undefined);
+    expect(mockClose).toHaveBeenCalledTimes(1);
+    expect(mockLog).toHaveBeenCalledWith('closing browser');
+    expect(run).toHaveBeenCalledTimes(1);
   });
   it('cleanup fn', async () => {
     const chromy1 = new Chromy();
-    differencify.chromeInstances[1] = chromy1;
     const chromy2 = new Chromy();
-    differencify.chromeInstances[2] = chromy2;
+    differencify.chromeInstances = [chromy1, chromy2];
     await differencify.cleanup();
-    expect(chromyCloseCallsCounter).toEqual(2);
-    expect(loggerCalls[0]).toEqual('All browsers been closed');
+    expect(mockClose).toHaveBeenCalledTimes(2);
+    expect(differencify.chromeInstances).toEqual({});
+    expect(logger.log).toHaveBeenCalledWith('All browsers been closed');
+  });
+  it('get-port fails test', async () => {
+    getPort.mockReturnValueOnce(Promise.reject());
+    const result = await differencify.test(testConfig);
+    expect(result).toEqual(false);
+    expect(logger.error).toHaveBeenCalledWith('Failed to get a free port', undefined);
   });
 });