From 8a0c51b44949e643d57e536d65e2a6668f03e371 Mon Sep 17 00:00:00 2001 From: Jonathan Lipps Date: Tue, 12 Feb 2013 14:22:41 -0800 Subject: [PATCH 1/3] add global error handling for appium controller methods --- app/controller.js | 5 +++++ app/routing.js | 3 ++- server.js | 9 +++++++++ test/functional/appium/jsonwp.js | 23 ++++++++++++++++++++--- 4 files changed, 36 insertions(+), 4 deletions(-) diff --git a/app/controller.js b/app/controller.js index 984520a8..f5d2dc77 100644 --- a/app/controller.js +++ b/app/controller.js @@ -138,6 +138,7 @@ exports.createSession = function(req, res) { } else { 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); } else { next(req.appium.sessionId, instance); @@ -527,3 +528,7 @@ var mobileCmdMap = { exports.produceError = function(req, res) { req.device.proxy("thisisnotvalidjs", getResponseHandler(req, res)); }; + +exports.crash = function() { + throw new Error("We just tried to crash Appium!"); +}; diff --git a/app/routing.js b/app/routing.js index fb5d213b..51b4180c 100644 --- a/app/routing.js +++ b/app/routing.js @@ -57,8 +57,9 @@ module.exports = function(appium) { rest.post('/wd/hub/session/:sessionId?/execute', controller.execute); rest.get('/wd/hub/session/:sessionId?/title', controller.title); - // this is for testing purposes only + // these are for testing purposes only rest.post('/wd/hub/produce_error', controller.produceError); + rest.post('/wd/hub/crash', controller.crash); // appium-specific extensions to JSONWP // these aren't part of JSONWP but we want them or something like them to be diff --git a/server.js b/server.js index a6023056..9f49a024 100644 --- a/server.js +++ b/server.js @@ -6,6 +6,7 @@ var http = require('http') , appium = require('./app/appium') , bodyParser = require('./middleware').parserWrap , checkWarpDrive = require('./warp.js').checkWarpDrive + , status = require('./app/uiauto/lib/status') , parser = require('./app/parser'); var doWarpCheck = function(wantWarp, cb) { @@ -51,6 +52,14 @@ var main = function(args, readyCb, doneCb) { rest.use(bodyParser); rest.use(express.methodOverride()); rest.use(rest.router); + // catch all error handler + rest.use(function(e, req, res, next) { + res.send(500, { + status: status.codes.UnknownError.code + , value: "ERROR running Appium command: " + e.message + }); + next(e); + }); }); // Instantiate the appium instance diff --git a/test/functional/appium/jsonwp.js b/test/functional/appium/jsonwp.js index 6e8059e4..5ebc7191 100644 --- a/test/functional/appium/jsonwp.js +++ b/test/functional/appium/jsonwp.js @@ -4,13 +4,17 @@ var should = require("should") , serverUrl = 'http://localhost:4723' , serverHub = serverUrl + '/wd/hub/session' + , path = require('path') + , appPath = "../../../sample-code/apps/TestApp/build/Release-iphonesimulator/TestApp.app" , request = require('request'); var describeWithSession = function(desc, tests) { describe(desc, function() { var sessObj = {sessionId: null}; beforeEach(function(done) { - var caps = {desiredCapabilities: {}}; + var caps = {desiredCapabilities: { + app: path.resolve(__dirname, appPath) + }}; request.post({url: serverHub, json: caps}, function(err, res) { should.not.exist(err); res.statusCode.should.equal(303); @@ -56,7 +60,7 @@ describe('JSONWP request', function() { request.post(url, function(err, res, body) { should.not.exist(err); res.statusCode.should.equal(501); - body.should.equal("Not Implemented"); + JSON.parse(body).status.should.equal(13); done(); }); }); @@ -79,7 +83,20 @@ describe('JSONWP request', function() { res.statusCode.should.equal(500); should.ok(body); body = JSON.parse(body); - should.ok(body.value.message); + should.ok(body.value); + done(); + }); + }); + }); + describeWithSession('that generates a server crash', function() { + it('should respond with a 500', function(done) { + var url = serverUrl + '/wd/hub/crash'; + request.post(url, function(err, res, body) { + should.not.exist(err); + res.statusCode.should.equal(500); + should.ok(body); + body = JSON.parse(body); + should.ok(body.value); done(); }); }); From 6929940eba785fff84aa76bcf6ea1ca78b9c2159 Mon Sep 17 00:00:00 2001 From: Jonathan Lipps Date: Tue, 12 Feb 2013 14:41:47 -0800 Subject: [PATCH 2/3] be much more eager about cleaning up extraneous files --- app/ios.js | 50 ++++++++++++++++++++++++++++++++++++++------------ package.json | 1 + 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/app/ios.js b/app/ios.js index 4ccc3625..0c4ca958 100644 --- a/app/ios.js +++ b/app/ios.js @@ -5,6 +5,7 @@ var path = require('path') , _ = require('underscore') , logger = require('../logger').get('appium') , sock = '/tmp/instruments_sock' + , glob = require('glob') , instruments = require('../instruments/instruments') , uuid = require('uuid-js') , timeWarp = require('../warp.js').timeWarp @@ -57,6 +58,28 @@ var IOS = function(rest, app, udid, verbose, removeTraceDir, warp) { }; }; +IOS.prototype.cleanup = function(cb) { + logger.info("Cleaning up instruments files"); + if (this.removeTraceDir) { + glob("*.trace", {}, function(err, files) { + if (err) { + logger.error("Could not glob for tracedirs: " + err.message); + } else { + _.each(files, function(file) { + file = path.resolve(process.cwd(), file); + rimraf(file, function() { + logger.info("Cleaned up " + file); + }); + }); + } + }); + } + rimraf(sock, function() { + logger.info("Cleaned up instruments socket " + sock); + cb(); + }); +}; + IOS.prototype.start = function(cb, onDie) { var me = this; var didLaunch = false; @@ -92,6 +115,7 @@ IOS.prototype.start = function(cb, onDie) { this.instruments = null; if (me.removeTraceDir && traceDir) { rimraf(traceDir, function() { + logger.info("Deleted tracedir we heard about from instruments (" + traceDir + ")"); me.onStop(code); }); } else { @@ -100,18 +124,20 @@ IOS.prototype.start = function(cb, onDie) { }; if (this.instruments === null) { - if (this.warp) { - timeWarp(50, 1000); - } - this.instruments = instruments( - this.app - , this.udid - , path.resolve(__dirname, 'uiauto/bootstrap.js') - , path.resolve(__dirname, 'uiauto/Automation.tracetemplate') - , sock - , onLaunch - , onExit - ); + this.cleanup(_.bind(function() { + if (this.warp) { + timeWarp(50, 1000); + } + this.instruments = instruments( + this.app + , this.udid + , path.resolve(__dirname, 'uiauto/bootstrap.js') + , path.resolve(__dirname, 'uiauto/Automation.tracetemplate') + , sock + , onLaunch + , onExit + ); + }, this)); } }; diff --git a/package.json b/package.json index f87dac0e..60d1a4c1 100644 --- a/package.json +++ b/package.json @@ -41,6 +41,7 @@ "node-bplist-creator": "~0.0.1", "node-uuid": "~1.4.0", "underscore.string": "~2.3.1", + "glob": "~3.1.20", "unzip": "~0.1.1" }, "scripts": { From 38bf9fedd2de478c38f7413204e466a16647dda3 Mon Sep 17 00:00:00 2001 From: Jonathan Lipps Date: Tue, 12 Feb 2013 14:47:22 -0800 Subject: [PATCH 3/3] update this test to use the new hotness behavior --- test/functional/appium/appiumutils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/appium/appiumutils.js b/test/functional/appium/appiumutils.js index bcbb2810..4dd211b1 100644 --- a/test/functional/appium/appiumutils.js +++ b/test/functional/appium/appiumutils.js @@ -109,8 +109,8 @@ describe("appiumutils", function() { it('should respond nicely to js errors', function(done) { device.proxy("blargimarg", function(err, res) { - should.exist(err); - should.ok(~res); + should.not.exist(err); + res.status.should.equal(17); done(); }); });