diff --git a/gulpfile.js b/gulpfile.js index 961f461e9..83dde7adb 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -1,4 +1,5 @@ /* eslint no-console:0 */ +/* eslint-disable promise/prefer-await-to-callbacks */ "use strict"; // turn all logging on since we have tests that rely on npmlog logs actually @@ -32,10 +33,12 @@ gulp.task('fixShrinkwrap', function (done) { boilerplate({ build: 'appium', jscs: false, + jshint: false, test: { files: ['${testDir}/**/*-specs.js'] }, extraPrepublishTasks: ['fixShrinkwrap'], + preCommitTasks: ['eslint', 'once'], }); // generates server arguments readme @@ -66,7 +69,7 @@ gulp.task('docs', ['transpile'], function () { } // handle empty objects - if (JSON.stringify(argOpts.defaultValue) === '{}'){ + if (JSON.stringify(argOpts.defaultValue) === '{}') { argOpts.defaultValue = '{}'; } diff --git a/lib/appium.js b/lib/appium.js index 599c8ba0f..89552f47f 100644 --- a/lib/appium.js +++ b/lib/appium.js @@ -150,18 +150,11 @@ class AppiumDriver extends BaseDriver { let [innerSessionId, dCaps] = await d.createSession(caps, reqCaps, curSessions); this.sessions[innerSessionId] = d; - // Remove the session on unexpected shutdown, so that we are in a position - // to open another session later on. - // TODO: this should be removed and replaced by a onShutdown callback. - d.onUnexpectedShutdown - .then(() => { throw new Error('Unexpected shutdown'); }) - .catch(B.CancellationError, () => {}) - .catch((err) => { - log.warn(`Closing session, cause was '${err.message}'`); - log.info(`Removing session ${innerSessionId} from our master session list`); - delete this.sessions[innerSessionId]; - }) - .done(); + // this is an async function but we don't await it because it handles + // an out-of-band promise which is fulfilled if the inner driver + // unexpectedly shuts down + this.attachUnexpectedShutdownHandler(d, innerSessionId); + log.info(`New ${InnerDriver.name} session created successfully, session ` + `${innerSessionId} added to master session list`); @@ -172,6 +165,26 @@ class AppiumDriver extends BaseDriver { return [innerSessionId, dCaps]; } + async attachUnexpectedShutdownHandler (driver, innerSessionId) { + // Remove the session on unexpected shutdown, so that we are in a position + // to open another session later on. + // TODO: this should be removed and replaced by a onShutdown callback. + try { + await driver.onUnexpectedShutdown; // this is a cancellable promise + // if we get here, we've had an unexpected shutdown, so error + throw new Error('Unexpected shutdown'); + } catch (e) { + if (e instanceof B.CancellationError) { + // if we cancelled the unexpected shutdown promise, that means we + // no longer care about it, and can safely ignore it + return; + } + log.warn(`Closing session, cause was '${e.message}'`); + log.info(`Removing session ${innerSessionId} from our master session list`); + delete this.sessions[innerSessionId]; + } + } + curSessionDataForDriver (InnerDriver) { let data = _.values(this.sessions) .filter((s) => s.constructor.name === InnerDriver.name) diff --git a/lib/logsink.js b/lib/logsink.js index e20e486f1..53f869ce5 100644 --- a/lib/logsink.js +++ b/lib/logsink.js @@ -53,7 +53,7 @@ function timestamp () { // having to create 2 loggers. function applyStripColorPatch (transport) { let _log = transport.log.bind(transport); - transport.log = function (level, msg, meta, callback) { + transport.log = function (level, msg, meta, callback) { // eslint-disable-line promise/prefer-await-to-callbacks let code = /\u001b\[(\d+(;\d+)*)?m/g; msg = ('' + msg).replace(code, ''); _log(level, msg, meta, callback); diff --git a/test/driver-specs.js b/test/driver-specs.js index 666ef58f2..080cbe368 100644 --- a/test/driver-specs.js +++ b/test/driver-specs.js @@ -9,8 +9,9 @@ import chai from 'chai'; import chaiAsPromised from 'chai-as-promised'; import { XCUITestDriver } from 'appium-xcuitest-driver'; import { IosDriver } from 'appium-ios-driver'; +import { sleep } from 'asyncbox'; - +chai.should(); chai.use(chaiAsPromised); const BASE_CAPS = {platformName: 'Fake', deviceName: 'Fake', app: TEST_FAKE_APP}; @@ -172,6 +173,42 @@ describe('AppiumDriver', () => { }); describe('sessionExists', () => { }); + describe('attachUnexpectedShutdownHandler', () => { + let appium + , mockFakeDriver; + beforeEach(() => { + [appium, mockFakeDriver] = getDriverAndFakeDriver(); + }); + afterEach(() => { + mockFakeDriver.restore(); + appium.args.defaultCapabilities = {}; + }); + + it('should remove session if inner driver unexpectedly exits with an error', async () => { + let [sessionId,] = await appium.createSession(_.clone(BASE_CAPS)); // eslint-disable-line comma-spacing + _.keys(appium.sessions).should.contain(sessionId); + appium.sessions[sessionId].unexpectedShutdownDeferred.reject(new Error("Oops")); + // let event loop spin so rejection is handled + await sleep(1); + _.keys(appium.sessions).should.not.contain(sessionId); + }); + it('should remove session if inner driver unexpectedly exits with no error', async () => { + let [sessionId,] = await appium.createSession(_.clone(BASE_CAPS)); // eslint-disable-line comma-spacing + _.keys(appium.sessions).should.contain(sessionId); + appium.sessions[sessionId].unexpectedShutdownDeferred.resolve(); + // let event loop spin so rejection is handled + await sleep(1); + _.keys(appium.sessions).should.not.contain(sessionId); + }); + it('should not remove session if inner driver cancels unexpected exit', async () => { + let [sessionId,] = await appium.createSession(_.clone(BASE_CAPS)); // eslint-disable-line comma-spacing + _.keys(appium.sessions).should.contain(sessionId); + appium.sessions[sessionId].onUnexpectedShutdown.cancel(); + // let event loop spin so rejection is handled + await sleep(1); + _.keys(appium.sessions).should.contain(sessionId); + }); + }); describe('getDriverForCaps', () => { it('should not blow up if user does not provide platformName', () => { let appium = new AppiumDriver({}); diff --git a/test/helpers.js b/test/helpers.js index 2dcbdc163..2e6e1f98a 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -1,5 +1,6 @@ import path from 'path'; import wd from 'wd'; +import B from 'bluebird'; const TEST_HOST = 'localhost'; const TEST_PORT = 4723; @@ -18,7 +19,7 @@ function initSession (caps) { after(async () => { await driver.quit(); }); - return new Promise((_resolve) => { + return new B((_resolve) => { resolve = _resolve; }); }