Use push on PYTHONPATH and add tests

Using push instead of the original unshift makes the logic a little
cleaner to read here.

PR-URL: #990
Reviewed-By: Ben Noordhuis <[email protected]>

lib/configure.js

@@ -287,9 +287,11 @@ function configure (gyp, argv, callback) {
       argv.unshift(gyp_script)
 
       // make sure python uses files that came with this particular node package
-      var pypath = process.env.PYTHONPATH ? [process.env.PYTHONPATH] : []
-      pypath.unshift(path.resolve(__dirname, '..', 'gyp', 'pylib'))
-      process.env.PYTHONPATH = pypath.join(process.platform === 'win32' ? ';' : ':')
+      var pypath = [path.join(__dirname, '..', 'gyp', 'pylib')]
+      if (process.env.PYTHONPATH) {
+        pypath.push(process.env.PYTHONPATH)
+      }
+      process.env.PYTHONPATH = pypath.join(win ? ';' : ':')
 
       var cp = gyp.spawn(python, argv)
       cp.on('exit', onCpExit)

test/test-configure-python.js

@@ -0,0 +1,72 @@
+'use strict'
+
+var test = require('tape')
+var path = require('path')
+var gyp = require('../lib/node-gyp')
+var requireInject = require('require-inject')
+var configure = requireInject('../lib/configure', {
+  'graceful-fs': {
+    'writeFile': function (file, data, cb) { cb() },
+    'stat': function (file, cb) { cb(null, {}) }
+  }
+})
+
+var EXPECTED_PYPATH = path.join(__dirname, '..', 'gyp', 'pylib')
+var SEPARATOR = process.platform == 'win32' ? ';' : ':'
+var SPAWN_RESULT = { on: function () { } }
+
+test('configure PYTHONPATH with no existing env', function (t) {
+  t.plan(1)
+
+  delete process.env.PYTHONPATH
+
+  var prog = gyp()
+  prog.parseArgv([])
+  prog.spawn = function () {
+    t.equal(process.env.PYTHONPATH, EXPECTED_PYPATH)
+    return SPAWN_RESULT
+  }
+  configure(prog, [])
+})
+
+test('configure PYTHONPATH with existing env of one dir', function (t) {
+  t.plan(2)
+
+  var existingPath = path.join('a', 'b')
+  process.env.PYTHONPATH = existingPath
+
+  var prog = gyp()
+  prog.parseArgv([])
+  prog.spawn = function () {
+
+    t.equal(process.env.PYTHONPATH, [EXPECTED_PYPATH, existingPath].join(SEPARATOR))
+
+    var dirs = process.env.PYTHONPATH.split(SEPARATOR)
+    t.deepEqual(dirs, [EXPECTED_PYPATH, existingPath])
+
+    return SPAWN_RESULT
+  }
+  configure(prog, [])
+})
+
+test('configure PYTHONPATH with existing env of multiple dirs', function (t) {
+  t.plan(2)
+
+  var pythonDir1 = path.join('a', 'b')
+  var pythonDir2 = path.join('b', 'c')
+  var existingPath = [pythonDir1, pythonDir2].join(SEPARATOR)
+  process.env.PYTHONPATH = existingPath
+
+  var prog = gyp()
+  prog.parseArgv([])
+  prog.spawn = function () {
+
+    t.equal(process.env.PYTHONPATH, [EXPECTED_PYPATH, existingPath].join(SEPARATOR))
+
+    var dirs = process.env.PYTHONPATH.split(SEPARATOR)
+    t.deepEqual(dirs, [EXPECTED_PYPATH, pythonDir1, pythonDir2])
+
+    return SPAWN_RESULT
+  }
+  configure(prog, [])
+})