From 6ad6ba4bb5995cfd6dd270595adef09eb32fea29 Mon Sep 17 00:00:00 2001 From: Andrey Belym Date: Wed, 28 Jun 2017 12:55:16 +0300 Subject: [PATCH] Handle SIGINT via async-exit-hook (closes #1378) --- bin/testcafe-with-v8-flag-filter.js | 2 +- package.json | 5 +- src/browser/connection/gateway.js | 5 +- src/browser/connection/index.js | 11 +++- src/browser/connection/status.js | 4 ++ .../provider/built-in/locally-installed.js | 4 -- src/browser/provider/built-in/path.js | 4 -- src/browser/provider/index.js | 48 +++++++++++------ src/browser/provider/plugin-host.js | 1 + src/cli/index.js | 53 ++++++++++++++++++- src/cli/log.js | 7 ++- src/client/browser/index.js | 18 +++++-- src/index.js | 7 ++- src/notifications/debug-logger.js | 2 +- src/testcafe.js | 6 +++ 15 files changed, 138 insertions(+), 39 deletions(-) create mode 100644 src/browser/connection/status.js diff --git a/bin/testcafe-with-v8-flag-filter.js b/bin/testcafe-with-v8-flag-filter.js index ea2ded01235..0cf63f43c31 100644 --- a/bin/testcafe-with-v8-flag-filter.js +++ b/bin/testcafe-with-v8-flag-filter.js @@ -5,4 +5,4 @@ var path = require('path'); var v8FlagsFilter = require('bin-v8-flags-filter'); -v8FlagsFilter(path.join(__dirname, '../lib/cli')); +v8FlagsFilter(path.join(__dirname, '../lib/cli'), { useShutdownMessage: true }); diff --git a/package.json b/package.json index 28837c3fcb8..4cdf3a706e7 100644 --- a/package.json +++ b/package.json @@ -54,6 +54,7 @@ "prepublish": "publish-please guard" }, "dependencies": { + "async-exit-hook": "^1.1.2", "babel-core": "^6.22.1", "babel-plugin-transform-class-properties": "^6.24.1", "babel-plugin-transform-runtime": "^6.22.0", @@ -61,7 +62,7 @@ "babel-preset-flow": "^6.23.0", "babel-preset-stage-2": "^6.22.0", "babel-runtime": "^6.22.0", - "bin-v8-flags-filter": "^1.0.0", + "bin-v8-flags-filter": "^1.1.2", "callsite": "^1.0.0", "callsite-record": "^4.0.0", "chai": "^3.0.0", @@ -78,7 +79,7 @@ "is-ci": "^1.0.10", "is-glob": "^2.0.1", "lodash": "^4.13.1", - "log-update": "^1.0.2", + "log-update-async-hook": "^2.0.2", "map-reverse": "^1.0.1", "mkdirp": "^0.5.1", "moment": "^2.10.3", diff --git a/src/browser/connection/gateway.js b/src/browser/connection/gateway.js index 6d5a70397ff..2b2f0a19334 100644 --- a/src/browser/connection/gateway.js +++ b/src/browser/connection/gateway.js @@ -75,8 +75,9 @@ export default class BrowserConnectionGateway { static onHeartbeat (req, res, connection) { if (BrowserConnectionGateway.ensureConnectionReady(res, connection)) { - connection.heartbeat(); - res.end(); + var status = connection.heartbeat(); + + respondWithJSON(res, status); } } diff --git a/src/browser/connection/index.js b/src/browser/connection/index.js index a7ba2e0c14f..7f7b70a335d 100644 --- a/src/browser/connection/index.js +++ b/src/browser/connection/index.js @@ -7,6 +7,7 @@ import { readSync as read } from 'read-file-relative'; import promisifyEvent from 'promisify-event'; import shortId from 'shortid'; import COMMAND from './command'; +import STATUS from './status'; import { GeneralError } from '../../errors/runtime'; import MESSAGE from '../../errors/runtime/message'; @@ -36,6 +37,7 @@ export default class BrowserConnection extends EventEmitter { this.provider = browserInfo.provider; this.permanent = permanent; + this.closing = false; this.closed = false; this.ready = false; this.opened = false; @@ -164,9 +166,11 @@ export default class BrowserConnection extends EventEmitter { } close () { - if (this.closed) + if (this.closed || this.closing) return; + this.closing = true; + this._closeBrowser() .then(() => { this.browserConnectionGateway.stopServingConnection(this); @@ -193,6 +197,11 @@ export default class BrowserConnection extends EventEmitter { heartbeat () { clearTimeout(this.heartbeatTimeout); this._waitForHeartbeat(); + + return { + code: this.closing ? STATUS.closing : STATUS.ok, + url: this.closing ? this.idleUrl : '' + }; } renderIdlePage () { diff --git a/src/browser/connection/status.js b/src/browser/connection/status.js new file mode 100644 index 00000000000..ce916980555 --- /dev/null +++ b/src/browser/connection/status.js @@ -0,0 +1,4 @@ +export default { + ok: 'ok', + closing: 'closing' +}; diff --git a/src/browser/provider/built-in/locally-installed.js b/src/browser/provider/built-in/locally-installed.js index 87466b840e5..229a8c6487a 100644 --- a/src/browser/provider/built-in/locally-installed.js +++ b/src/browser/provider/built-in/locally-installed.js @@ -17,10 +17,6 @@ export default { await browserTools.open(openParameters, pageUrl); }, - async closeBrowser (browserId) { - await browserTools.close(browserId); - }, - async isLocalBrowser () { return true; }, diff --git a/src/browser/provider/built-in/path.js b/src/browser/provider/built-in/path.js index 633704e84d0..c74ff40224f 100644 --- a/src/browser/provider/built-in/path.js +++ b/src/browser/provider/built-in/path.js @@ -55,10 +55,6 @@ export default { await browserTools.open(openParameters, pageUrl); }, - async closeBrowser (browserId) { - await browserTools.close(browserId); - }, - async isLocalBrowser () { return true; } diff --git a/src/browser/provider/index.js b/src/browser/provider/index.js index a82a9338398..3189f89fdb4 100644 --- a/src/browser/provider/index.js +++ b/src/browser/provider/index.js @@ -48,10 +48,10 @@ function subtractSizes (sizeA, sizeB) { export default class BrowserProvider { constructor (plugin) { - this.plugin = plugin; - this.initPromise = Promise.resolve(false); + this.plugin = plugin; + this.initPromise = Promise.resolve(false); - this.isMultiBrowser = this.plugin.isMultiBrowser; + this.isMultiBrowser = this.plugin.isMultiBrowser; // HACK: The browser window has different border sizes in normal and maximized modes. So, we need to be sure that the window is // not maximized before resizing it in order to keep the mechanism of correcting the client area size working. When browser is started, // we are resizing it for the first time to switch the window to normal mode, and for the second time - to restore the client area size. @@ -195,7 +195,17 @@ export default class BrowserProvider { } async closeBrowser (browserId) { - await this.plugin.closeBrowser(browserId); + var isLocalBrowser = await this.plugin.isLocalBrowser(browserId); + var customActionsInfo = await this.plugin.hasCustomActionForBrowser(browserId); + var hasCustomCloseBrowser = customActionsInfo.hasCloseBrowser; + var usePluginsCloseBrowser = hasCustomCloseBrowser || !isLocalBrowser; + + if (usePluginsCloseBrowser) { + await this.plugin.closeBrowser(browserId); + return; + } + + await browserTools.close(this.windowDescriptors[browserId]); } async getBrowserList () { @@ -207,10 +217,12 @@ export default class BrowserProvider { } async resizeWindow (browserId, width, height, currentWidth, currentHeight) { - var isLocalBrowser = await this.plugin.isLocalBrowser(browserId); - var supportedFeatures = await this.plugin.hasCustomActionForBrowser(browserId); + var isLocalBrowser = await this.plugin.isLocalBrowser(browserId); + var customActionsInfo = await this.plugin.hasCustomActionForBrowser(browserId); + var hasCustomResizeWindow = customActionsInfo.hasResizeWindow; + - if (isLocalBrowser && !supportedFeatures.hasResizeWindow) { + if (isLocalBrowser && !hasCustomResizeWindow) { await this._resizeLocalBrowserWindow(browserId, width, height, currentWidth, currentHeight); return; } @@ -219,30 +231,34 @@ export default class BrowserProvider { } async canResizeWindowToDimensions (browserId, width, height) { - var isLocalBrowser = await this.plugin.isLocalBrowser(browserId); - var supportedFeatures = await this.plugin.hasCustomActionForBrowser(browserId); + var isLocalBrowser = await this.plugin.isLocalBrowser(browserId); + var customActionsInfo = await this.plugin.hasCustomActionForBrowser(browserId); + var hasCustomCanResizeToDimensions = customActionsInfo.hasCanResizeWindowToDimensions; + - if (isLocalBrowser && !supportedFeatures.hasCanResizeWindowToDimensions) + if (isLocalBrowser && !hasCustomCanResizeToDimensions) return await this._canResizeLocalBrowserWindowToDimensions(browserId, width, height); return await this.plugin.canResizeWindowToDimensions(browserId, width, height); } async maximizeWindow (browserId) { - var isLocalBrowser = await this.plugin.isLocalBrowser(browserId); - var supportedFeatures = await this.plugin.hasCustomActionForBrowser(browserId); + var isLocalBrowser = await this.plugin.isLocalBrowser(browserId); + var customActionsInfo = await this.plugin.hasCustomActionForBrowser(browserId); + var hasCustomMaximizeWindow = customActionsInfo.hasMaximizeWindow; - if (isLocalBrowser && !supportedFeatures.hasCanResizeWindowToDimensions) + if (isLocalBrowser && !hasCustomMaximizeWindow) return await this._maximizeLocalBrowserWindow(browserId); return await this.plugin.maximizeWindow(browserId); } async takeScreenshot (browserId, screenshotPath, pageWidth, pageHeight) { - var isLocalBrowser = await this.plugin.isLocalBrowser(browserId); - var supportedFeatures = await this.plugin.hasCustomActionForBrowser(browserId); + var isLocalBrowser = await this.plugin.isLocalBrowser(browserId); + var customActionsInfo = await this.plugin.hasCustomActionForBrowser(browserId); + var hasCustomTakeScreenshot = customActionsInfo.hasTakeScreenshot; - if (isLocalBrowser && !supportedFeatures.hasTakeScreenshot) { + if (isLocalBrowser && !hasCustomTakeScreenshot) { await this._takeLocalBrowserScreenshot(browserId, screenshotPath, pageWidth, pageHeight); return; } diff --git a/src/browser/provider/plugin-host.js b/src/browser/provider/plugin-host.js index 4f480461d5f..97f6048de54 100644 --- a/src/browser/provider/plugin-host.js +++ b/src/browser/provider/plugin-host.js @@ -87,6 +87,7 @@ export default class BrowserProviderPluginHost { async hasCustomActionForBrowser (/* browserId */) { return { + hasCloseBrowser: this.hasOwnProperty('closeBrowser'), hasResizeWindow: this.hasOwnProperty('resizeWindow'), hasTakeScreenshot: this.hasOwnProperty('takeScreenshot'), hasCanResizeWindowToDimensions: this.hasOwnProperty('canResizeWindowToDimensions'), diff --git a/src/cli/index.js b/src/cli/index.js index d8a37043c64..b8b11810b98 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -1,6 +1,6 @@ +import fs from 'fs'; import chalk from 'chalk'; import resolveCwd from 'resolve-cwd'; -import fs from 'fs'; import browserProviderPool from '../browser/provider/pool'; import { GeneralError, APIError } from '../errors/runtime'; import MESSAGE from '../errors/runtime/message'; @@ -9,8 +9,53 @@ import log from './log'; import remotesWizard from './remotes-wizard'; import createTestCafe from '../'; + +const TERMINATION_TYPES = { + sigint: 'sigint', + sigbreak: 'sigbreak', + shutdown: 'shutdown' +}; + +var showMessageOnExit = true; +var exitMessageShown = false; +var exiting = false; + +var handledSignalsCount = { + [TERMINATION_TYPES.sigint]: 0, + [TERMINATION_TYPES.sigbreak]: 0, + [TERMINATION_TYPES.shutdown]: 0 +}; + +function exitHandler (terminationType) { + handledSignalsCount[terminationType]++; + + if (showMessageOnExit && !exitMessageShown) { + exitMessageShown = true; + + log.hideSpinner(); + log.write('Stopping TestCafe...'); + log.showSpinner(); + + process.on('exit', () => log.hideSpinner(true)); + } + + if (exiting || handledSignalsCount[terminationType] < 2) + return; + + exiting = true; + + exit(0); +} + +function setupExitHandler () { + process.on('SIGINT', () => exitHandler(TERMINATION_TYPES.sigint)); + process.on('SIGBREAK', () => exitHandler(TERMINATION_TYPES.sigbreak)); + + process.on('message', message => message === 'shutdown' && exitHandler(TERMINATION_TYPES.shutdown)); +} + function exit (code) { - log.hideSpinner(); + log.hideSpinner(true); // NOTE: give a process time to flush the output. // It's necessary in some environments. @@ -78,6 +123,7 @@ async function runTests (argParser) { } finally { + showMessageOnExit = false; await testCafe.close(); } @@ -123,6 +169,8 @@ function useLocalInstallation () { if (useLocalInstallation()) return; + setupExitHandler(exitHandler); + try { var argParser = new CliArgumentParser(); @@ -134,6 +182,7 @@ function useLocalInstallation () { await runTests(argParser); } catch (err) { + showMessageOnExit = false; error(err); } })(); diff --git a/src/cli/log.js b/src/cli/log.js index a55bb8baa72..ab0026ec4fd 100644 --- a/src/cli/log.js +++ b/src/cli/log.js @@ -1,6 +1,6 @@ import tty from 'tty'; import elegantSpinner from 'elegant-spinner'; -import logUpdate from 'log-update'; +import logUpdate from 'log-update-async-hook'; import chalk from 'chalk'; import isCI from 'is-ci'; @@ -24,11 +24,14 @@ export default { } }, - hideSpinner () { + hideSpinner (isExit) { if (this.animation) { clearInterval(this.animation); logUpdate.stderr.clear(); + if (isExit) + logUpdate.stderr.done(); + this.animation = null; } }, diff --git a/src/client/browser/index.js b/src/client/browser/index.js index 101adf2bc19..8db1adb48ba 100644 --- a/src/client/browser/index.js +++ b/src/client/browser/index.js @@ -1,8 +1,9 @@ // TODO: once we'll have client commons load it from there instead of node modules (currently it's leads to two copies of this packages on client) import Promise from 'pinkie'; import COMMAND from '../../browser/connection/command'; +import STATUS from '../../browser/connection/status'; -const HEARTBEAT_INTERVAL = 30 * 1000; +const HEARTBEAT_INTERVAL = 2 * 1000; var allowInitScriptExecution = false; @@ -37,8 +38,19 @@ function isCurrentLocation (url) { //API export function startHeartbeat (heartbeatUrl, createXHR) { - sendXHR(heartbeatUrl, createXHR); - window.setInterval(() => sendXHR(heartbeatUrl, createXHR), HEARTBEAT_INTERVAL); + function heartbeat () { + sendXHR(heartbeatUrl, createXHR) + .then(status => { + if (status.code === STATUS.closing && !isCurrentLocation(status.url)) { + stopInitScriptExecution(); + document.location = status.url; + } + }); + } + + window.setInterval(heartbeat, HEARTBEAT_INTERVAL); + + heartbeat(); } function executeInitScript (initScriptUrl, createXHR) { diff --git a/src/index.js b/src/index.js index e312599808a..b03ebd9c922 100644 --- a/src/index.js +++ b/src/index.js @@ -1,6 +1,7 @@ import Promise from 'pinkie'; import TestCafe from './testcafe'; import * as endpointUtils from 'endpoint-utils'; +import setupExitHook from 'async-exit-hook'; import { GeneralError } from './errors/runtime'; import MESSAGE from './errors/runtime/message'; import embeddingUtils from './embedding-utils'; @@ -42,7 +43,11 @@ async function createTestCafe (hostname, port1, port2) { getValidPort(port2) ]); - return new TestCafe(hostname, port1, port2); + var testcafe = new TestCafe(hostname, port1, port2); + + setupExitHook(cb => testcafe.close().then(cb)); + + return testcafe; } // Embedding utils diff --git a/src/notifications/debug-logger.js b/src/notifications/debug-logger.js index 3fd89144810..95c4bbbd10e 100644 --- a/src/notifications/debug-logger.js +++ b/src/notifications/debug-logger.js @@ -1,6 +1,6 @@ import chalk from 'chalk'; import { findIndex } from 'lodash'; -import logUpdate from 'log-update'; +import logUpdate from 'log-update-async-hook'; import createStackFilter from '../errors/create-stack-filter'; export default { diff --git a/src/testcafe.js b/src/testcafe.js index c8b88590bf0..253a3332a50 100644 --- a/src/testcafe.js +++ b/src/testcafe.js @@ -19,6 +19,7 @@ const FAVICON = read('./client/ui/favicon.ico', true); export default class TestCafe { constructor (hostname, port1, port2) { + this.closed = false; this.proxy = new Proxy(hostname, port1, port2); this.browserConnectionGateway = new BrowserConnectionGateway(this.proxy); this.runners = []; @@ -62,6 +63,11 @@ export default class TestCafe { } async close () { + if (this.closed) + return; + + this.closed = true; + await Promise.all(this.runners.map(runner => runner.stop())); await browserProviderPool.dispose();