Permalink
Browse files

refactor unexpected shutdown, and fix lint so we can use upgraded app…

…ium eslint
1 parent ec7de82 commit 3db81cc19a22e7bd9dcab0affd0c6c511ed0839b @jlipps jlipps committed Dec 7, 2016
Showing with 70 additions and 16 deletions.
  1. +4 −1 gulpfile.js
  2. +25 −12 lib/appium.js
  3. +1 −1 lib/logsink.js
  4. +38 −1 test/driver-specs.js
  5. +2 −1 test/helpers.js
View
@@ -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 = '{}';
}
View
@@ -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)
View
@@ -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);
View
@@ -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({});
View
@@ -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;
});
}

0 comments on commit 3db81cc

Please sign in to comment.