Merge pull request #669 from gemini-testing/correct-handling-of-proce…

…ss-events

Correct exit on 'SIGHUP', 'SIGINT' or 'SIGTERM'

doc/events.md

@@ -9,3 +9,6 @@
  * `result.currentPath` - absolute path to the current image on your disk
  * `result.equal` - boolean value which is `true` when images are equal and `false` when aren't
  * `result.saveDiffTo` - function is responsible for building diff and present in the `result` only if images aren't equal
+
+* `INTERRUPT` - emitted on signal events `SIGHUP`, `SIGINT` or `SIGTERM`. The event is emitted with 1 argument `data`:
+ * `data.exitCode` - exit code with which gemini will be interrupted

lib/browser-pool/basic-pool.js

@@ -1,60 +1,53 @@
 'use strict';
-var util = require('util'),
- Browser = require('../browser'),
- Pool = require('./pool'),
- signalHandler = require('../signal-handler'),
- _ = require('lodash'),
- Promise = require('bluebird');
-
-var activeSessions = {};
-
-/**
- * @constructor
- * @extends BasicPool
- * @param {Config} config
- * @param {Calibrator} calibrator
- */
-function BasicPool(config, calibrator) {
- this._config = config;
- this._calibrator = calibrator;
-}
-
-util.inherits(BasicPool, Pool);
-
-Pool.prototype.getBrowser = function(id) {
- var _this = this,
- browser = Browser.create(this._config.forBrowser(id)),
- launchPromise = browser.launch(this._calibrator);
-
- activeSessions[browser.id] = {
- browser: browser,
- launchPromise: launchPromise
- };
-
- return launchPromise
- .then(browser.reset.bind(browser)).thenReturn(browser)
- .catch(function(e) {
- return _this.freeBrowser(browser)
- .then(function() {
- return Promise.reject(e);
- });
- });
-};
 
-Pool.prototype.freeBrowser = function(browser) {
- delete activeSessions[browser.id];
- return browser.quit();
+const Promise = require('bluebird');
+const _ = require('lodash');
+const Browser = require('../browser');
+const CancelledError = require('../errors/cancelled-error');
+const Pool = require('./pool');
+
+module.exports = class BasicPool extends Pool {
+ /**
+ * @constructor
+ * @extends Pool
+ * @param {Config} config
+ * @param {Calibrator} calibrator
+ */
+ constructor(config, calibrator) {
+ super();
+
+ this._config = config;
+ this._calibrator = calibrator;
+
+ this._activeSessions = {};
+ }
+
+ getBrowser(id) {
+ const browser = Browser.create(this._config.forBrowser(id));
+
+ return browser.launch(this._calibrator)
+ .then(() => {
+ if (this._cancelled) {
+ return Promise.reject(new CancelledError());
+ }
+
+ this._activeSessions[browser.sessionId] = browser;
+ })
+ .then(() => browser.reset())
+ .thenReturn(browser)
+ .catch((e) => this.freeBrowser(browser).then(() => Promise.reject(e)));
+ }
+
+ freeBrowser(browser) {
+ delete this._activeSessions[browser.sessionId];
+ return browser.quit();
+ }
+
+ cancel() {
+ this._cancelled = true;
+
+ _.forEach(this._activeSessions, (browser) => browser.quit());
+
+ this._activeSessions = {};
+ }
 };
-
-signalHandler.on('exit', function() {
- console.log('Killing browsers...');
- return _(activeSessions)
- .map(function(session) {
- var quit_ = session.browser.quit.bind(session.browser);
- return session.launchPromise.then(quit_);
- })
- .thru(Promise.all)
- .value();
-});
-
-module.exports = BasicPool;

lib/browser/new-browser.js

@@ -341,7 +341,7 @@ module.exports = class NewBrowser extends Browser {
  .then(() => this._wd.quit())
  .then(() => this.log('kill browser %o', this))
  .then(() => this._setHttpTimeout())
- .catch((err) => console.warn(err));
+ .catch((err) => this.log(err));
  }
 
  inspect() {

lib/cli/index.js

@@ -5,12 +5,15 @@ const program = require('commander');
 const Promise = require('bluebird');
 
 const Config = require('../config');
+const Events = require('../constants/events');
 const Gemini = require('../gemini');
 const pkg = require('../../package.json');
 const handleErrors = require('./errors').handleErrors;
 const checkForDeprecations = require('./deprecations').checkForDeprecations;
 const handleUncaughtExceptions = require('./errors').handleUncaughtExceptions;
 
+let exitCode;
+
 exports.run = () => {
  program
  .version(pkg.version)
@@ -86,7 +89,11 @@ function runGemini(method, paths, options) {
 
  return Promise.try(() => {
  checkForDeprecations();
- return new Gemini(program.config, {cli: true, env: true});
+
+ const gemini = new Gemini(program.config, {cli: true, env: true});
+ gemini.on(Events.INTERRUPT, (data) => exitCode = data.exitCode);
+
+ return gemini;
  })
  .then((gemini) => {
  return gemini[method](paths, {
@@ -109,5 +116,5 @@ function runGemini(method, paths, options) {
 }
 
 function exit(code) {
- process.on('exit', () => process.exit(code));
+ process.on('exit', () => process.exit(exitCode || code));
 }

lib/constants/events.js

@@ -33,5 +33,7 @@ module.exports = {
  UPDATE_RESULT: 'updateResult',
 
  BEFORE_FILE_READ: 'beforeFileRead',
- AFTER_FILE_READ: 'afterFileRead'
+ AFTER_FILE_READ: 'afterFileRead',
+
+ INTERRUPT: 'interrupt'
 };

lib/gemini.js

@@ -65,6 +65,13 @@ module.exports = class Gemini extends PassthroughEmitter {
 
  this._passThroughEvents(runner);
 
+ // it is important to require signal handler here in order to guarantee subscribing to "INTERRUPT" event
+ require('./signal-handler').on(Events.INTERRUPT, (data) => {
+ this.emit(Events.INTERRUPT, data);
+
+ runner.cancel();
+ });
+
  if (browsers) {
  this.checkUnknownBrowsers(browsers);
 

lib/reporters/html/view-model.js

@@ -49,28 +49,36 @@ module.exports = class ViewModel {
  * @param {TestStateResult} result
  */
  addFail(result) {
+ this._addFailResult(result);
+
+ this._counter.onFailed(result);
+ }
+
+ _addFailResult(result) {
  this._addTestResult(result, {
  fail: true,
  actualPath: lib.currentPath(result),
  expectedPath: lib.referencePath(result),
  diffPath: lib.diffPath(result)
  });
-
- this._counter.onFailed(result);
  }
 
  /**
  * @param {ErrorStateResult} result
  */
  addError(result) {
+ this._addErrorResult(result);
+
+ this._counter.onFailed(result);
+ }
+
+ _addErrorResult(result) {
  this._addTestResult(result, {
  actualPath: result.state ? lib.currentPath(result) : '',
  error: true,
  image: !!result.imagePath || !!result.currentPath,
  reason: (result.stack || result.message || result || '')
  });
-
- this._counter.onFailed(result);
  }
 
  /**
@@ -91,9 +99,9 @@ module.exports = class ViewModel {
  */
  addRetry(result) {
  if (result.hasOwnProperty('equal')) {
- this.addFail(result);
+ this._addFailResult(result);
  } else {
- this.addError(result);
+ this._addErrorResult(result);
  }
 
  this._counter.onRetry(result);

lib/runner/browser-runner/index.js

@@ -1,7 +1,6 @@
 'use strict';
 
 const _ = require('lodash');
-const Promise = require('bluebird');
 const promiseUtils = require('q-promise-utils');
 const BrowserAgent = require('./browser-agent');
 const Runner = require('../runner');
@@ -35,14 +34,9 @@ module.exports = class BrowserRunner extends Runner {
  }
 
  cancel() {
- this._runSuite = this._doNothing;
  this._suiteRunners.forEach((runner) => runner.cancel());
  }
 
- _doNothing() {
- return Promise.resolve();
- }
-
  _runSuites(suiteCollection, stateProcessor) {
  const suites = suiteCollection.clone().allSuites();
 

lib/runner/index.js

@@ -40,7 +40,7 @@ module.exports = class TestsRunner extends Runner {
  return Promise.resolve(this.emitAndWait(Events.START_RUNNER, this))
  .then(() => this.emit(Events.BEGIN, this._formatBeginEventData(suiteCollection)))
  .then(() => this._stateProcessor.prepare(this))
- .then(() => this._runTests(suiteCollection))
+ .then(() => !this._cancelled && this._runTests(suiteCollection))
  .then(() => this.coverage && this.coverage.processStats())
  .finally(() => {
  this.emit(Events.END);
@@ -76,8 +76,6 @@ module.exports = class TestsRunner extends Runner {
  _runTestsInBrowser(suiteCollection, browserId) {
  const runner = BrowserRunner.create(browserId, this.config, this._browserPool);
 
- this._browserRunners.push(runner);
-
  this.passthroughEvent(runner, [
  Events.RETRY,
  Events.START_BROWSER,
@@ -97,11 +95,8 @@ module.exports = class TestsRunner extends Runner {
  runner.on(Events.TEST_RESULT, (result) => this._handleResult(result, [Events.END_TEST, Events.TEST_RESULT]));
  runner.on(Events.UPDATE_RESULT, (result) => this._handleResult(result, Events.UPDATE_RESULT));
 
- return runner.run(suiteCollection, this._stateProcessor)
- .catch((e) => {
- this._cancel();
- return Promise.reject(e);
- });
+ this._browserRunners.push(runner);
+ return runner.run(suiteCollection, this._stateProcessor);
  }
 
  _handleResult(result, events) {
@@ -116,8 +111,11 @@ module.exports = class TestsRunner extends Runner {
  }
  }
 
- _cancel() {
+ cancel() {
+ this._cancelled = true;
+
  this._browserRunners.forEach((runner) => runner.cancel());
+
  this._browserPool.cancel();
  }
 };

lib/runner/suite-runner/insistent-suite-runner.js

@@ -20,10 +20,6 @@ module.exports = class InsistentSuiteRunner extends SuiteRunner {
  this._retriesPerformed = 0;
  }
 
- cancel() {
- this._suiteRunner.cancel();
- }
-
  _doRun(stateProcessor) {
  this._suiteRunner = this._initSuiteRunner();
  this._statesToRetry = [];
@@ -61,7 +57,7 @@ module.exports = class InsistentSuiteRunner extends SuiteRunner {
  }
 
  _retry(stateProcessor) {
- if (_.isEmpty(this._statesToRetry)) {
+ if (_.isEmpty(this._statesToRetry) || this._cancelled) {
  return;
  }
 

lib/runner/suite-runner/regular-suite-runner.js

@@ -53,14 +53,6 @@ module.exports = class RegularSuiteRunner extends SuiteRunner {
  });
  }
 
- cancel() {
- this._runStateInSession = this._doNothing;
- }
-
- _doNothing() {
- return Promise.resolve();
- }
-
  _processStates(stateProcessor) {
  const browser = this._session.browser;
 

lib/runner/suite-runner/suite-runner.js

@@ -33,7 +33,11 @@ var SuiteRunner = inherit(Runner, {
  throw new Error('Not implemented');
  },
 
- cancel: _.noop
+ cancel: function() {
+ this._cancelled = true;
+
+ this.emit = _.noop;
+ }
 });
 
 module.exports = SuiteRunner;

lib/signal-handler.js

@@ -1,30 +1,27 @@
 'use strict';
 
-var QEmitter = require('qemitter'),
- signalHandler = new QEmitter();
+const EventEmitter = require('events').EventEmitter;
+const Events = require('./constants/events');
+const logger = require('./utils').logger;
 
-module.exports = signalHandler;
+const signalHandler = module.exports = new EventEmitter();
 
-process.on('SIGHUP', notifyAndExit(1));
-process.on('SIGINT', notifyAndExit(2));
-process.on('SIGTERM', notifyAndExit(15));
+process.on('SIGHUP', handleSignal(1));
+process.on('SIGINT', handleSignal(2));
+process.on('SIGTERM', handleSignal(15));
 
-var callCount = 0;
+let callCount = 0;
 
-function notifyAndExit(signalNo) {
- var exitCode = 128 + signalNo;
+function handleSignal(signalNo) {
+ const exitCode = 128 + signalNo;
 
- return function() {
+ return () => {
  if (callCount++ > 0) {
- console.log('Force quit.');
+ logger.warn('Force quit.');
  process.exit(exitCode);
  }
 
- signalHandler.emitAndWait('exit')
- .then(function() {
- console.log('Done.');
- process.exit(exitCode);
- })
- .done();
+ logger.warn('Cancelling...');
+ signalHandler.emit(Events.INTERRUPT, {exitCode});
  };
 }