Merge f8ecc31 into 4dbeb00

lib/commands/bin.js

@@ -1,4 +1,4 @@
-const envPath = require('../utils/path.js')
+const { PATH } = require('../utils/path.js')
 const BaseCommand = require('../base-command.js')
 
 class Bin extends BaseCommand {
@@ -9,7 +9,7 @@ class Bin extends BaseCommand {
   async exec (args) {
     const b = this.npm.bin
     this.npm.output(b)
-    if (this.npm.config.get('global') && !envPath.includes(b)) {
+    if (this.npm.config.get('global') && !PATH.includes(b)) {
       // XXX: does this need to be console?
       console.error('(not in PATH env variable)')
     }

lib/utils/path.js

@@ -1,4 +1,13 @@
 // return the PATH array in a cross-platform way
-const PATH = process.env.PATH || process.env.Path || process.env.path
 const { delimiter } = require('path')
-module.exports = PATH.split(delimiter)
+
+const keys = ['PATH', 'Path', 'path']
+const key = keys.find(k => process.env[k])
+const value = process.env[key]
+
+module.exports = {
+  PATH: value.split(delimiter),
+  key,
+  keys,
+  value,
+}

test/fixtures/mock-globals.js

@@ -3,6 +3,8 @@
 // This file is only used in tests but it is still tested itself.
 // Hopefully it can be removed for a feature in tap in the future
 
+const PATH = require('../../lib/utils/path')
+
 const sep = '.'
 const has = (o, k) => Object.prototype.hasOwnProperty.call(o, k)
 const opd = (o, k) => Object.getOwnPropertyDescriptor(o, k)
@@ -54,41 +56,41 @@ const protoDescriptor = (obj, key) => {
   return descriptor
 }
 
-// Path can be different cases across platform so get the original case
-// of the path before anything is changed
+// Path can be different cases across platform so get the original case of
+// the key so we can reset it on teardown
 // XXX: other special cases to handle?
-const specialCaseKeys = (() => {
-  const originalKeys = {
-    PATH: process.env.PATH ? 'PATH' : process.env.Path ? 'Path' : 'path',
-  }
-  return (key) => {
-    switch (key.toLowerCase()) {
-      case 'process.env.path':
-        return originalKeys.PATH
-    }
-  }
-})()
+const getCaseSensitiveKey = (k) => ({
+  'process.env.path': PATH.keys,
+})[k.toLowerCase()]
 
 const _setGlobal = Symbol('setGlobal')
 const _nextDescriptor = Symbol('nextDescriptor')
 
 class DescriptorStack {
   #stack = []
   #global = null
-  #valueKey = null
-  #defaultDescriptor = { configurable: true, writable: true, enumerable: true }
+  #valueKeys = null
+  #descriptor = null
   #delete = () => ({ DELETE: true })
   #isDelete = (o) => o && o.DELETE === true
 
   constructor (key) {
     const keys = splitLast(key)
     this.#global = keys.length === 1 ? global : get(global, keys[0])
-    this.#valueKey = specialCaseKeys(key) || last(keys)
-    // If the global object doesnt return a descriptor for the key
-    // then we mark it for deletion on teardown
-    this.#stack = [
-      protoDescriptor(this.#global, this.#valueKey) || this.#delete(),
-    ]
+    this.#valueKeys = getCaseSensitiveKey(key) || [last(keys)]
+
+    const initial = []
+    let initialDescriptor = null
+    for (const key of this.#valueKeys) {
+      const descriptor = protoDescriptor(this.#global, key) || this.#delete()
+      initial.push({ key, descriptor })
+      if (!initialDescriptor && !this.#isDelete(descriptor)) {
+        initialDescriptor = descriptor
+      }
+    }
+
+    this.#stack = [initial]
+    this.#descriptor = initialDescriptor || { configurable: true, writable: true, enumerable: true }
   }
 
   add (value) {
@@ -111,7 +113,7 @@ class DescriptorStack {
     }
   }
 
-  reset () {
+  teardown () {
     // Everything could be reset manually so only
     // teardown if we have an initial descriptor left
     // and then delete the rest of the stack
@@ -121,26 +123,29 @@ class DescriptorStack {
     }
   }
 
-  [_setGlobal] (d) {
-    if (this.#isDelete(d)) {
-      delete this.#global[this.#valueKey]
-    } else {
-      Object.defineProperty(this.#global, this.#valueKey, d)
-    }
-    return d
+  [_setGlobal] (descriptors) {
+    descriptors.forEach(({ key, descriptor }) => {
+      if (this.#isDelete(descriptor)) {
+        delete this.#global[key]
+      } else {
+        Object.defineProperty(this.#global, key, descriptor)
+      }
+    })
+    return descriptors
   }
 
   [_nextDescriptor] (value) {
-    if (value === undefined) {
-      return this.#delete()
-    }
-    const d = last(this.#stack)
-    return {
-      // If the previous descriptor was one to delete the property
-      // then use the default descriptor as the base
-      ...(this.#isDelete(d) ? this.#defaultDescriptor : d),
-      ...(d && d.get ? { get: () => value } : { value }),
-    }
+    return this.#valueKeys.map((key) => ({
+      key,
+      descriptor: value === undefined
+        ? this.#delete()
+        : {
+        // If the previous descriptor was one to delete the property
+        // then use the default descriptor as the base
+          ...this.#descriptor,
+          ...(this.#descriptor.get ? { get: () => value } : { value }),
+        },
+    }))
   }
 }
 
@@ -179,10 +184,10 @@ class MockGlobals {
 
   teardown (key) {
     if (!key) {
-      Object.values(this.#descriptors).forEach((d) => d.reset())
+      Object.values(this.#descriptors).forEach((d) => d.teardown())
       return
     }
-    this.#descriptors[key].reset()
+    this.#descriptors[key].teardown()
   }
 }
 

test/lib/commands/bin.js

@@ -33,7 +33,7 @@ t.test('bin -g', async t => {
   const dir = '/bin/dir'
 
   const Bin = t.mock('../../../lib/commands/bin.js', {
-    '../../../lib/utils/path.js': [dir],
+    '../../../lib/utils/path.js': { PATH: [dir] },
   })
 
   const npm = mockNpm({
@@ -61,7 +61,7 @@ t.test('bin -g (not in path)', async t => {
   const dir = '/bin/dir'
 
   const Bin = t.mock('../../../lib/commands/bin.js', {
-    '../../../lib/utils/path.js': ['/not/my/dir'],
+    '../../../lib/utils/path.js': { PATH: ['/not/my/dir'] },
   })
   const npm = mockNpm({
     bin: dir,

test/lib/commands/exec.js

@@ -72,7 +72,7 @@ const read = (options, cb) => {
   process.nextTick(() => cb(READ_ERROR, READ_RESULT))
 }
 
-const PATH = require('../../../lib/utils/path.js')
+const { PATH } = require('../../../lib/utils/path.js')
 
 let CI_NAME = 'travis-ci'
 

test/lib/fixtures/mock-globals.js

@@ -8,6 +8,9 @@ const originals = {
   stderrWrite: process.stderr.write,
   shell: process.env.SHELL,
   home: process.env.HOME,
+  PATH: process.env.PATH,
+  Path: process.env.Path,
+  path: process.env.path,
   argv: process.argv,
   env: process.env,
   setInterval,
@@ -196,6 +199,35 @@ t.test('falsy values', async t => {
   t.equal(process.platform, originals.platform)
 })
 
+t.test('path case insensitivity', async t => {
+  const setPath = (t, p, v) => mockGlobals(t, { [`process.env.${p}`]: v })
+
+  await t.test('uppercase', async t => {
+    setPath(t, 'PATH', '1')
+    t.equal(process.env.PATH, '1')
+    t.equal(process.env.Path, '1')
+    t.equal(process.env.path, '1')
+  })
+
+  await t.test('mixed case', async t => {
+    setPath(t, 'Path', '2')
+    t.equal(process.env.PATH, '2')
+    t.equal(process.env.Path, '2')
+    t.equal(process.env.path, '2')
+  })
+
+  await t.test('lowercase', async t => {
+    setPath(t, 'path', '3')
+    t.equal(process.env.PATH, '3')
+    t.equal(process.env.Path, '3')
+    t.equal(process.env.path, '3')
+  })
+
+  t.equal(process.env.PATH, originals.PATH)
+  t.equal(process.env.Path, originals.Path)
+  t.equal(process.env.path, originals.path)
+})
+
 t.test('date', async t => {
   await t.test('mocks', async t => {
     mockGlobals(t, {

test/lib/utils/path.js

@@ -1,12 +1,38 @@
 const t = require('tap')
-const mod = '../../../lib/utils/path.js'
-const delim = require('../../../lib/utils/is-windows.js') ? ';' : ':'
-Object.defineProperty(process, 'env', {
-  value: {},
+const mockGlobals = require('../../fixtures/mock-globals')
+const { delimiter } = require('path')
+
+const mockPath = () => t.mock('../../../lib/utils/path.js')
+
+mockGlobals(t, { 'process.env': {} }, { replace: true })
+t.strictSame(process.env, {})
+
+t.test('uppercase', async t => {
+  const parts = ['foo', 'bar', 'baz']
+  mockGlobals(t, { 'process.env.PATH': parts.join(delimiter) })
+  t.match(mockPath(), {
+    PATH: parts,
+    key: 'PATH',
+    value: parts.join(delimiter),
+  })
+})
+
+t.test('mixed case', async t => {
+  const parts = ['a', 'b', 'c']
+  mockGlobals(t, { 'process.env.Path': parts.join(delimiter) })
+  t.match(mockPath(), {
+    PATH: parts,
+    key: 'Path',
+    value: parts.join(delimiter),
+  })
+})
+
+t.test('lowercase', async t => {
+  const parts = ['x', 'y', 'z']
+  mockGlobals(t, { 'process.env.path': parts.join(delimiter) })
+  t.match(mockPath(), {
+    PATH: parts,
+    key: 'path',
+    value: parts.join(delimiter),
+  })
 })
-process.env.path = ['foo', 'bar', 'baz'].join(delim)
-t.strictSame(t.mock(mod), ['foo', 'bar', 'baz'])
-process.env.Path = ['a', 'b', 'c'].join(delim)
-t.strictSame(t.mock(mod), ['a', 'b', 'c'])
-process.env.PATH = ['x', 'y', 'z'].join(delim)
-t.strictSame(t.mock(mod), ['x', 'y', 'z'])