Skip to content

Commit

Permalink
fix: consolidate path delimiter logic
Browse files Browse the repository at this point in the history
  • Loading branch information
wraithgar authored and lukekarrys committed Mar 28, 2022
1 parent 57d8f75 commit 0a957f5
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 70 deletions.
8 changes: 6 additions & 2 deletions lib/commands/bin.js
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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)')
}
}
Expand Down
5 changes: 0 additions & 5 deletions lib/utils/path.js

This file was deleted.

62 changes: 17 additions & 45 deletions test/lib/commands/bin.js
Original file line number Diff line number Diff line change
@@ -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)']])
})
8 changes: 3 additions & 5 deletions test/lib/commands/exec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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',
},
Expand Down Expand Up @@ -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',
},
Expand Down Expand Up @@ -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',
},
Expand Down
13 changes: 0 additions & 13 deletions test/lib/utils/path.js

This file was deleted.

0 comments on commit 0a957f5

Please # to comment.