diff options
author | Luke Karrys <luke@lukekarrys.com> | 2022-03-19 07:31:37 +0300 |
---|---|---|
committer | Nathan Fritz <fritzy@github.com> | 2022-03-24 23:21:44 +0300 |
commit | cc6c09431d7fe2db8ac1dc7a707f2dab7a7a1f83 (patch) | |
tree | 834f053afb6dc6ddf1ab4e36a6a5e8e9f4c8124b /test/lib | |
parent | 81afa5a8838c71a3a5037e2c8b4ae196e19fe0d7 (diff) |
feat: add logs-dir config to set custom logging location
This also allows logs-max to be set to 0 to disable log file writing.
Closes #4466
Closes #4206
Diffstat (limited to 'test/lib')
-rw-r--r-- | test/lib/cli.js | 155 | ||||
-rw-r--r-- | test/lib/commands/bin.js | 90 | ||||
-rw-r--r-- | test/lib/commands/doctor.js | 14 | ||||
-rw-r--r-- | test/lib/npm.js | 292 | ||||
-rw-r--r-- | test/lib/utils/exit-handler.js | 78 | ||||
-rw-r--r-- | test/lib/utils/log-file.js | 17 | ||||
-rw-r--r-- | test/lib/utils/replace-info.js | 40 | ||||
-rw-r--r-- | test/lib/utils/timers.js | 32 | ||||
-rw-r--r-- | test/lib/utils/update-notifier.js | 3 |
9 files changed, 416 insertions, 305 deletions
diff --git a/test/lib/cli.js b/test/lib/cli.js index f02c57d8c..b6606c69f 100644 --- a/test/lib/cli.js +++ b/test/lib/cli.js @@ -1,9 +1,8 @@ const t = require('tap') -const mockGlobals = require('../fixtures/mock-globals.js') const { load: loadMockNpm } = require('../fixtures/mock-npm.js') -const cliMock = async (t, mocks) => { +const cliMock = async (t, opts) => { let exitHandlerArgs = null let npm = null const exitHandlerMock = (...args) => { @@ -12,10 +11,9 @@ const cliMock = async (t, mocks) => { } exitHandlerMock.setNpm = _npm => npm = _npm - const { Npm, outputs, logMocks, logs } = await loadMockNpm(t, { mocks, init: false }) + const { Npm, outputs, logMocks, logs } = await loadMockNpm(t, { ...opts, init: false }) const cli = t.mock('../../lib/cli.js', { '../../lib/npm.js': Npm, - '../../lib/utils/update-notifier.js': async () => null, '../../lib/utils/unsupported.js': { checkForBrokenNode: () => {}, checkForUnsupportedNode: () => {}, @@ -31,6 +29,7 @@ const cliMock = async (t, mocks) => { exitHandlerCalled: () => exitHandlerArgs, exitHandlerNpm: () => npm, logs, + logsBy: (title) => logs.verbose.filter(([p]) => p === title).map(([p, ...rest]) => rest), } } @@ -39,17 +38,15 @@ t.afterEach(() => { }) t.test('print the version, and treat npm_g as npm -g', async t => { - mockGlobals(t, { - 'process.argv': ['node', 'npm_g', '-v'], + const { logsBy, logs, cli, Npm, outputs, exitHandlerCalled } = await cliMock(t, { + globals: { 'process.argv': ['node', 'npm_g', '-v'] }, }) - - const { logs, cli, Npm, outputs, exitHandlerCalled } = await cliMock(t) await cli(process) t.strictSame(process.argv, ['node', 'npm', '-g', '-v'], 'system process.argv was rewritten') - t.strictSame(logs.verbose.filter(([p]) => p !== 'logfile'), [ - ['cli', process.argv], - ]) + t.strictSame(logsBy('cli'), [['node npm']]) + t.strictSame(logsBy('title'), [['npm']]) + t.strictSame(logsBy('argv'), [['"--global" "--version"']]) t.strictSame(logs.info, [ ['using', 'npm@%s', Npm.version], ['using', 'node@%s', process.version], @@ -59,68 +56,82 @@ t.test('print the version, and treat npm_g as npm -g', async t => { }) t.test('calling with --versions calls npm version with no args', async t => { - t.plan(6) - mockGlobals(t, { - 'process.argv': ['node', 'npm', 'install', 'or', 'whatever', '--versions'], - }) - const { logs, cli, Npm, outputs, exitHandlerCalled } = await cliMock(t, { - '../../lib/commands/version.js': class Version { - async exec (args) { - t.strictSame(args, []) - } + const { logsBy, cli, outputs, exitHandlerCalled } = await cliMock(t, { + mocks: { + '../../lib/commands/version.js': class Version { + async exec (args) { + t.strictSame(args, []) + } + }, + }, + globals: { + 'process.argv': ['node', 'npm', 'install', 'or', 'whatever', '--versions'], }, }) - await cli(process) - t.equal(process.title, 'npm install or whatever') - t.strictSame(logs.verbose.filter(([p]) => p !== 'logfile'), [ - ['cli', process.argv], - ]) - t.strictSame(logs.info, [ - ['using', 'npm@%s', Npm.version], - ['using', 'node@%s', process.version], - ]) + t.equal(process.title, 'npm install or whatever') + t.strictSame(logsBy('cli'), [['node npm']]) + t.strictSame(logsBy('title'), [['npm install or whatever']]) + t.strictSame(logsBy('argv'), [['"install" "or" "whatever" "--versions"']]) t.strictSame(outputs, []) t.strictSame(exitHandlerCalled(), []) }) t.test('logged argv is sanitized', async t => { - mockGlobals(t, { - 'process.argv': [ - 'node', - 'npm', - 'version', - 'https://username:password@npmjs.org/test_url_with_a_password', - ], - }) - const { logs, cli, Npm } = await cliMock(t, { - '../../lib/commands/version.js': class Version { - async exec (args) {} + const { logsBy, cli } = await cliMock(t, { + mocks: { + '../../lib/commands/version.js': class Version { + async exec () {} + }, + }, + globals: { + 'process.argv': [ + 'node', + 'npm', + 'version', + '--registry', + 'https://u:password@npmjs.org/password', + ], }, }) await cli(process) - t.ok(process.title.startsWith('npm version https://username:***@npmjs.org')) - t.strictSame(logs.verbose.filter(([p]) => p !== 'logfile'), [ - [ - 'cli', - ['node', 'npm', 'version', 'https://username:***@npmjs.org/test_url_with_a_password'], - ], - ]) - t.strictSame(logs.info, [ - ['using', 'npm@%s', Npm.version], - ['using', 'node@%s', process.version], - ]) + t.equal(process.title, 'npm version') + t.strictSame(logsBy('cli'), [['node npm']]) + t.strictSame(logsBy('title'), [['npm version']]) + t.strictSame(logsBy('argv'), [['"version" "--registry" "https://u:***@npmjs.org/password"']]) }) -t.test('print usage if no params provided', async t => { - mockGlobals(t, { - 'process.argv': ['node', 'npm'], +t.test('logged argv is sanitized with equals', async t => { + const { logsBy, cli } = await cliMock(t, { + mocks: { + '../../lib/commands/version.js': class Version { + async exec () {} + }, + }, + globals: { + 'process.argv': [ + 'node', + 'npm', + 'version', + '--registry=https://u:password@npmjs.org', + ], + }, }) + await cli(process) + + t.strictSame(logsBy('argv'), [['"version" "--registry" "https://u:***@npmjs.org"']]) +}) - const { cli, outputs, exitHandlerCalled, exitHandlerNpm } = await cliMock(t) +t.test('print usage if no params provided', async t => { + const { cli, outputs, exitHandlerCalled, exitHandlerNpm } = await cliMock(t, { + globals: { + 'process.argv': ['node', 'npm'], + }, + }) await cli(process) + t.match(outputs[0][0], 'Usage:', 'outputs npm usage') t.match(exitHandlerCalled(), [], 'should call exitHandler with no args') t.ok(exitHandlerNpm(), 'exitHandler npm is set') @@ -128,12 +139,13 @@ t.test('print usage if no params provided', async t => { }) t.test('print usage if non-command param provided', async t => { - mockGlobals(t, { - 'process.argv': ['node', 'npm', 'tset'], + const { cli, outputs, exitHandlerCalled, exitHandlerNpm } = await cliMock(t, { + globals: { + 'process.argv': ['node', 'npm', 'tset'], + }, }) - - const { cli, outputs, exitHandlerCalled, exitHandlerNpm } = await cliMock(t) await cli(process) + t.match(outputs[0][0], 'Unknown command: "tset"') t.match(outputs[0][0], 'Did you mean this?') t.match(exitHandlerCalled(), [], 'should call exitHandler with no args') @@ -142,21 +154,22 @@ t.test('print usage if non-command param provided', async t => { }) t.test('load error calls error handler', async t => { - mockGlobals(t, { - 'process.argv': ['node', 'npm', 'asdf'], - }) - const err = new Error('test load error') const { cli, exitHandlerCalled } = await cliMock(t, { - '../../lib/utils/config/index.js': { - definitions: null, - flatten: null, - shorthands: null, + mocks: { + '../../lib/utils/config/index.js': { + definitions: null, + flatten: null, + shorthands: null, + }, + '@npmcli/config': class BadConfig { + async load () { + throw err + } + }, }, - '@npmcli/config': class BadConfig { - async load () { - throw err - } + globals: { + 'process.argv': ['node', 'npm', 'asdf'], }, }) await cli(process) diff --git a/test/lib/commands/bin.js b/test/lib/commands/bin.js index 4de5a923b..a889b1336 100644 --- a/test/lib/commands/bin.js +++ b/test/lib/commands/bin.js @@ -1,76 +1,60 @@ const t = require('tap') -const { fake: mockNpm } = require('../../fixtures/mock-npm') +const { relative, join } = require('path') +const { load: loadMockNpm } = require('../../fixtures/mock-npm') +const mockGlobals = require('../../fixtures/mock-globals') -t.test('bin', async t => { - t.plan(2) - const dir = '/bin/dir' - - const Bin = require('../../../lib/commands/bin.js') +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, + } +} - const npm = mockNpm({ - bin: dir, +t.test('bin', async t => { + const { cmd, bin, prefix, outputErrors } = await mockBin(t, { config: { global: false }, - output: (output) => { - t.equal(output, dir, 'prints the correct directory') - }, }) - const bin = new Bin(npm) - t.match(bin.usage, 'bin', 'usage has command name in it') - await bin.exec([]) + 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 -g', async t => { - t.plan(1) - const consoleError = console.error - t.teardown(() => { - console.error = consoleError + mockGlobals(t, { 'process.platform': 'posix' }) + const { globalPrefix, bin, outputErrors } = await mockBin(t, { + config: { global: true }, }) - console.error = (output) => { - t.fail('should not have printed to console.error') - } - const dir = '/bin/dir' - - const Bin = t.mock('../../../lib/commands/bin.js', { - '../../../lib/utils/path.js': [dir], - }) + t.equal(relative(globalPrefix, bin), 'bin', 'prints the correct directory') + t.strictSame(outputErrors, []) +}) - const npm = mockNpm({ - bin: dir, +t.test('bin -g win32', async t => { + mockGlobals(t, { 'process.platform': 'win32' }) + const { globalPrefix, bin, outputErrors } = await mockBin(t, { config: { global: true }, - output: (output) => { - t.equal(output, dir, 'prints the correct directory') - }, }) - const bin = new Bin(npm) - await bin.exec([]) + t.equal(relative(globalPrefix, bin), '', 'prints the correct directory') + t.strictSame(outputErrors, []) }) t.test('bin -g (not in path)', async t => { - t.plan(2) - const consoleError = console.error - t.teardown(() => { - console.error = consoleError - }) - - console.error = (output) => { - t.equal(output, '(not in PATH env variable)', 'prints env warning') - } - const dir = '/bin/dir' - - const Bin = t.mock('../../../lib/commands/bin.js', { - '../../../lib/utils/path.js': ['/not/my/dir'], - }) - const npm = mockNpm({ - bin: dir, + const { logs } = await mockBin(t, { config: { global: true }, - output: (output) => { - t.equal(output, dir, 'prints the correct directory') + globals: { + 'process.env.PATH': 'emptypath', }, }) - const bin = new Bin(npm) - await bin.exec([]) + t.strictSame(logs.error[0], ['bin', '(not in PATH env variable)']) }) diff --git a/test/lib/commands/doctor.js b/test/lib/commands/doctor.js index 5badab99a..620d908d3 100644 --- a/test/lib/commands/doctor.js +++ b/test/lib/commands/doctor.js @@ -52,17 +52,7 @@ const dirs = { }, } -let consoleError = false -t.afterEach(() => { - consoleError = false -}) - const globals = { - console: { - error: () => { - consoleError = true - }, - }, process: { platform: 'test-not-windows', version: 'v1.0.0', @@ -104,7 +94,6 @@ t.test('all clear', async t => { .get('/dist/index.json').reply(200, nodeVersions) await npm.exec('doctor', []) t.matchSnapshot(joinedOutput(), 'output') - t.notOk(consoleError, 'console.error not called') t.matchSnapshot({ info: logs.info, warn: logs.warn, error: logs.error }, 'logs') }) @@ -122,7 +111,6 @@ t.test('all clear in color', async t => { npm.config.set('color', 'always') await npm.exec('doctor', []) t.matchSnapshot(joinedOutput(), 'everything is ok in color') - t.notOk(consoleError, 'console.error not called') t.matchSnapshot({ info: logs.info, warn: logs.warn, error: logs.error }, 'logs') }) @@ -142,7 +130,6 @@ t.test('silent', async t => { .get('/dist/index.json').reply(200, nodeVersions) await npm.exec('doctor', []) t.matchSnapshot(joinedOutput(), 'output') - t.notOk(consoleError, 'console.error not called') t.matchSnapshot({ info: logs.info, warn: logs.warn, error: logs.error }, 'logs') }) @@ -159,7 +146,6 @@ t.test('ping 404', async t => { .get('/dist/index.json').reply(200, nodeVersions) await t.rejects(npm.exec('doctor', [])) t.matchSnapshot(joinedOutput(), 'ping 404') - t.ok(consoleError, 'console.error called') t.matchSnapshot({ info: logs.info, warn: logs.warn, error: logs.error }, 'logs') }) diff --git a/test/lib/npm.js b/test/lib/npm.js index 3ae2af35c..4302437a6 100644 --- a/test/lib/npm.js +++ b/test/lib/npm.js @@ -139,10 +139,11 @@ t.test('npm.load', async t => { }) t.test('forceful loading', async t => { - mockGlobals(t, { - 'process.argv': [...process.argv, '--force', '--color', 'always'], + const { logs } = await loadMockNpm(t, { + globals: { + 'process.argv': [...process.argv, '--force', '--color', 'always'], + }, }) - const { logs } = await loadMockNpm(t) t.match(logs.warn, [ [ 'using --force', @@ -153,23 +154,21 @@ t.test('npm.load', async t => { t.test('node is a symlink', async t => { const node = process.platform === 'win32' ? 'node.exe' : 'node' - mockGlobals(t, { - 'process.argv': [ - node, - process.argv[1], - '--usage', - '--scope=foo', - 'token', - 'revoke', - 'blergggg', - ], - }) const { npm, logs, outputs, prefix } = await loadMockNpm(t, { prefixDir: { bin: t.fixture('symlink', dirname(process.execPath)), }, globals: ({ prefix }) => ({ 'process.env.PATH': resolve(prefix, 'bin'), + 'process.argv': [ + node, + process.argv[1], + '--usage', + '--scope=foo', + 'token', + 'revoke', + 'blergggg', + ], }), }) @@ -181,6 +180,9 @@ t.test('npm.load', async t => { ], [ ['npm:load:whichnode', /Completed in [0-9.]+ms/], ['node symlink', resolve(prefix, 'bin', node)], + ['title', 'npm token revoke blergggg'], + ['argv', '"--usage" "--scope" "foo" "token" "revoke" "blergggg"'], + ['logfile', /logs-max:\d+ dir:.*/], ['logfile', /.*-debug-0.log/], ['npm:load', /Completed in [0-9.]+ms/], ]) @@ -226,15 +228,6 @@ t.test('npm.load', async t => { }) t.test('--no-workspaces with --workspace', async t => { - mockGlobals(t, { - 'process.argv': [ - process.execPath, - process.argv[1], - '--color', 'false', - '--workspaces', 'false', - '--workspace', 'a', - ], - }) const { npm } = await loadMockNpm(t, { load: false, prefixDir: { @@ -253,6 +246,15 @@ t.test('npm.load', async t => { workspaces: ['./packages/*'], }), }, + globals: { + 'process.argv': [ + process.execPath, + process.argv[1], + '--color', 'false', + '--workspaces', 'false', + '--workspace', 'a', + ], + }, }) await t.rejects( npm.exec('run', []), @@ -261,14 +263,6 @@ t.test('npm.load', async t => { }) t.test('workspace-aware configs and commands', async t => { - mockGlobals(t, { - 'process.argv': [ - process.execPath, - process.argv[1], - '--color', 'false', - '--workspaces', 'true', - ], - }) const { npm, outputs } = await loadMockNpm(t, { prefixDir: { packages: { @@ -293,6 +287,14 @@ t.test('npm.load', async t => { workspaces: ['./packages/*'], }), }, + globals: { + 'process.argv': [ + process.execPath, + process.argv[1], + '--color', 'false', + '--workspaces', 'true', + ], + }, }) // verify that calling the command with a short name still sets @@ -317,17 +319,6 @@ t.test('npm.load', async t => { }) t.test('workspaces in global mode', async t => { - mockGlobals(t, { - 'process.argv': [ - process.execPath, - process.argv[1], - '--color', - 'false', - '--workspaces', - '--global', - 'true', - ], - }) const { npm } = await loadMockNpm(t, { prefixDir: { packages: { @@ -352,6 +343,17 @@ t.test('npm.load', async t => { workspaces: ['./packages/*'], }), }, + globals: { + 'process.argv': [ + process.execPath, + process.argv[1], + '--color', + 'false', + '--workspaces', + '--global', + 'true', + ], + }, }) // verify that calling the command with a short name still sets // the npm.command property to the full canonical name of the cmd. @@ -365,68 +367,93 @@ t.test('npm.load', async t => { t.test('set process.title', async t => { t.test('basic title setting', async t => { - mockGlobals(t, { - 'process.argv': [ - process.execPath, - process.argv[1], - '--usage', - '--scope=foo', - 'ls', - ], + const { npm } = await loadMockNpm(t, { + globals: { + 'process.argv': [ + process.execPath, + process.argv[1], + '--usage', + '--scope=foo', + 'ls', + ], + }, }) - const { npm } = await loadMockNpm(t) t.equal(npm.title, 'npm ls') t.equal(process.title, 'npm ls') }) t.test('do not expose token being revoked', async t => { - mockGlobals(t, { - 'process.argv': [ - process.execPath, - process.argv[1], - '--usage', - '--scope=foo', - 'token', - 'revoke', - 'deadbeefcafebad', - ], + const { npm } = await loadMockNpm(t, { + globals: { + 'process.argv': [ + process.execPath, + process.argv[1], + '--usage', + '--scope=foo', + 'token', + 'revoke', + `npm_${'a'.repeat(36)}`, + ], + }, }) - const { npm } = await loadMockNpm(t) - t.equal(npm.title, 'npm token revoke ***') - t.equal(process.title, 'npm token revoke ***') + t.equal(npm.title, 'npm token revoke npm_***') + t.equal(process.title, 'npm token revoke npm_***') }) t.test('do show *** unless a token is actually being revoked', async t => { - mockGlobals(t, { - 'process.argv': [ - process.execPath, - process.argv[1], - '--usage', - '--scope=foo', - 'token', - 'revoke', - ], + const { npm } = await loadMockNpm(t, { + globals: { + 'process.argv': [ + process.execPath, + process.argv[1], + '--usage', + '--scope=foo', + 'token', + 'revoke', + 'notatoken', + ], + }, }) - const { npm } = await loadMockNpm(t) - t.equal(npm.title, 'npm token revoke') - t.equal(process.title, 'npm token revoke') + t.equal(npm.title, 'npm token revoke notatoken') + t.equal(process.title, 'npm token revoke notatoken') }) }) -t.test('debug-log', async t => { - const { npm, debugFile } = await loadMockNpm(t, { load: false }) +t.test('debug log', async t => { + t.test('writes log file', async t => { + const { npm, debugFile } = await loadMockNpm(t, { load: false }) + + const log1 = ['silly', 'test', 'before load'] + const log2 = ['silly', 'test', 'after load'] - const log1 = ['silly', 'test', 'before load'] - const log2 = ['silly', 'test', 'after load'] + process.emit('log', ...log1) + await npm.load() + process.emit('log', ...log2) - process.emit('log', ...log1) - await npm.load() - process.emit('log', ...log2) + const debug = await debugFile() + t.equal(npm.logFiles.length, 1, 'one debug file') + t.match(debug, log1.join(' '), 'before load appears') + t.match(debug, log2.join(' '), 'after load log appears') + }) + + t.test('with bad dir', async t => { + const { npm } = await loadMockNpm(t, { + config: { + 'logs-dir': 'LOGS_DIR', + }, + mocks: { + '@npmcli/fs': { + mkdir: async (dir) => { + if (dir.includes('LOGS_DIR')) { + throw new Error('err') + } + }, + }, + }, + }) - const debug = await debugFile() - t.equal(npm.logFiles.length, 1, 'one debug file') - t.match(debug, log1.join(' '), 'before load appears') - t.match(debug, log2.join(' '), 'after load log appears') + t.equal(npm.logFiles.length, 0, 'no log file') + }) }) t.test('timings', async t => { @@ -458,13 +485,14 @@ t.test('timings', async t => { }) t.test('writes timings file', async t => { - const { npm, timingFile } = await loadMockNpm(t, { + const { npm, cache, timingFile } = await loadMockNpm(t, { config: { timing: true }, }) process.emit('time', 'foo') process.emit('timeEnd', 'foo') process.emit('time', 'bar') - npm.unload() + npm.writeTimingFile() + t.equal(npm.timingFile, join(cache, '_timing.json')) const timings = await timingFile() t.match(timings, { command: [], @@ -484,21 +512,16 @@ t.test('timings', async t => { const { npm, timingFile } = await loadMockNpm(t, { config: { false: true }, }) - npm.unload() + npm.writeTimingFile() await t.rejects(() => timingFile()) }) }) t.test('output clears progress and console.logs the message', async t => { - t.plan(2) + t.plan(4) let showingProgress = true const logs = [] - mockGlobals(t, { - 'console.log': (...args) => { - t.equal(showingProgress, false, 'should not be showing progress right now') - logs.push(args) - }, - }) + const errors = [] const { npm } = await loadMockNpm(t, { load: false, mocks: { @@ -507,9 +530,22 @@ t.test('output clears progress and console.logs the message', async t => { showProgress: () => showingProgress = true, }, }, + globals: { + 'console.log': (...args) => { + t.equal(showingProgress, false, 'should not be showing progress right now') + logs.push(args) + }, + 'console.error': (...args) => { + t.equal(showingProgress, false, 'should not be showing progress right now') + errors.push(args) + }, + }, }) npm.originalOutput('hello') + npm.originalOutputError('error') + t.match(logs, [['hello']]) + t.match(errors, [['error']]) t.end() }) @@ -522,14 +558,6 @@ t.test('unknown command', async t => { }) t.test('explicit workspace rejection', async t => { - mockGlobals(t, { - 'process.argv': [ - process.execPath, - process.argv[1], - '--color', 'false', - '--workspace', './packages/a', - ], - }) const mock = await loadMockNpm(t, { prefixDir: { packages: { @@ -547,6 +575,14 @@ t.test('explicit workspace rejection', async t => { workspaces: ['./packages/a'], }), }, + globals: { + 'process.argv': [ + process.execPath, + process.argv[1], + '--color', 'false', + '--workspace', './packages/a', + ], + }, }) await t.rejects( mock.npm.exec('ping', []), @@ -572,15 +608,17 @@ t.test('implicit workspace rejection', async t => { workspaces: ['./packages/a'], }), }, - }) - const cwd = join(mock.npm.config.localPrefix, 'packages', 'a') - mock.npm.config.set('workspace', [cwd], 'default') - mockGlobals(t, { - 'process.argv': [ - process.execPath, - process.argv[1], - '--color', 'false', - ], + globals: { + 'process.argv': [ + process.execPath, + process.argv[1], + '--color', 'false', + '--workspace', './packages/a', + ], + }, + config: ({ prefix }) => ({ + workspace: { value: [join(prefix, 'packages', 'a')], where: 'default' }, + }), }) await t.rejects( mock.npm.exec('owner', []), @@ -606,19 +644,17 @@ t.test('implicit workspace accept', async t => { workspaces: ['./packages/a'], }), }, + globals: ({ prefix }) => ({ + 'process.cwd': () => prefix, + 'process.argv': [ + process.execPath, + process.argv[1], + '--color', 'false', + ], + }), + config: ({ prefix }) => ({ + workspace: { value: [join(prefix, 'packages', 'a')], where: 'default' }, + }), }) - const cwd = join(mock.npm.config.localPrefix, 'packages', 'a') - mock.npm.config.set('workspace', [cwd], 'default') - mockGlobals(t, { - 'process.cwd': () => mock.npm.config.cwd, - 'process.argv': [ - process.execPath, - process.argv[1], - '--color', 'false', - ], - }) - await t.rejects( - mock.npm.exec('org', []), - /.*Usage/ - ) + await t.rejects(mock.npm.exec('org', []), /.*Usage/) }) diff --git a/test/lib/utils/exit-handler.js b/test/lib/utils/exit-handler.js index 6a96d92dd..73bbf06fe 100644 --- a/test/lib/utils/exit-handler.js +++ b/test/lib/utils/exit-handler.js @@ -21,9 +21,10 @@ t.formatSnapshot = (obj) => { } t.cleanSnapshot = (path) => cleanDate(cleanCwd(path)) -// Config loading is dependent on env so strip those from snapshots + // Config loading is dependent on env so strip those from snapshots .replace(/.*timing config:load:.*\n/gm, '') .replace(/(Completed in )\d+(ms)/g, '$1{TIME}$2') + .replace(/(removing )\d+( files)/g, '$1${NUM}2') // cut off process from script so that it won't quit the test runner // while trying to run through the myriad of cases. need to make it @@ -44,9 +45,8 @@ mockGlobals(t, { }), }, { replace: true }) -const mockExitHandler = async (t, { init, load, testdir, config } = {}) => { +const mockExitHandler = async (t, { init, load, testdir, config, globals, mocks } = {}) => { const errors = [] - mockGlobals(t, { 'console.error': (err) => errors.push(err) }) const { npm, logMocks, ...rest } = await loadMockNpm(t, { init, @@ -56,11 +56,15 @@ const mockExitHandler = async (t, { init, load, testdir, config } = {}) => { '../../package.json': { version: '1.0.0', }, + ...mocks, }, config: { loglevel: 'notice', ...config, }, + globals: { + 'console.error': (err) => errors.push(err), + }, }) const exitHandler = t.mock('../../../lib/utils/exit-handler.js', { @@ -74,6 +78,7 @@ const mockExitHandler = async (t, { init, load, testdir, config } = {}) => { release: () => '1.0.0', }, ...logMocks, + ...mocks, }) if (npm) { @@ -89,13 +94,14 @@ const mockExitHandler = async (t, { init, load, testdir, config } = {}) => { ...rest, errors, npm, - // Make it async to make testing ergonomics a little - // easier so we dont need to t.plan() every test to - // make sure we get process.exit called - exitHandler: (...args) => new Promise(resolve => { + // Make it async to make testing ergonomics a little easier so we dont need + // to t.plan() every test to make sure we get process.exit called. Also + // introduce a small artificial delay so the logs are consistently finished + // by the time the exit handler forces process.exit + exitHandler: (...args) => new Promise(resolve => setTimeout(() => { process.once('exit', resolve) exitHandler(...args) - }), + }, 50)), } } @@ -199,17 +205,15 @@ t.test('exit handler called - no npm with error without stack', async (t) => { }) t.test('console.log output using --json', async (t) => { - const { exitHandler, errors } = await mockExitHandler(t, { - config: { - json: true, - }, + const { exitHandler, outputErrors } = await mockExitHandler(t, { + config: { json: true }, }) await exitHandler(err('Error: EBADTHING Something happened')) t.equal(process.exitCode, 1) t.same( - JSON.parse(errors[0]), + JSON.parse(outputErrors[0]), { error: { code: 'EBADTHING', // should default error code to E[A-Z]+ @@ -273,11 +277,43 @@ t.test('npm.config not ready', async (t) => { ], 'should exit with config error msg') }) -t.test('timing with no error', async (t) => { - const { exitHandler, timingFile, npm, logs } = await mockExitHandler(t, { +t.test('no logs dir', async (t) => { + const { exitHandler, logs } = await mockExitHandler(t, { + config: { 'logs-max': 0 }, + }) + + await exitHandler(new Error()) + + t.match(logs.error.filter(([t]) => t === ''), [ + ['', 'Log files were not written due to the config logs-max=0'], + ]) +}) + +t.test('log file error', async (t) => { + const { exitHandler, logs } = await mockExitHandler(t, { config: { + 'logs-dir': 'LOGS_DIR', timing: true, }, + mocks: { + '@npmcli/fs': { + mkdir: async (dir) => { + if (dir.includes('LOGS_DIR')) { + throw new Error('err') + } + }, + }, + }, + }) + + await exitHandler(new Error()) + + t.match(logs.error.filter(([t]) => t === ''), [['', `error writing to the directory`]]) +}) + +t.test('timing with no error', async (t) => { + const { exitHandler, timingFile, npm, logs } = await mockExitHandler(t, { + config: { timing: true }, }) await exitHandler() @@ -285,9 +321,9 @@ t.test('timing with no error', async (t) => { t.equal(process.exitCode, 0) - t.match(logs.error, [ - ['', /A complete log of this run can be found in:[\s\S]*-debug-\d\.log/], - ]) + const msg = logs.info.filter(([t]) => t === '')[0][1] + t.match(msg, /A complete log of this run can be found in:/) + t.match(msg, /Timing info written to:/) t.match( timingFileData, @@ -308,9 +344,7 @@ t.test('timing with no error', async (t) => { t.test('unfinished timers', async (t) => { const { exitHandler, timingFile, npm } = await mockExitHandler(t, { - config: { - timing: true, - }, + config: { timing: true }, }) process.emit('time', 'foo') @@ -376,7 +410,7 @@ t.test('verbose logs replace info on err props', async t => { await exitHandler(err('Error with code type number', properties)) t.equal(process.exitCode, 1) t.match( - logs.verbose.filter(([p]) => p !== 'logfile'), + logs.verbose.filter(([p]) => !['logfile', 'title', 'argv'].includes(p)), keys.map((k) => [k, `${k}-https://user:***@registry.npmjs.org/`]), 'all special keys get replaced' ) diff --git a/test/lib/utils/log-file.js b/test/lib/utils/log-file.js index 007ce221b..ce6f0bf4c 100644 --- a/test/lib/utils/log-file.js +++ b/test/lib/utils/log-file.js @@ -116,12 +116,12 @@ t.test('max files per process', async t => { } for (const i of range(5)) { - logFile.log('verbose', `log ${i}`) + logFile.log('verbose', `ignored after maxlogs hit ${i}`) } const logs = await readLogs() t.equal(logs.length, maxFilesPerProcess, 'total log files') - t.equal(last(last(logs).logs), '49 error log 49') + t.match(last(last(logs).logs), /49 error log \d+/) }) t.test('stream error', async t => { @@ -182,8 +182,7 @@ t.test('turns off', async t => { logFile.load() const logs = await readLogs() - t.equal(logs.length, 1) - t.equal(logs[0].logs[0], '0 error test') + t.match(last(last(logs).logs), /^\d+ error test$/) }) t.test('cleans logs', async t => { @@ -198,7 +197,7 @@ t.test('cleans logs', async t => { }) t.test('doesnt clean current log by default', async t => { - const logsMax = 0 + const logsMax = 1 const { readLogs, logFile } = await loadLogFile(t, { logsMax, testdir: makeOldLogs(10), @@ -207,7 +206,6 @@ t.test('doesnt clean current log by default', async t => { logFile.log('error', 'test') const logs = await readLogs() - t.equal(logs.length, 1) t.match(last(logs).content, /\d+ error test/) }) @@ -221,8 +219,7 @@ t.test('negative logs max', async t => { logFile.log('error', 'test') const logs = await readLogs() - t.equal(logs.length, 1) - t.match(last(logs).content, /\d+ error test/) + t.equal(logs.length, 0) }) t.test('doesnt need to clean', async t => { @@ -257,7 +254,7 @@ t.test('cleans old style logs too', async t => { const oldLogs = 10 const { readLogs } = await loadLogFile(t, { logsMax, - testdir: makeOldLogs(oldLogs, false), + testdir: makeOldLogs(oldLogs, true), }) const logs = await readLogs() @@ -304,7 +301,7 @@ t.test('delete log file while open', async t => { }) t.test('snapshot', async t => { - const { logFile, readLogs } = await loadLogFile(t) + const { logFile, readLogs } = await loadLogFile(t, { logsMax: 10 }) logFile.log('error', '', 'no prefix') logFile.log('error', 'prefix', 'with prefix') diff --git a/test/lib/utils/replace-info.js b/test/lib/utils/replace-info.js index e4b83783a..c7fffdb54 100644 --- a/test/lib/utils/replace-info.js +++ b/test/lib/utils/replace-info.js @@ -20,12 +20,30 @@ t.equal( ) t.equal( + replaceInfo(' == = = '), + ' == = = ', + 'should return same string with only separators' +) + +t.equal( + replaceInfo(''), + '', + 'should return empty string' +) + +t.equal( replaceInfo('https://user:pass@registry.npmjs.org/'), 'https://user:***@registry.npmjs.org/', 'should replace single item' ) t.equal( + replaceInfo(`https://registry.npmjs.org/path/npm_${'a'.repeat('36')}`), + 'https://registry.npmjs.org/path/npm_***', + 'should replace single item token' +) + +t.equal( replaceInfo('https://example.npmjs.org'), 'https://example.npmjs.org', 'should not replace single item with no password' @@ -49,6 +67,12 @@ t.equal( 'should replace single item within a phrase' ) +t.equal( + replaceInfo('Something --x=https://user:pass@registry.npmjs.org/ foo bar'), + 'Something --x=https://user:***@registry.npmjs.org/ foo bar', + 'should replace single item within a phrase separated by =' +) + t.same( replaceInfo([ 'Something https://user:pass@registry.npmjs.org/ foo bar', @@ -60,7 +84,21 @@ t.same( 'http://foo:***@registry.npmjs.org', 'http://example.npmjs.org', ], - 'should replace single item within a phrase' + 'should replace items in an array' +) + +t.same( + replaceInfo([ + 'Something --x=https://user:pass@registry.npmjs.org/ foo bar', + '--url=http://foo:bar@registry.npmjs.org', + '--url=http://example.npmjs.org', + ]), + [ + 'Something --x=https://user:***@registry.npmjs.org/ foo bar', + '--url=http://foo:***@registry.npmjs.org', + '--url=http://example.npmjs.org', + ], + 'should replace items in an array with equals' ) t.same( diff --git a/test/lib/utils/timers.js b/test/lib/utils/timers.js index 6127f346b..30e54700c 100644 --- a/test/lib/utils/timers.js +++ b/test/lib/utils/timers.js @@ -1,5 +1,5 @@ const t = require('tap') -const { resolve } = require('path') +const { resolve, join } = require('path') const fs = require('graceful-fs') const mockLogs = require('../../fixtures/mock-logs') @@ -31,6 +31,17 @@ t.test('listens/stops on process', async (t) => { t.notOk(timers.unfinished.get('baz')) }) +t.test('convenience time method', async (t) => { + const { timers } = mockTimers(t) + + const end = timers.time('later') + timers.time('sync', () => {}) + await timers.time('async', () => new Promise(r => setTimeout(r, 10))) + end() + + t.match(timers.finished, { later: Number, sync: Number, async: Number }) +}) + t.test('initial timer', async (t) => { const { timers } = mockTimers(t, { start: 'foo' }) process.emit('timeEnd', 'foo') @@ -75,8 +86,21 @@ t.test('writes file', async (t) => { t.test('fails to write file', async (t) => { const { logs, timers } = mockTimers(t) + const dir = t.testdir() + + timers.load({ dir: join(dir, 'does', 'not', 'exist') }) + timers.writeFile() + + t.match(logs.warn, [['timing', 'could not write timing file']]) + t.equal(timers.file, null) +}) + +t.test('no dir and no file', async (t) => { + const { logs, timers } = mockTimers(t) + + timers.load() timers.writeFile() - t.match(logs.warn, [ - ['timing', 'could not write timing file', Error], - ]) + + t.strictSame(logs, []) + t.equal(timers.file, null) }) diff --git a/test/lib/utils/update-notifier.js b/test/lib/utils/update-notifier.js index fa4af2947..a35886c6e 100644 --- a/test/lib/utils/update-notifier.js +++ b/test/lib/utils/update-notifier.js @@ -85,12 +85,11 @@ t.afterEach(() => { const runUpdateNotifier = async ({ color = true, ...npmOptions } = {}) => { const _npm = { ...defaultNpm, ...npmOptions, logColor: color } - await t.mock('../../../lib/utils/update-notifier.js', { + return t.mock('../../../lib/utils/update-notifier.js', { '@npmcli/ci-detect': () => ciMock, pacote, fs, })(_npm) - return _npm.updateNotification } t.test('situations in which we do not notify', t => { |