Update changelog.txt for 1.3.7-Beta

Conflicts:
	package.json

CHANGELOG.txt

@@ -1,3 +1,12 @@
+CHANGES IN VERSION 1.3.7-Beta
+======================================
+
+#### General
+- fix failure to remap session id in proxied responses
+
+#### iOS
+- fix intermittent failure to find Xcode
+
 CHANGES IN VERSION 1.3.6
 ======================================
 

lib/devices/ios/xcode.js

@@ -2,21 +2,65 @@
 
 var logger = require('../../server/logger.js').get('appium')
   , fs = require('fs')
-  , path = require('path')
   , async = require('async')
-  , exec = require('child_process').exec;
+  , exec = require('child_process').exec
+  , util = require('appium-support').util
+  , env = process.env;
 
+var XCODE_SUBDIR = "/Contents/Developer";
 var xcode = {};
 
+var _hasExpectedSubDir = function (path) {
+  return path.substring(path.length - XCODE_SUBDIR.length) === XCODE_SUBDIR;
+};
+
+var _getPathFromSymlink = function (stdout, cb) {
+  // Node's invocation of xcode-select sometimes flakes and returns an empty string.
+  // Not clear why. As a workaround, Appium can reliably deduce the version in use by checking
+  // the locations xcode-select uses to store the selected version's path. This should be 100%
+  // reliable so long as the link locations remain the same. However, since we're relying on
+  // hardcoded paths, this approach will break the next time Apple changes the symlink location.
+  var symlnkPath = "/var/db/xcode_select_link";
+  var legacySymlnkPath = "/usr/share/xcode-select/xcode_dir_path"; //  Xcode < 5.x (?)
+
+  // xcode-select allows users to override its settings with the DEVELOPER_DIR env var,
+  // so check that first
+  if (util.hasContent(env.DEVELOPER_DIR)) {
+    var dir = _hasExpectedSubDir(env.DEVELOPER_DIR) ?
+                env.DEVELOPER_DIR :
+                env.DEVELOPER_DIR + XCODE_SUBDIR;
+    cb(null, dir);
+  } else if (fs.existsSync(symlnkPath)) {
+    fs.readlink(symlnkPath, cb);
+  } else if (fs.existsSync(legacySymlnkPath)) {
+    fs.readlink(legacySymlnkPath, cb);
+  } else {
+    // We should only get here is we failed to capture xcode-select's stdout and our
+    // other checks failed. Either Apple has moved the symlink to a new location or the user
+    // is using the default install. 99.999% chance it's the latter, so issue a warning
+    // should we ever hit the edge case.
+    logger.warn("xcode-select returned an invalid path: " + stdout +
+                ". No links to alternate versions were detected. Using Xcode's " +
+                "default location.");
+    var path = "/Applications/Xcode.app" + XCODE_SUBDIR;
+    var err = fs.existsSync(path) ? null : new Error("No Xcode installation found.");
+    cb(err, path);
+  }
+};
+
 xcode.getPath = function (cb) {
   exec('xcode-select --print-path', { maxBuffer: 524288, timeout: 3000 }, function (err, stdout, stderr) {
     if (!err) {
       var xcodeFolderPath = stdout.replace("\n", "");
-      if (fs.existsSync(xcodeFolderPath)) {
+
+      if (util.hasContent(xcodeFolderPath) && fs.existsSync(xcodeFolderPath)) {
         cb(null, xcodeFolderPath);
+      } else if (!util.hasContent(xcodeFolderPath)) {
+        _getPathFromSymlink(xcodeFolderPath, cb);
       } else {
-        logger.error(xcodeFolderPath + "does not exist.");
-        cb(new Error("xcode-select could not find xcode"), null);
+        var msg = "xcode-select could not find xcode. Path: " + xcodeFolderPath + " does not exist.";
+        logger.error(msg);
+        cb(new Error(msg), null);
       }
     } else {
       logger.error("xcode-select threw error " + err);
@@ -29,24 +73,31 @@ xcode.getPath = function (cb) {
 
 xcode.getVersion = function (cb) {
   xcode.getPath(function (err, xcodePath) {
-    if (err  !== null) {
-      cb(err, null);
+    if (!err && !_hasExpectedSubDir(xcodePath)) {
+      err = new Error("Expected a path ending with '" + XCODE_SUBDIR + "', but got " + xcodePath);
+    }
+    if (err) {
+      return cb(err, null);
+    }
+    // we want to read the CFBundleShortVersionString from Xcode's plist.
+    // It should be in /[root]/XCode.app/Contents/
+    var plistPath = xcodePath.replace(XCODE_SUBDIR, "/Contents/Info.plist");
+    if (!fs.existsSync(plistPath)) {
+      return cb(new Error("Could not get Xcode version: " + plistPath + " does not exist on disk."), null);
     } else {
-      var xcodebuildPath = path.resolve(xcodePath, "usr/bin/xcodebuild");
-      if (!fs.existsSync(xcodebuildPath)) {
-        cb(new Error("Could not get Xcode version: " + xcodebuildPath + " does not exist on disk."), null);
-      } else {
-        exec(JSON.stringify(xcodebuildPath) + ' -version', { maxBuffer: 524288, timeout: 3000 }, function (err, stdout) {
-          var versionPattern = /\d\.\d\.*\d*/;
-          var match = versionPattern.exec(stdout);
-          if (match === null) {
-            cb(new Error("Could not parse Xcode version (xcodebuild output was: " + stdout + ")"), null);
-          } else {
-            var versionNumber = match[0];
-            cb(null, versionNumber);
-          }
-        });
-      }
+      var cmd = "/usr/libexec/PlistBuddy -c 'Print CFBundleShortVersionString' " + plistPath;
+      exec(cmd, { maxBuffer: 524288, timeout: 3000 }, function (err, stdout) {
+        if (err !== null) return cb(err, null);
+        var versionPattern = /\d\.\d\.*\d*/;
+        // need to use string#match here; previous code used regexp#exec, which does not return null
+        var match = stdout.match(versionPattern);
+        if (match === null || !util.hasContent(match[0])) {
+          cb(new Error("Could not parse Xcode version (xcodebuild output was: " + stdout + ")."), null);
+        } else {
+          var versionNumber = match[0];
+          cb(null, versionNumber);
+        }
+      });
     }
   });
 };

lib/helpers.js

@@ -338,16 +338,4 @@ exports.truncateDecimals = function (number, digits) {
   return truncatedNum / multiplier;
 };
 
-// return true if the the value is not undefined, null, or NaN.
-exports.notNullOrUndefined = function (val) {
-  var isDefined = false;
 
-  // avoid incorrectly evaluating `0` as false
-  if (_.isNumber(val)) {
-    isDefined = !_.isNaN(val);
-  } else {
-    isDefined = !_.isUndefined(val) && !_.isNull(val);
-  }
-
-  return isDefined;
-};

lib/server/controller.js

@@ -3,6 +3,7 @@
 "use strict";
 var status = require('./status.js')
   , logger = require('./logger.js').get('appium')
+  , _ = require('underscore')
   , _s = require("underscore.string")
   , swig = require('swig')
   , path = require('path')
@@ -14,11 +15,10 @@ var status = require('./status.js')
   , respondSuccess = responses.respondSuccess
   , checkMissingParams = responses.checkMissingParams
   , notYetImplemented = responses.notYetImplemented
+  , hasValue = require('appium-support').util.hasValue
   , helpers = require('../helpers.js')
-  , notNullOrUndefined = helpers.notNullOrUndefined
   , logCustomDeprecationWarning = helpers.logCustomDeprecationWarning
-  , _ = require('underscore')
-  , safely = require('./helpers').safely
+  , safely = require('./helpers.js').safely
   , NotImplementedError = require('./errors').NotImplementedError;
 
 exports.getGlobalBeforeFilter = function (appium) {
@@ -485,7 +485,7 @@ exports.mobileSwipe = function (req, res, gestures) {
     // going the long way and checking for undefined and null since
     // we can't be assured `elId` is a string and not an int. Same
     // thing with destElement below.
-    return notNullOrUndefined(val) ? val : 0.5;
+    return hasValue(val) ? val : 0.5;
   };
 
   var touchCount = req.body.touchCount || 1
@@ -498,7 +498,7 @@ exports.mobileSwipe = function (req, res, gestures) {
     , destElement = gestures[2].options.element || gestures[0].options.element;
 
   // there's no destination element handling in bootstrap and since it applies to all platforms, we handle it here
-  if (notNullOrUndefined(destElement)) {
+  if (hasValue(destElement)) {
     req.device.getLocation(destElement, function (err, locResult) {
       if (err) {
         respondError(req, res, err);
@@ -519,7 +519,7 @@ exports.mobileSwipe = function (req, res, gestures) {
             var destY = locResult.value.y + offsetY;
 
             // if the target element was provided, the coordinates for the destination need to be relative to it.
-            if (notNullOrUndefined(element)) {
+            if (hasValue(element)) {
               req.device.getLocation(element, function (e, firstElLocation) {
                 if (e) {
                   respondError(req, res, e);

lib/server/proxy.js

@@ -43,10 +43,10 @@ module.exports.doProxy = function (req, res) {
                req.device.proxyPort);
   var sessRe = new RegExp('^/wd/hub/session/([^/]+)');
   var sessionRegxMatch = sessRe.exec(req.originalUrl);
-  var newPath;
+  var newPath, origSessId = null;
   // there might be no session id in the orig. req.
   if (sessionRegxMatch) {
-    var origSessId = sessionRegxMatch[1];
+    origSessId = sessionRegxMatch[1];
     var sessId = req.device.proxySessionId ? req.device.proxySessionId :
                                              origSessId;
     newPath = req.originalUrl.replace(origSessId, sessId);
@@ -68,12 +68,19 @@ module.exports.doProxy = function (req, res) {
         return respondError(req, res, resStatusCode, resErrorMessage);
       }
     }
-
     var sbody = body ? JSON.stringify(body).slice(0, 10000) : body;
 
     logger.debug("Proxied response received with status " +
                  response.statusCode + ": " +
                  sbody);
+
+    // if the proxied response contains a sessionId that the downstream
+    // driver has generated, we don't want to return that to the client.
+    // Instead, return the id for the current appium session
+    if (body && body.sessionId && origSessId) {
+      body.sessionId = origSessId;
+    }
+
     safely(req, function () {
       res.headers = response.headers;
       res.set('Content-Type', response.headers['content-type']);

package.json

@@ -11,7 +11,7 @@
     "firefoxos",
     "testing"
   ],
-  "version": "1.3.6",
+  "version": "1.3.7-beta",
   "author": "https://github.com/appium",
   "licenses": [
     {
@@ -43,9 +43,9 @@
     "adm-zip": "~0.4.7",
     "appium-adb": "=1.7.4",
     "appium-atoms": "=0.0.5",
-    "appium-instruments": "=1.5.3",
-    "appium-support": "=0.0.5",
-    "appium-uiauto": "=1.10.3",
+    "appium-instruments": "=1.5.4",
+    "appium-support": "=0.1.1",
+    "appium-uiauto": "=1.10.6",
     "argparse": "~1.0.1",
     "async": "~0.9.0",
     "binary-cookies": "~0.1.1",

test/functional/ios/testapp/device-specs.js

@@ -3,12 +3,50 @@
 var _ = require('underscore'),
     initSession = require('../../../helpers/session').initSession,
     should = require('chai').should(),
+    expect = require('chai').expect,
+    XCODE = require('../../../../lib/devices/ios/xcode.js'),
+    fs = require('fs'),
+    env = process.env,
     desired = require('./desired');
 
 require('../../../helpers/setup-chai.js');
 
 describe('testapp - device', function () {
 
+  describe('xcode#getpath', function () {
+    it('should always return a valid path', function (done) {
+      this.timeout(5000);
+      // getPath failures have been intermittent, so try a few times
+      var iterations = 100;
+      var nextTest = function () {
+        XCODE.getPath(function (err, path) {
+          expect(err).to.be.null;
+          expect(path).not.to.be.empty;
+          expect(fs.existsSync(path)).to.be.true;
+          if (--iterations > 0) {
+            nextTest();
+          } else {
+            done();
+          }
+        });
+      };
+      nextTest();
+    });
+
+    // careful, there's a bug in mocha so if you call this before
+    // 'should always return a valid path', then the other test
+    // will get passed this test's `err` object.
+    it('should fail with a bad path', function (done) {
+      env.DEVELOPER_DIR = "/foo/bar";
+
+      XCODE.getPath(function (err) {
+        env.DEVELOPER_DIR = "";
+        expect(err).not.to.be.null;
+        done();
+      });
+    });
+  });
+
   describe('invalid deviceName @skip-ios6', function () {
     var newDesired = _.extend(_.clone(desired), {deviceName: "iFailure 3.5-inch"});
     var session = initSession(newDesired, {'no-retry': true});

test/unit/helper-specs.js

@@ -23,28 +23,6 @@ describe("Helpers", function () {
       });
     });
 
-    describe("notNullOrUndefined ", function () {
-      it("should work as expected", function () {
-        var undef
-          , nan = NaN
-          , none = null
-          , f = function () {}
-          , o = {}
-          , n = 0
-          , s = "string"
-          , b = false;
-
-        helpers.notNullOrUndefined(undef).should.be.false;
-        helpers.notNullOrUndefined(nan).should.be.false;
-        helpers.notNullOrUndefined(none).should.be.false;
-        helpers.notNullOrUndefined(f).should.be.true;
-        helpers.notNullOrUndefined(o).should.be.true;
-        helpers.notNullOrUndefined(n).should.be.true;
-        helpers.notNullOrUndefined(s).should.be.true;
-        helpers.notNullOrUndefined(b).should.be.true;
-      });
-    });
-
     describe("with a desired capability", function () {
       it("emits the capability message", function () {
         var deprecated = "oldCap";