Skip to content

Commit

Permalink
remove confusing multiple session handling code
Browse files Browse the repository at this point in the history 
it doesn't actually do anything and was cruft
also, make session overriding non-default based on the mobile
JSONWP discussions
  • Loading branch information
@jlipps
jlipps committed Oct 28, 2013
1 parent 27e684b commit b537c4a
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 107 deletions.
4 changes: 2 additions & 2 deletions docs/server-args.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`|
Expand Down
128 changes: 35 additions & 93 deletions lib/appium.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down Expand Up @@ -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));
};
Expand Down Expand Up @@ -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") {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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 = {
Expand All @@ -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) {
Expand All @@ -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));
}
};
Expand Down
2 changes: 1 addition & 1 deletion lib/server/controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions lib/server/parser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}],

Expand Down
21 changes: 13 additions & 8 deletions test/unit/queue.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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);
});
};
Expand All @@ -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});

Expand Down

0 comments on commit b537c4a

Please sign in to comment.