diff --git a/lib/cli.js b/lib/cli.js index 106dd629d4e19..9544f8451f8ae 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -1,5 +1,5 @@ // Separated out for easier unit testing -module.exports = (process) => { +module.exports = async (process) => { // set it here so that regardless of what happens later, we don't // leak any private CLI configs to other programs process.title = 'npm' @@ -39,12 +39,8 @@ module.exports = (process) => { // now actually fire up npm and run the command. // this is how to use npm programmatically: - npm.load(async er => { - // Any exceptions here will be picked up by the uncaughtException handler - if (er) - return exitHandler(er) - - // npm --version=cli + try { + await npm.load() if (npm.config.get('version', 'cli')) { npm.output(npm.version) return exitHandler() @@ -75,5 +71,7 @@ module.exports = (process) => { } impl(npm.argv, exitHandler) - }) + } catch (err) { + return exitHandler(err) + } } diff --git a/lib/npm.js b/lib/npm.js index 937459501c0a5..7046a84d0bcfa 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -11,6 +11,7 @@ const Config = require('@npmcli/config') // Patch the global fs module here at the app level require('graceful-fs').gracefulify(require('fs')) +// TODO make this only ever load once (or unload) in tests const procLogListener = require('./utils/proc-log-listener.js') const proxyCmds = new Proxy({}, { @@ -48,6 +49,7 @@ const _title = Symbol('_title') const npm = module.exports = new class extends EventEmitter { constructor () { super() + // TODO make this only ever load once (or unload) in tests require('./utils/perf.js') this.started = Date.now() this.command = null @@ -77,8 +79,8 @@ const npm = module.exports = new class extends EventEmitter { [_runCmd] (cmd, impl, args, cb) { if (!this.loaded) { throw new Error( - 'Call npm.load(cb) before using this command.\n' + - 'See the README.md or bin/npm-cli.js for example usage.' + 'Call npm.load() before using this command.\n' + + 'See lib/cli.js for example usage.' ) } @@ -96,7 +98,7 @@ const npm = module.exports = new class extends EventEmitter { args.filter(arg => /^[\u2010-\u2015\u2212\uFE58\uFE63\uFF0D]/.test(arg)) .forEach(arg => { warnedNonDashArg = true - log.error('arg', 'Argument starts with non-ascii dash, this is probably invalid:', arg) + this.log.error('arg', 'Argument starts with non-ascii dash, this is probably invalid:', arg) }) } @@ -123,33 +125,32 @@ const npm = module.exports = new class extends EventEmitter { } } - // call with parsed CLI options and a callback when done loading - // XXX promisify this and stop taking a callback load (cb) { - if (!cb || typeof cb !== 'function') - throw new TypeError('must call as: npm.load(callback)') - - this.once('load', cb) - if (this.loaded || this.loadErr) { - this.emit('load', this.loadErr) - return + if (cb && typeof cb !== 'function') + throw new TypeError('callback must be a function if provided') + + if (!this.loadPromise) { + process.emit('time', 'npm:load') + this.log.pause() + this.loadPromise = new Promise((resolve, reject) => { + this[_load]().catch(er => er).then((er) => { + this.loadErr = er + if (!er && this.config.get('force')) + this.log.warn('using --force', 'Recommended protections disabled.') + + process.emit('timeEnd', 'npm:load') + if (er) + return reject(er) + resolve() + }) + }) } - if (this.loading) - return - - this.loading = true + if (!cb) + return this.loadPromise - process.emit('time', 'npm:load') - this.log.pause() - return this[_load]().catch(er => er).then((er) => { - this.loading = false - this.loadErr = er - if (!er && this.config.get('force')) - this.log.warn('using --force', 'Recommended protections disabled.') - - process.emit('timeEnd', 'npm:load') - this.emit('load', er) - }) + // loadPromise is returned here for legacy purposes, old code was allowing + // the mixing of callback and promise here. + return this.loadPromise.then(cb, cb) } get loaded () { @@ -167,10 +168,15 @@ const npm = module.exports = new class extends EventEmitter { async [_load] () { process.emit('time', 'npm:load:whichnode') - const node = await which(process.argv[0]).catch(er => null) + let node + try { + node = which.sync(process.argv[0]) + } catch (_) { + // TODO should we throw here? + } process.emit('timeEnd', 'npm:load:whichnode') if (node && node.toUpperCase() !== process.execPath.toUpperCase()) { - log.verbose('node symlink', node) + this.log.verbose('node symlink', node) process.execPath = node this.config.execPath = node } @@ -198,10 +204,10 @@ const npm = module.exports = new class extends EventEmitter { process.env.COLOR = this.color ? '1' : '0' process.emit('time', 'npm:load:cleanupLog') - cleanUpLogFiles(this.cache, this.config.get('logs-max'), log.warn) + cleanUpLogFiles(this.cache, this.config.get('logs-max'), this.log.warn) process.emit('timeEnd', 'npm:load:cleanupLog') - log.resume() + this.log.resume() process.emit('time', 'npm:load:configScope') const configScope = this.config.get('scope') @@ -314,9 +320,8 @@ const npm = module.exports = new class extends EventEmitter { // now load everything required by the class methods const log = require('npmlog') -const { promisify } = require('util') -const which = promisify(require('which')) +const which = require('which') const deref = require('./utils/deref-command.js') const setupLog = require('./utils/setup-log.js') diff --git a/lib/utils/perf.js b/lib/utils/perf.js index 3f81ee4b049e4..4961054d909ad 100644 --- a/lib/utils/perf.js +++ b/lib/utils/perf.js @@ -14,3 +14,10 @@ process.on('timeEnd', (name) => { } else log.silly('timing', "Tried to end timer that doesn't exist:", name) }) + +// for tests +/* istanbul ignore next */ +exports.reset = () => { + process.removeAllListeners('time') + process.removeAllListeners('timeEnd') +} diff --git a/lib/utils/proc-log-listener.js b/lib/utils/proc-log-listener.js index 1dc4b4399eaea..2cfe94ecb0cf2 100644 --- a/lib/utils/proc-log-listener.js +++ b/lib/utils/proc-log-listener.js @@ -14,3 +14,9 @@ module.exports = () => { } }) } + +// for tests +/* istanbul ignore next */ +module.exports.reset = () => { + process.removeAllListeners('log') +} diff --git a/test/lib/cli.js b/test/lib/cli.js index 132d4224b6b79..89c3736e886d2 100644 --- a/test/lib/cli.js +++ b/test/lib/cli.js @@ -1,23 +1,8 @@ const t = require('tap') -let LOAD_ERROR = null -const npmOutputs = [] -const npmock = { - log: { level: 'silent' }, - output: (...msg) => npmOutputs.push(msg), - usage: 'npm usage test example', - version: '99.99.99', - load: cb => cb(LOAD_ERROR), - argv: [], - config: { - settings: {}, - get: (k) => npmock.config.settings[k], - set: (k, v) => { - npmock.config.settings[k] = v - }, - }, - commands: {}, -} +// NOTE lib/npm.js is wrapped in t.mock() every time because it's currently a +// singleton. In the next semver major we will export the class and lib/cli.js +// can call `new` on it and then we won't have to do that anymore const unsupportedMock = { checkForBrokenNode: () => {}, @@ -43,166 +28,130 @@ const npmlogMock = { info: (...msg) => logs.push(['info', ...msg]), } -const cli = t.mock('../../lib/cli.js', { - '../../lib/npm.js': npmock, +const cliMock = (npm) => t.mock('../../lib/cli.js', { + '../../lib/npm.js': npm, '../../lib/utils/update-notifier.js': async () => null, - '../../lib/utils/did-you-mean.js': () => '\ntest did you mean', '../../lib/utils/unsupported.js': unsupportedMock, '../../lib/utils/exit-handler.js': exitHandlerMock, npmlog: npmlogMock, }) -t.test('print the version, and treat npm_g to npm -g', t => { - t.teardown(() => { - delete npmock.config.settings.version - process.argv = argv - npmock.argv.length = 0 - proc.argv.length = 0 - logs.length = 0 - npmOutputs.length = 0 - exitHandlerCalled = null - }) +const npmOutputs = [] +const npmMock = () => { + const npm = t.mock('../../lib/npm.js') + npm.output = (...msg) => npmOutputs.push(msg) + return npm +} - const { argv } = process - const proc = { - argv: ['node', 'npm_g', '-v'], - version: '420.69.lol', +const processMock = (proc) => { + const mocked = { + ...process, on: () => {}, + ...proc, } - process.argv = proc.argv - npmock.config.settings.version = true + // nopt looks at process directly + process.argv = mocked.argv + return mocked +} + +const { argv } = process - cli(proc) +t.afterEach(() => { + logs.length = 0 + process.argv = argv + npmOutputs.length = 0 + exitHandlerCalled = null + exitHandlerNpm = null +}) - t.strictSame(npmock.argv, []) - t.strictSame(proc.argv, ['node', 'npm', '-g', '-v']) +t.test('print the version, and treat npm_g as npm -g', async t => { + const proc = processMock({ + argv: ['node', 'npm_g', '-v'], + version: process.version, + }) + + const npm = npmMock() + const cli = cliMock(npm) + await cli(proc) + + t.strictSame(proc.argv, ['node', 'npm', '-g', '-v'], 'npm process.argv was rewritten') + t.strictSame(process.argv, ['node', 'npm', '-g', '-v'], 'system process.argv was rewritten') t.strictSame(logs, [ 'pause', - ['verbose', 'cli', ['node', 'npm', '-g', '-v']], - ['info', 'using', 'npm@%s', '99.99.99'], - ['info', 'using', 'node@%s', '420.69.lol'], + ['verbose', 'cli', proc.argv], + ['info', 'using', 'npm@%s', npm.version], + ['info', 'using', 'node@%s', process.version], ]) - t.strictSame(npmOutputs, [['99.99.99']]) + t.strictSame(npmOutputs, [[npm.version]]) t.strictSame(exitHandlerCalled, []) - - t.end() }) -t.test('calling with --versions calls npm version with no args', t => { - const processArgv = process.argv - const proc = { +t.test('calling with --versions calls npm version with no args', async t => { + const proc = processMock({ argv: ['node', 'npm', 'install', 'or', 'whatever', '--versions'], - on: () => {}, - } - process.argv = proc.argv - npmock.config.set('versions', true) - - t.teardown(() => { - delete npmock.config.settings.versions - process.argv = processArgv - npmock.argv.length = 0 - proc.argv.length = 0 - logs.length = 0 - npmOutputs.length = 0 - exitHandlerCalled = null - delete npmock.commands.version }) + const npm = npmMock() + const cli = cliMock(npm) - npmock.commands.version = (args, cb) => { - t.equal(proc.title, 'npm') - t.strictSame(npmock.argv, []) - t.strictSame(proc.argv, ['node', 'npm', 'install', 'or', 'whatever', '--versions']) - t.strictSame(logs, [ - 'pause', - ['verbose', 'cli', ['node', 'npm', 'install', 'or', 'whatever', '--versions']], - ['info', 'using', 'npm@%s', '99.99.99'], - ['info', 'using', 'node@%s', undefined], - ]) - - t.strictSame(npmOutputs, []) - t.strictSame(exitHandlerCalled, null) - - t.strictSame(args, []) - t.end() + let versionArgs + npm.commands.version = (args, cb) => { + versionArgs = args + cb() } - cli(proc) -}) + await cli(proc) + t.strictSame(versionArgs, []) + t.equal(proc.title, 'npm') + t.strictSame(npm.argv, []) + t.strictSame(logs, [ + 'pause', + ['verbose', 'cli', proc.argv], + ['info', 'using', 'npm@%s', npm.version], + ['info', 'using', 'node@%s', process.version], + ]) -t.test('print usage if no params provided', t => { - const { output } = npmock - t.teardown(() => { - npmock.output = output - }) - const proc = { - argv: ['node', 'npm'], - on: () => {}, - } - npmock.argv = [] - npmock.output = (msg) => { - if (msg) { - t.match(msg, 'npm usage test example', 'outputs npm usage') - t.end() - } - } - cli(proc) + t.strictSame(npmOutputs, []) + t.strictSame(exitHandlerCalled, []) }) -t.test('print usage if non-command param provided', t => { - const { output } = npmock - t.teardown(() => { - npmock.output = output +t.test('print usage if no params provided', async t => { + const proc = processMock({ + argv: ['node', 'npm'], }) - const proc = { - argv: ['node', 'npm', 'asdf'], - on: () => {}, - } - npmock.argv = ['asdf'] - npmock.output = (msg) => { - if (msg) { - t.match(msg, 'Unknown command: "asdf"\ntest did you mean', 'outputs did you mean') - t.end() - } - } - cli(proc) + + const npm = npmMock() + const cli = cliMock(npm) + await cli(proc) + t.match(npmOutputs[0][0], 'Usage:', 'outputs npm usage') + t.match(exitHandlerCalled, [], 'should call exitHandler with no args') + t.ok(exitHandlerNpm, 'exitHandler npm is set') + t.match(proc.exitCode, 1) }) -t.test('gracefully handles error printing usage', t => { - const { output } = npmock - t.teardown(() => { - npmock.output = output - exitHandlerCb = null - exitHandlerCalled = null +t.test('print usage if non-command param provided', async t => { + const proc = processMock({ + argv: ['node', 'npm', 'tset'], }) - const proc = { - argv: ['node', 'npm'], - on: () => {}, - } - npmock.argv = [] - exitHandlerCb = () => { - t.match(exitHandlerCalled, [], 'should call exitHandler with no args') - t.match(exitHandlerNpm, npmock, 'exitHandler npm is set') - t.end() - } - cli(proc) + + const npm = npmMock() + const cli = cliMock(npm) + await cli(proc) + t.match(npmOutputs[0][0], 'Unknown command: "tset"') + t.match(npmOutputs[0][0], 'Did you mean this?') + t.match(exitHandlerCalled, [], 'should call exitHandler with no args') + t.ok(exitHandlerNpm, 'exitHandler npm is set') + t.match(proc.exitCode, 1) }) -t.test('load error calls error handler', t => { - t.teardown(() => { - exitHandlerCb = null - exitHandlerCalled = null - LOAD_ERROR = null +t.test('load error calls error handler', async t => { + const proc = processMock({ + argv: ['node', 'npm', 'asdf'], }) + const npm = npmMock() + const cli = cliMock(npm) const er = new Error('test load error') - LOAD_ERROR = er - const proc = { - argv: ['node', 'npm', 'asdf'], - on: () => {}, - } - exitHandlerCb = () => { - t.strictSame(exitHandlerCalled, [er]) - t.end() - } - cli(proc) + npm.load = () => Promise.reject(er) + await cli(proc) + t.strictSame(exitHandlerCalled, [er]) }) diff --git a/test/lib/npm.js b/test/lib/npm.js index 6909c43e4ff0e..1ccfbbd7ea507 100644 --- a/test/lib/npm.js +++ b/test/lib/npm.js @@ -1,8 +1,13 @@ const t = require('tap') +const npmlog = require('npmlog') +const perf = require('../../lib/utils/perf.js') +const procLog = require('../../lib/utils/proc-log-listener.js') + // delete this so that we don't have configs from the fact that it // is being run by 'npm test' const event = process.env.npm_lifecycle_event + for (const env of Object.keys(process.env).filter(e => /^npm_/.test(e))) { if (env === 'npm_command') { // should only be running this in the 'test' or 'run-script' command! @@ -23,52 +28,57 @@ for (const env of Object.keys(process.env).filter(e => /^npm_/.test(e))) { const { resolve, dirname } = require('path') const actualPlatform = process.platform - const beWindows = () => { Object.defineProperty(process, 'platform', { value: 'win32', configurable: true, }) } - const bePosix = () => { Object.defineProperty(process, 'platform', { value: 'posix', configurable: true, }) } +const argv = [...process.argv] -const npmlog = require('npmlog') +const realLog = {} +for (const level of ['silly', 'verbose', 'timing', 'notice', 'warn', 'error']) + realLog[level] = npmlog[level] -const npmPath = resolve(__dirname, '..', '..') -const Config = require('@npmcli/config') -const { definitions, shorthands, flatten } = require('../../lib/utils/config') -const freshConfig = (opts = {}) => { +t.afterEach(() => { for (const env of Object.keys(process.env).filter(e => /^npm_/.test(e))) delete process.env[env] - process.env.npm_config_cache = CACHE - - npm.config = new Config({ - definitions, - shorthands, - npmPath, - log: npmlog, - ...opts, - flatten, + process.argv = argv + Object.defineProperty(process, 'platform', { + value: actualPlatform, + configurable: true, }) -} - -const logs = [] -for (const level of ['silly', 'verbose', 'timing', 'notice', 'warn', 'error']) - npmlog[level] = (...msg) => logs.push([level, ...msg]) + for (const level of ['silly', 'verbose', 'timing', 'notice', 'warn', 'error']) + npmlog[level] = realLog[level] + perf.reset() + procLog.reset() +}) -const npm = require('../../lib/npm.js') +const npmMock = () => { + const logs = [] + const outputs = [] + const npm = t.mock('../../lib/npm.js') + for (const level of ['silly', 'verbose', 'timing', 'notice', 'warn', 'error']) { + npmlog[level] = (...msg) => { + logs.push([level, ...msg]) + } + } + npm.output = (...msg) => outputs.push(msg) + return { npm, logs, outputs } +} const CACHE = t.testdir() process.env.npm_config_cache = CACHE t.test('not yet loaded', t => { + const { npm, logs } = npmMock() t.match(npm, { started: Number, command: null, @@ -89,160 +99,125 @@ t.test('not yet loaded', t => { t.equal(npm.commands.asdfasdf, undefined) t.equal(npm.deref('list'), 'ls') t.same(logs, []) - logs.length = 0 t.end() }) t.test('npm.load', t => { - t.test('must be called with proper args', t => { - const er = new TypeError('must call as: npm.load(callback)') - t.throws(() => npm.load(), er) + t.test('callback must be a function', t => { + const { npm, logs } = npmMock() + const er = new TypeError('callback must be a function if provided') t.throws(() => npm.load({}), er) t.same(logs, []) - logs.length = 0 t.end() }) - t.test('load error', t => { - const { load } = npm.config + t.test('callback style', t => { + const { npm } = npmMock() + npm.load((err) => { + if (err) + throw err + t.ok(npm.loaded) + t.end() + }) + }) + + t.test('load error', async t => { + const { npm } = npmMock() const loadError = new Error('load error') npm.config.load = async () => { throw loadError } - npm.load(er => { + await npm.load().catch(er => { t.equal(er, loadError) t.equal(npm.loadErr, loadError) - npm.config.load = load - // loading again just returns the same error - npm.load(er => { - t.equal(er, loadError) - t.equal(npm.loadErr, loadError) - npm.loadErr = null - t.end() - }) + }) + npm.config.load = async () => { + throw new Error('new load error') + } + await npm.load().catch(er => { + t.equal(er, loadError, 'loading again returns the original error') + t.equal(npm.loadErr, loadError) }) }) - t.test('basic loading', t => { + t.test('basic loading', async t => { + const { npm, logs } = npmMock() const dir = t.testdir({ node_modules: {}, }) - let firstCalled = false - const first = (er) => { - if (er) - throw er - - firstCalled = true - t.equal(npm.loaded, true) - t.equal(npm.config.loaded, true) - t.equal(npm.config.get('force'), false) - } - - let secondCalled = false - const second = () => { - secondCalled = true - } - - t.equal(npm.loading, false, 'not loading yet') - const p = npm.load(first).then(() => { - t.ok(npm.usage, 'has usage') - npm.config.set('prefix', dir) - t.match(npm, { - loaded: true, - loading: false, - flatOptions: {}, - }) - t.equal(firstCalled, true, 'first callback got called') - t.equal(secondCalled, true, 'second callback got called') - let thirdCalled = false - const third = () => { - thirdCalled = true - } - npm.load(third) - t.equal(thirdCalled, true, 'third callbback got called') - t.match(logs, [ - ['timing', 'npm:load', /Completed in [0-9.]+ms/], - ]) - logs.length = 0 - - bePosix() - t.equal(resolve(npm.cache), resolve(CACHE), 'cache is cache') - const newCache = t.testdir() - npm.cache = newCache - t.equal(npm.config.get('cache'), newCache, 'cache setter sets config') - t.equal(npm.cache, newCache, 'cache getter gets new config') - t.equal(npm.log, npmlog, 'npmlog getter') - t.equal(npm.lockfileVersion, 2, 'lockfileVersion getter') - t.equal(npm.prefix, npm.localPrefix, 'prefix is local prefix') - t.not(npm.prefix, npm.globalPrefix, 'prefix is not global prefix') - npm.globalPrefix = npm.prefix - t.equal(npm.prefix, npm.globalPrefix, 'globalPrefix setter') - npm.localPrefix = dir + '/extra/prefix' - t.equal(npm.prefix, npm.localPrefix, 'prefix is local prefix after localPrefix setter') - t.not(npm.prefix, npm.globalPrefix, 'prefix is not global prefix after localPrefix setter') - - npm.prefix = dir + '/some/prefix' - t.equal(npm.prefix, npm.localPrefix, 'prefix is local prefix after prefix setter') - t.not(npm.prefix, npm.globalPrefix, 'prefix is not global prefix after prefix setter') - t.equal(npm.bin, npm.localBin, 'bin is local bin after prefix setter') - t.not(npm.bin, npm.globalBin, 'bin is not global bin after prefix setter') - t.equal(npm.dir, npm.localDir, 'dir is local dir after prefix setter') - t.not(npm.dir, npm.globalDir, 'dir is not global dir after prefix setter') - - npm.config.set('global', true) - t.equal(npm.prefix, npm.globalPrefix, 'prefix is global prefix after setting global') - t.not(npm.prefix, npm.localPrefix, 'prefix is not local prefix after setting global') - t.equal(npm.bin, npm.globalBin, 'bin is global bin after setting global') - t.not(npm.bin, npm.localBin, 'bin is not local bin after setting global') - t.equal(npm.dir, npm.globalDir, 'dir is global dir after setting global') - t.not(npm.dir, npm.localDir, 'dir is not local dir after setting global') - - npm.prefix = dir + '/new/global/prefix' - t.equal(npm.prefix, npm.globalPrefix, 'prefix is global prefix after prefix setter') - t.not(npm.prefix, npm.localPrefix, 'prefix is not local prefix after prefix setter') - t.equal(npm.bin, npm.globalBin, 'bin is global bin after prefix setter') - t.not(npm.bin, npm.localBin, 'bin is not local bin after prefix setter') - - beWindows() - t.equal(npm.bin, npm.globalBin, 'bin is global bin in windows mode') - t.equal(npm.dir, npm.globalDir, 'dir is global dir in windows mode') - bePosix() - - const tmp = npm.tmp - t.match(tmp, String, 'npm.tmp is a string') - t.equal(tmp, npm.tmp, 'getter only generates it once') + await npm.load() + t.equal(npm.loaded, true) + t.equal(npm.config.loaded, true) + t.equal(npm.config.get('force'), false) + t.ok(npm.usage, 'has usage') + npm.config.set('prefix', dir) + + t.match(npm, { + flatOptions: {}, }) - - t.equal(npm.loaded, false, 'not loaded yet') - t.equal(npm.loading, true, 'working on it tho') - t.type(p, Promise, 'npm.load() returned a Promise first time') - t.equal(npm.load(second), undefined, - 'npm.load() returns nothing second time') - - return p + t.match(logs, [ + ['timing', 'npm:load', /Completed in [0-9.]+ms/], + ]) + + bePosix() + t.equal(resolve(npm.cache), resolve(CACHE), 'cache is cache') + const newCache = t.testdir() + npm.cache = newCache + t.equal(npm.config.get('cache'), newCache, 'cache setter sets config') + t.equal(npm.cache, newCache, 'cache getter gets new config') + t.equal(npm.log, npmlog, 'npmlog getter') + t.equal(npm.lockfileVersion, 2, 'lockfileVersion getter') + t.equal(npm.prefix, npm.localPrefix, 'prefix is local prefix') + t.not(npm.prefix, npm.globalPrefix, 'prefix is not global prefix') + npm.globalPrefix = npm.prefix + t.equal(npm.prefix, npm.globalPrefix, 'globalPrefix setter') + npm.localPrefix = dir + '/extra/prefix' + t.equal(npm.prefix, npm.localPrefix, 'prefix is local prefix after localPrefix setter') + t.not(npm.prefix, npm.globalPrefix, 'prefix is not global prefix after localPrefix setter') + + npm.prefix = dir + '/some/prefix' + t.equal(npm.prefix, npm.localPrefix, 'prefix is local prefix after prefix setter') + t.not(npm.prefix, npm.globalPrefix, 'prefix is not global prefix after prefix setter') + t.equal(npm.bin, npm.localBin, 'bin is local bin after prefix setter') + t.not(npm.bin, npm.globalBin, 'bin is not global bin after prefix setter') + t.equal(npm.dir, npm.localDir, 'dir is local dir after prefix setter') + t.not(npm.dir, npm.globalDir, 'dir is not global dir after prefix setter') + + npm.config.set('global', true) + t.equal(npm.prefix, npm.globalPrefix, 'prefix is global prefix after setting global') + t.not(npm.prefix, npm.localPrefix, 'prefix is not local prefix after setting global') + t.equal(npm.bin, npm.globalBin, 'bin is global bin after setting global') + t.not(npm.bin, npm.localBin, 'bin is not local bin after setting global') + t.equal(npm.dir, npm.globalDir, 'dir is global dir after setting global') + t.not(npm.dir, npm.localDir, 'dir is not local dir after setting global') + + npm.prefix = dir + '/new/global/prefix' + t.equal(npm.prefix, npm.globalPrefix, 'prefix is global prefix after prefix setter') + t.not(npm.prefix, npm.localPrefix, 'prefix is not local prefix after prefix setter') + t.equal(npm.bin, npm.globalBin, 'bin is global bin after prefix setter') + t.not(npm.bin, npm.localBin, 'bin is not local bin after prefix setter') + + beWindows() + t.equal(npm.bin, npm.globalBin, 'bin is global bin in windows mode') + t.equal(npm.dir, npm.globalDir, 'dir is global dir in windows mode') + bePosix() + + const tmp = npm.tmp + t.match(tmp, String, 'npm.tmp is a string') + t.equal(tmp, npm.tmp, 'getter only generates it once') }) - t.test('forceful loading', t => { - // also, don't get thrown off if argv[0] isn't found for some reason - const [argv0] = process.argv - t.teardown(() => { - process.argv[0] = argv0 - }) - freshConfig({ argv: [...process.argv, '--force', '--color', 'always'] }) - process.argv[0] = 'this exe does not exist or else this test will fail' - return npm.load(er => { - if (er) - throw er - - t.match(logs.filter(l => l[0] !== 'timing'), [ - [ - 'warn', - 'using --force', - 'Recommended protections disabled.', - ], - ]) - logs.length = 0 - }) + t.test('forceful loading', async t => { + process.argv = [...process.argv, '--force', '--color', 'always'] + const { npm, logs } = npmMock() + await npm.load() + t.match(logs.filter(l => l[0] !== 'timing'), [ + [ + 'warn', + 'using --force', + 'Recommended protections disabled.', + ], + ]) }) t.test('node is a symlink', async t => { @@ -254,7 +229,7 @@ t.test('npm.load', t => { const PATH = process.env.PATH || process.env.Path process.env.PATH = resolve(dir, 'bin') - const { execPath, argv: processArgv } = process + const { execPath } = process process.argv = [ node, process.argv[1], @@ -267,48 +242,34 @@ t.test('npm.load', t => { 'blergggg', ] - freshConfig() - const { log } = console - const consoleLogs = [] - console.log = (...msg) => consoleLogs.push(msg) - t.teardown(() => { - console.log = log process.env.PATH = PATH - process.argv = processArgv - freshConfig() - logs.length = 0 process.execPath = execPath }) - logs.length = 0 - - await npm.load(er => { - if (er) - throw er - - t.equal(npm.config.get('scope'), '@foo', 'added the @ sign to scope') - t.match(logs.filter(l => l[0] !== 'timing' || !/^config:/.test(l[1])), [ - [ - 'timing', - 'npm:load:whichnode', - /Completed in [0-9.]+ms/, - ], - [ - 'verbose', - 'node symlink', - resolve(dir, 'bin', node), - ], - [ - 'timing', - 'npm:load', - /Completed in [0-9.]+ms/, - ], - ]) - logs.length = 0 - t.equal(process.execPath, resolve(dir, 'bin', node)) - }) + const { npm, logs, outputs } = npmMock() + await npm.load() + t.equal(npm.config.get('scope'), '@foo', 'added the @ sign to scope') + t.match(logs.filter(l => l[0] !== 'timing' || !/^config:/.test(l[1])), [ + [ + 'timing', + 'npm:load:whichnode', + /Completed in [0-9.]+ms/, + ], + [ + 'verbose', + 'node symlink', + resolve(dir, 'bin', node), + ], + [ + 'timing', + 'npm:load', + /Completed in [0-9.]+ms/, + ], + ]) + t.equal(process.execPath, resolve(dir, 'bin', node)) + outputs.length = 0 await npm.commands.ll([], (er) => { if (er) throw er @@ -316,13 +277,13 @@ t.test('npm.load', t => { t.equal(npm.command, 'll', 'command set to first npm command') t.equal(npm.flatOptions.npmCommand, 'll', 'npmCommand flatOption set') - t.same(consoleLogs, [[npm.commands.ll.usage]], 'print usage') - consoleLogs.length = 0 + t.same(outputs, [[npm.commands.ll.usage]], 'print usage') npm.config.set('usage', false) t.equal(npm.commands.ll, npm.commands.ll, 'same command, different name') - logs.length = 0 }) + outputs.length = 0 + logs.length = 0 await npm.commands.get(['scope', '\u2010not-a-dash'], (er) => { if (er) throw er @@ -348,7 +309,7 @@ t.test('npm.load', t => { /Completed in [0-9.]+ms/, ], ]) - t.same(consoleLogs, [['scope=@foo\n\u2010not-a-dash=undefined']]) + t.same(outputs, [['scope=@foo\n\u2010not-a-dash=undefined']]) }) // need this here or node 10 will improperly end the promise ahead of time @@ -381,33 +342,21 @@ t.test('npm.load', t => { '.npmrc': '', }) - const { log } = console - const consoleLogs = [] - console.log = (...msg) => consoleLogs.push(msg) - const { execPath } = process - t.teardown(() => { - console.log = log - }) - freshConfig({ - argv: [ - execPath, - process.argv[1], - '--userconfig', - resolve(dir, '.npmrc'), - '--color', - 'false', - '--workspaces', - 'true', - ], - }) - - await npm.load(er => { - if (er) - throw er - }) + process.argv = [ + execPath, + process.argv[1], + '--userconfig', + resolve(dir, '.npmrc'), + '--color', + 'false', + '--workspaces', + 'true', + ] + const { npm, outputs } = npmMock() + await npm.load() npm.localPrefix = dir await new Promise((res, rej) => { @@ -421,7 +370,7 @@ t.test('npm.load', t => { t.equal(npm.command, 'run-script', 'npm.command set to canonical name') t.match( - consoleLogs, + outputs, [ ['Lifecycle scripts included in a@1.0.0:'], [' test\n echo test a'], @@ -463,23 +412,19 @@ t.test('npm.load', t => { }), }) const { execPath } = process - freshConfig({ - argv: [ - execPath, - process.argv[1], - '--userconfig', - resolve(dir, '.npmrc'), - '--color', - 'false', - '--workspaces', - '--global', - 'true', - ], - }) - await npm.load(er => { - if (er) - throw er - }) + process.argv = [ + execPath, + process.argv[1], + '--userconfig', + resolve(dir, '.npmrc'), + '--color', + 'false', + '--workspaces', + '--global', + 'true', + ] + const { npm } = npmMock() + await npm.load() npm.localPrefix = dir await new Promise((res, rej) => { // verify that calling the command with a short name still sets @@ -491,7 +436,6 @@ t.test('npm.load', t => { }) }) }) - t.end() }) @@ -512,82 +456,49 @@ t.test('loading as main will load the cli', t => { }) t.test('set process.title', t => { - const { argv: processArgv } = process - const { log } = console - const titleDesc = Object.getOwnPropertyDescriptor(process, 'title') - Object.defineProperty(process, 'title', { - value: '', - settable: true, - enumerable: true, - configurable: true, - }) - const consoleLogs = [] - console.log = (...msg) => consoleLogs.push(msg) - - t.teardown(() => { - console.log = log - process.argv = processArgv - Object.defineProperty(process, 'title', titleDesc) - freshConfig() - }) - - t.afterEach(() => consoleLogs.length = 0) - t.test('basic title setting', async t => { - freshConfig({ - argv: [ - process.execPath, - process.argv[1], - '--usage', - '--scope=foo', - 'ls', - ], - }) - await npm.load(er => { - if (er) - throw er - t.equal(npm.title, 'npm ls') - t.equal(process.title, 'npm ls') - }) + process.argv = [ + process.execPath, + process.argv[1], + '--usage', + '--scope=foo', + 'ls', + ] + const { npm } = npmMock() + await npm.load() + t.equal(npm.title, 'npm ls') + t.equal(process.title, 'npm ls') }) t.test('do not expose token being revoked', async t => { - freshConfig({ - argv: [ - process.execPath, - process.argv[1], - '--usage', - '--scope=foo', - 'token', - 'revoke', - 'deadbeefcafebad', - ], - }) - await npm.load(er => { - if (er) - throw er - t.equal(npm.title, 'npm token revoke ***') - t.equal(process.title, 'npm token revoke ***') - }) + process.argv = [ + process.execPath, + process.argv[1], + '--usage', + '--scope=foo', + 'token', + 'revoke', + 'deadbeefcafebad', + ] + const { npm } = npmMock() + await npm.load() + t.equal(npm.title, 'npm token revoke ***') + t.equal(process.title, 'npm token revoke ***') }) t.test('do show *** unless a token is actually being revoked', async t => { - freshConfig({ - argv: [ - process.execPath, - process.argv[1], - '--usage', - '--scope=foo', - 'token', - 'revoke', - ], - }) - await npm.load(er => { - if (er) - throw er - t.equal(npm.title, 'npm token revoke') - t.equal(process.title, 'npm token revoke') - }) + process.argv = [ + process.execPath, + process.argv[1], + '--usage', + '--scope=foo', + 'token', + 'revoke', + ] + const { npm } = npmMock() + await npm.load() + t.equal(npm.title, 'npm token revoke') + t.equal(process.title, 'npm token revoke') }) t.end() diff --git a/test/lib/utils/exit-handler.js b/test/lib/utils/exit-handler.js index beb1ef8c56690..72f94bfded7b0 100644 --- a/test/lib/utils/exit-handler.js +++ b/test/lib/utils/exit-handler.js @@ -355,7 +355,7 @@ t.test('it worked', (t) => { const _exit = process.exit process.exit = (code) => { process.exit = _exit - t.false(code, 'should exit with no code') + t.notOk(code, 'should exit with no code') const _info = npmlog.info npmlog.info = (msg) => {