From b537c4ae711a5725f42175dcc8f1a46dde19e443 Mon Sep 17 00:00:00 2001 From: Jonathan Lipps Date: Fri, 25 Oct 2013 15:46:48 -0700 Subject: [PATCH] remove confusing multiple session handling code it doesn't actually do anything and was cruft also, make session overriding non-default based on the mobile JSONWP discussions --- docs/server-args.md | 4 +- lib/appium.js | 128 +++++++++++---------------------------- lib/server/controller.js | 2 +- lib/server/parser.js | 6 +- test/unit/queue.js | 21 ++++--- 5 files changed, 54 insertions(+), 107 deletions(-) diff --git a/docs/server-args.md b/docs/server-args.md index 9372807a16a..cd451b896bd 100644 --- a/docs/server-args.md +++ b/docs/server-args.md @@ -16,8 +16,8 @@ All flags are optional, but some are required in conjunction with certain others |`-p`, `--port`|4723|port to listen on|`--port 4723`| |`-dp`, `--device-port`|4724|(Android-only) port to connect to device on|`--device-port 4724`| |`-k`, `--keep-artifacts`|false|(IOS-only) Keep Instruments trace directories|| -|`--no-session-override`|false|Disables session override|| -|`--full-reset`|false|(Android-only) Reset app state by uninstalling app instead of using clean.apk|| +|`--session-override`|false|Enables session override (clobbering)|| +|`--full-reset`|false|(Android-only) Reset app state by uninstalling app instead of clearing app data|| |`--no-reset`|false|Don't reset app state between sessions (IOS: don't delete app plist files; Android: don't uninstall app before new session)|| |`-l`, `--pre-launch`|false|Pre-launch the application before allowing the first session (Requires --app and, for Android, --app-pkg and --app-activity)|| |`-g`, `--log`|null|Log output to this file instead of stdout|`--log /path/to/appium.log`| diff --git a/lib/appium.js b/lib/appium.js index 1ceb8a5f8f5..65e9ecbc0e3 100644 --- a/lib/appium.js +++ b/lib/appium.js @@ -47,19 +47,15 @@ var Appium = function(args) { } this.rest = null; this.webSocket = null; - this.devices = {}; this.deviceType = null; this.device = null; this.sessionId = null; this.desiredCapabilities = {}; - this.sessions = []; - this.counter = -1; - this.progress = -1; + this.session = null; this.tempFiles = []; - this.origApp = null; this.preLaunched = false; this.fastReset = !this.args.fullReset && !this.args.noReset; - this.sessionOverride = !this.args.noSessionOverride; + this.sessionOverride = this.args.sessionOverride; this.resetting = false; }; @@ -100,15 +96,13 @@ Appium.prototype.preLaunch = function(cb) { }; Appium.prototype.start = function(desiredCaps, cb) { - this.origApp = this.args.app; this.desiredCapabilities = desiredCaps; this.configure(desiredCaps, function(err) { if (err) { logger.info("Got configuration error, not starting session"); cb(err, null); } else { - this.sessions[++this.counter] = { sessionId: '', callback: cb }; - this.clearPreviousSession(); + this.invoke(cb); } }.bind(this)); }; @@ -138,6 +132,8 @@ Appium.prototype.getDeviceType = function(desiredCaps) { return "android"; } } else if (desiredCaps.browserName) { + logger.warn("WARNING: use of browserName is deprecated. Please migrate " + + "your tests"); if (desiredCaps.browserName[0].toLowerCase() === "i") { return "ios"; } else if (desiredCaps.browserName.toLowerCase() === "safari") { @@ -445,35 +441,18 @@ Appium.prototype.unzipApp = function(zipPath, cb) { }.bind(this)); }; -Appium.prototype.clearPreviousSession = function() { - if (this.sessionOverride && this.device) { - logger.info("Clearing out previous session"); - this.device.stop(function() { - this.devices = []; - this.device = null; - this.sessions[this.progress] = {}; - this.invoke(); - }.bind(this)); - } else { - this.invoke(); - } -}; - -Appium.prototype.invoke = function() { - if (this.progress >= this.counter) { - return; - } +Appium.prototype.invoke = function(cb) { if (this.sessionOverride || this.sessionId === null) { this.sessionId = UUID.create().hex; logger.info('Creating new appium session ' + this.sessionId); - if (typeof this.devices[this.deviceType] === 'undefined') { + if (this.device === null || this.sessionOverride) { if (this.isMockIos()) { var device = ios({rest: this.rest}); device.start = function(cb) { cb(); }; device.stop = function(cb) { cb(); }; - this.devices[this.deviceType] = device; + this.device = device; } else if (this.isIos()) { var iosOpts = { rest: this.rest @@ -495,7 +474,7 @@ Appium.prototype.invoke = function() { , robotPort: this.args.robotPort , robotAddress: this.args.robotAddress }; - this.devices[this.deviceType] = ios(iosOpts); + this.device = ios(iosOpts); } else if (this.isAndroid()) { var androidOpts = { rest: this.rest @@ -520,9 +499,9 @@ Appium.prototype.invoke = function() { }; if (this.isChrome()) { androidOpts.chromium = this.args.chromium; - this.devices[this.deviceType] = chrome(androidOpts); + this.device = chrome(androidOpts); } else { - this.devices[this.deviceType] = android(androidOpts); + this.device = android(androidOpts); } } else if (this.isSelendroid()) { var selendroidOpts = { @@ -544,83 +523,42 @@ Appium.prototype.invoke = function() { , keyAlias: this.args.keyAlias , keyPassword: this.args.keyPassword }; - this.devices[this.deviceType] = selendroid(selendroidOpts); + this.device = selendroid(selendroidOpts); } else if (this.isFirefoxOS()) { var firefoxOpts = { app: this.args.app , desiredCaps: this.desiredCapabilities , verbose: !this.args.quiet }; - this.devices[this.deviceType] = firefoxOs(firefoxOpts); + this.device = firefoxOs(firefoxOpts); } else { throw new Error("Tried to start a device that doesn't exist: " + this.deviceType); } } - this.device = this.devices[this.deviceType]; - - if (typeof this.desiredCapabilities.launch === "undefined" || this.desiredCapabilities.launch !== false ) { - this.device.start(function(err, sessionIdOverride) { - this.progress++; - // Ensure we don't use an undefined session. - if (typeof this.sessions[this.progress] === 'undefined') { - this.progress--; - return; - } - if (sessionIdOverride) { - this.sessionId = sessionIdOverride; - logger.info("Overriding session id with " + sessionIdOverride); - } - - this.sessions[this.progress].sessionId = this.sessionId; - this.sessions[this.progress].callback(err, this.device); - if (err) { - this.onDeviceDie(1); - } - }.bind(this), this.onDeviceDie.bind(this)); - } else { - //required if we dont want to launch the app, but we do want a session. - this.progress++; - // Ensure we don't use an undefined session. - if (typeof this.sessions[this.progress] === 'undefined') { - this.progress--; - return; + this.device.start(function(err, sessionIdOverride) { + if (sessionIdOverride) { + this.sessionId = sessionIdOverride; + logger.info("Overriding session id with " + sessionIdOverride); } + if (err) return this.cleanupSession(err, cb); + cb(null, this.device); + }.bind(this), this.cleanupSession.bind(this)); - this.sessions[this.progress].sessionId = this.sessionId; - this.sessions[this.progress].callback(null, this.device); - } - + } else { + return cb(new Error("Requested a new session but one was in progress")); } + }; -Appium.prototype.onDeviceDie = function(code, cb) { +Appium.prototype.cleanupSession = function(err, cb) { + logger.info("Cleaning up appium session"); var dyingSession = this.sessionId; this.sessionId = null; - // reset app to whatever it was before this session so we don't accidentally - // reuse a bad app - this.args.app = this.origApp; - this.args.bundleId = null; - if (code !== null) { - logger.info('Clearing out appium devices'); - this.devices = []; - this.device = null; - //logger.info(this.progress + " sessions.deviceType =" + this.sessions.length); - this.sessions[this.progress] = {}; - } else { - logger.info('Not clearing out appium devices'); - } - - // if we're not restarting internally, call invoke again, in case we have - // sessions queued - if (!this.resetting) { - this.clearPreviousSession(); - } - - if (cb) { - cb(null, {status: status.codes.Success.code, value: null, - sessionId: dyingSession}); - } + this.device = null; + if (err) return cb(err); + cb(null, {status: status.codes.Success.code, value: null, + sessionId: dyingSession}); }; Appium.prototype.stop = function(cb) { @@ -635,10 +573,14 @@ Appium.prototype.stop = function(cb) { if (this.device.udid === null){ this.device.instruments.shutdown(); } - this.onDeviceDie(0, cb); + this.cleanupSession(null, cb); } else { this.device.stop(function(code) { - this.onDeviceDie(code, cb); + var err; + if (code && code > 0) { + err = new Error("Device exited with code: " + code); + } + this.cleanupSession(err, cb); }.bind(this)); } }; diff --git a/lib/server/controller.js b/lib/server/controller.js index 84a7d215149..572fea764d4 100644 --- a/lib/server/controller.js +++ b/lib/server/controller.js @@ -148,7 +148,7 @@ exports.createSession = function(req, res) { req.appium.start(req.body.desiredCapabilities, function(err, instance) { if (err) { logger.error("Failed to start an Appium session, err was: " + err); - respondError(req, res, status.codes.NoSuchDriver, err); + respondError(req, res, status.codes.SessionNotCreatedException, err); } else { logger.info("Appium session started with sessionId " + req.appium.sessionId); next(req.headers.host, req.appium.sessionId, instance); diff --git a/lib/server/parser.js b/lib/server/parser.js index 0591a20797a..c039b8f9380 100644 --- a/lib/server/parser.js +++ b/lib/server/parser.js @@ -67,12 +67,12 @@ var args = [ , nargs: 0 }], - [['--no-session-override'] , { + [['--session-override'] , { defaultValue: false - , dest: 'noSessionOverride' + , dest: 'sessionOverride' , action: 'storeTrue' , required: false - , help: 'Disables session override' + , help: 'Enables session override (clobbering)' , nargs: 0 }], diff --git a/test/unit/queue.js b/test/unit/queue.js index e63cdc341f6..e15eb0beb51 100644 --- a/test/unit/queue.js +++ b/test/unit/queue.js @@ -43,17 +43,17 @@ describe('IOS', function() { describe('Appium', function() { var intercept = [] , logPath = path.resolve(__dirname, "../../../appium.log") - , inst = appium({log: logPath, noSessionOverride: true }); + , inst = appium({log: logPath}); inst.registerConfig({ios: true}); describe('#start', function() { - return it('should queue up subsequent calls and execute them sequentially', function(done) { + return it('should fail if a session is in progress', function(done) { var doneYet = function(num) { intercept.push(num); if (intercept.length > 9) { for (var i=0; i < intercept.length; i++) { - intercept[i].should.equal(i); + intercept[i].should.not.equal(i); } done(); } @@ -63,11 +63,16 @@ describe('Appium', function() { if (num > 9) return; - inst.start({app: "/path/to/fake.app", device: "mock_ios"}, function() { + inst.start({app: "/path/to/fake.app", device: "mock_ios"}, function(err) { var n = num; - setTimeout(function() { - inst.stop(function() { doneYet(n); }); - }, Math.round(Math.random()*100)); + if (n > 0) { + should.exist(err); + doneYet(n); + } else { + setTimeout(function() { + inst.stop(function() { doneYet(n); }); + }, 500); + } loop(++num); }); }; @@ -79,7 +84,7 @@ describe('Appium', function() { describe('Appium with clobber', function() { var logPath = path.resolve(__dirname, "../../../appium.log") - , inst = appium({log: logPath, noSessionOverride: false }); + , inst = appium({log: logPath, sessionOverride: true }); inst.registerConfig({mock_ios: true});