From 0a957f5e2fbcce51c407d22b19e38004d09c51af Mon Sep 17 00:00:00 2001 From: Gar Date: Thu, 24 Mar 2022 09:09:26 -0700 Subject: [PATCH] fix: consolidate path delimiter logic --- lib/commands/bin.js | 8 +++-- lib/utils/path.js | 5 ---- test/lib/commands/bin.js | 62 +++++++++++---------------------------- test/lib/commands/exec.js | 8 ++--- test/lib/utils/path.js | 13 -------- 5 files changed, 26 insertions(+), 70 deletions(-) delete mode 100644 lib/utils/path.js delete mode 100644 test/lib/utils/path.js diff --git a/lib/commands/bin.js b/lib/commands/bin.js index 07d33167d0d07..4200d5b8ca556 100644 --- a/lib/commands/bin.js +++ b/lib/commands/bin.js @@ -1,6 +1,10 @@ const log = require('../utils/log-shim.js') -const envPath = require('../utils/path.js') const BaseCommand = require('../base-command.js') +// TODO this may not be needed at all. Ignoring because our tests normalize +// process.env.path already +/* istanbul ignore next */ +const path = process.env.path || process.env.Path || process.env.PATH +const { delimiter } = require('path') class Bin extends BaseCommand { static description = 'Display npm bin folder' @@ -11,7 +15,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.split(delimiter).includes(b)) { log.error('bin', '(not in PATH env variable)') } } diff --git a/lib/utils/path.js b/lib/utils/path.js deleted file mode 100644 index fcbf92e569757..0000000000000 --- a/lib/utils/path.js +++ /dev/null @@ -1,5 +0,0 @@ -// return the PATH array in a cross-platform way -// TODO this is only used in a single place -const PATH = process.env.PATH || process.env.Path || process.env.path -const { delimiter } = require('path') -module.exports = PATH.split(delimiter) diff --git a/test/lib/commands/bin.js b/test/lib/commands/bin.js index a889b13361624..46170e765de82 100644 --- a/test/lib/commands/bin.js +++ b/test/lib/commands/bin.js @@ -1,60 +1,32 @@ const t = require('tap') -const { relative, join } = require('path') const { load: loadMockNpm } = require('../../fixtures/mock-npm') const mockGlobals = require('../../fixtures/mock-globals') -const mockBin = async (t, { args = [], config = {} } = {}) => { - const { npm, outputs, ...rest } = await loadMockNpm(t, { - config, - }) - const cmd = await npm.cmd('bin') - await npm.exec('bin', args) - - return { - npm, - cmd, - bin: outputs[0][0], - ...rest, - } -} - -t.test('bin', async t => { - const { cmd, bin, prefix, outputErrors } = await mockBin(t, { - config: { global: false }, - }) - - t.match(cmd.usage, 'bin', 'usage has command name in it') - t.equal(relative(prefix, bin), join('node_modules/.bin'), 'prints the correct directory') - t.strictSame(outputErrors, []) +t.test('bin not global', async t => { + const { npm, joinedOutput, logs } = await loadMockNpm(t) + await npm.exec('bin', []) + t.match(joinedOutput(), npm.localBin) + t.match(logs.error, []) }) -t.test('bin -g', async t => { - mockGlobals(t, { 'process.platform': 'posix' }) - const { globalPrefix, bin, outputErrors } = await mockBin(t, { +t.test('bin global in env.path', async t => { + const { npm, joinedOutput, logs } = await loadMockNpm(t, { config: { global: true }, }) - t.equal(relative(globalPrefix, bin), 'bin', 'prints the correct directory') - t.strictSame(outputErrors, []) -}) - -t.test('bin -g win32', async t => { - mockGlobals(t, { 'process.platform': 'win32' }) - const { globalPrefix, bin, outputErrors } = await mockBin(t, { - config: { global: true }, + mockGlobals(t, { + 'process.env': { path: npm.globalBin }, }) - - t.equal(relative(globalPrefix, bin), '', 'prints the correct directory') - t.strictSame(outputErrors, []) + await npm.exec('bin', []) + t.match(joinedOutput(), npm.globalBin) + t.match(logs.error, []) }) -t.test('bin -g (not in path)', async t => { - const { logs } = await mockBin(t, { +t.test('bin not in path', async t => { + const { npm, joinedOutput, logs } = await loadMockNpm(t, { config: { global: true }, - globals: { - 'process.env.PATH': 'emptypath', - }, }) - - t.strictSame(logs.error[0], ['bin', '(not in PATH env variable)']) + await npm.exec('bin', []) + t.match(joinedOutput(), npm.globalBin) + t.match(logs.error, [['bin', '(not in PATH env variable)']]) }) diff --git a/test/lib/commands/exec.js b/test/lib/commands/exec.js index 3c75c1d8d8273..1f7230d25b654 100644 --- a/test/lib/commands/exec.js +++ b/test/lib/commands/exec.js @@ -72,8 +72,6 @@ const read = (options, cb) => { process.nextTick(() => cb(READ_ERROR, READ_RESULT)) } -const PATH = require('../../../lib/utils/path.js') - let CI_NAME = 'travis-ci' const log = { @@ -154,7 +152,7 @@ t.test('npx foo, bin already exists locally', async t => { stdioString: true, event: 'npx', env: { - PATH: [npm.localBin, ...PATH].join(delimiter), + PATH: [npm.localBin, process.env.PATH].join(delimiter), }, stdio: 'inherit', }, @@ -183,7 +181,7 @@ t.test('npx foo, bin already exists globally', async t => { stdioString: true, event: 'npx', env: { - PATH: [npm.globalBin, ...PATH].join(delimiter), + PATH: [npm.globalBin, process.env.PATH].join(delimiter), }, stdio: 'inherit', }, @@ -1175,7 +1173,7 @@ t.test('workspaces', t => { stdioString: true, event: 'npx', env: { - PATH: [npm.localBin, ...PATH].join(delimiter), + PATH: [npm.localBin, process.env.PATH].join(delimiter), }, stdio: 'inherit', }, diff --git a/test/lib/utils/path.js b/test/lib/utils/path.js deleted file mode 100644 index b3bb269f32a62..0000000000000 --- a/test/lib/utils/path.js +++ /dev/null @@ -1,13 +0,0 @@ -const t = require('tap') -const mod = '../../../lib/utils/path.js' -const { isWindows } = require('../../../lib/utils/is-windows.js') -const delim = isWindows ? ';' : ':' -Object.defineProperty(process, 'env', { - value: {}, -}) -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'])