diff --git a/lib/cli.js b/lib/cli.js index d4a67645858ae..fbceb459db97c 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -20,6 +20,7 @@ module.exports = (process) => { const npm = require('../lib/npm.js') const errorHandler = require('../lib/utils/error-handler.js') + errorHandler.setNpm(npm) // if npm is called as "npmg" or "npm_g", then // run in global mode. diff --git a/lib/utils/cache-file.js b/lib/utils/cache-file.js deleted file mode 100644 index b33881e872ec2..0000000000000 --- a/lib/utils/cache-file.js +++ /dev/null @@ -1,66 +0,0 @@ -const npm = require('../npm.js') -const path = require('path') -const chownr = require('chownr') -const writeFileAtomic = require('write-file-atomic') -const mkdirp = require('mkdirp-infer-owner') -const fs = require('graceful-fs') - -let cache = null -let cacheUid = null -let cacheGid = null -let needChown = typeof process.getuid === 'function' - -const getCacheOwner = () => { - let st - try { - st = fs.lstatSync(cache) - } catch (er) { - if (er.code !== 'ENOENT') - throw er - - st = fs.lstatSync(path.dirname(cache)) - } - - cacheUid = st.uid - cacheGid = st.gid - - needChown = st.uid !== process.getuid() || - st.gid !== process.getgid() -} - -const writeOrAppend = (method, file, data) => { - if (!cache) - cache = npm.config.get('cache') - - // redundant if already absolute, but prevents non-absolute files - // from being written as if they're part of the cache. - file = path.resolve(cache, file) - - if (cacheUid === null && needChown) - getCacheOwner() - - const dir = path.dirname(file) - const firstMade = mkdirp.sync(dir) - - if (!needChown) - return method(file, data) - - let methodThrew = true - try { - method(file, data) - methodThrew = false - } finally { - // always try to leave it in the right ownership state, even on failure - // let the method error fail it instead of the chownr error, though - if (!methodThrew) - chownr.sync(firstMade || file, cacheUid, cacheGid) - else { - try { - chownr.sync(firstMade || file, cacheUid, cacheGid) - } catch (_) {} - } - } -} - -exports.append = (file, data) => writeOrAppend(fs.appendFileSync, file, data) -exports.write = (file, data) => writeOrAppend(writeFileAtomic.sync, file, data) diff --git a/lib/utils/error-handler.js b/lib/utils/error-handler.js index da716679d2705..f40e1f04fb180 100644 --- a/lib/utils/error-handler.js +++ b/lib/utils/error-handler.js @@ -1,28 +1,27 @@ +let npm // set by the cli let cbCalled = false const log = require('npmlog') -const npm = require('../npm.js') let itWorked = false const path = require('path') +const writeFileAtomic = require('write-file-atomic') +const mkdirp = require('mkdirp-infer-owner') +const fs = require('graceful-fs') let wroteLogFile = false let exitCode = 0 const errorMessage = require('./error-message.js') const replaceInfo = require('./replace-info.js') -const cacheFile = require('./cache-file.js') - let logFileName const getLogFile = () => { + // we call this multiple times, so we need to treat it as a singleton because + // the date is part of the name if (!logFileName) logFileName = path.resolve(npm.config.get('cache'), '_logs', (new Date()).toISOString().replace(/[.:]/g, '_') + '-debug.log') return logFileName } -const timings = { - version: npm.version, - command: process.argv.slice(2), - logfile: null, -} +const timings = {} process.on('timing', (name, value) => { if (timings[name]) timings[name] += value @@ -35,9 +34,21 @@ process.on('exit', code => { log.disableProgress() if (npm.config && npm.config.loaded && npm.config.get('timing')) { try { - timings.logfile = getLogFile() - cacheFile.append('_timing.json', JSON.stringify(timings) + '\n') - } catch (_) { + const file = path.resolve(npm.config.get('cache'), '_timing.json') + const dir = path.dirname(npm.config.get('cache')) + mkdirp.sync(dir) + + fs.appendFileSync(file, JSON.stringify({ + command: process.argv.slice(2), + logfile: getLogFile(), + version: npm.version, + ...timings, + }) + '\n') + + const st = fs.lstatSync(path.dirname(npm.config.get('cache'))) + fs.chownSync(dir, st.uid, st.gid) + fs.chownSync(file, st.uid, st.gid) + } catch (ex) { // ignore } } @@ -174,7 +185,7 @@ const errorHandler = (er) => { log.error(k, v) } - const msg = errorMessage(er) + const msg = errorMessage(er, npm) for (const errline of [...msg.summary, ...msg.detail]) log.error(...errline) @@ -214,7 +225,15 @@ const writeLogFile = () => { logOutput += line + os.EOL }) }) - cacheFile.write(getLogFile(), logOutput) + + const file = getLogFile() + const dir = path.dirname(file) + mkdirp.sync(dir) + writeFileAtomic.sync(file, logOutput) + + const st = fs.lstatSync(path.dirname(npm.config.get('cache'))) + fs.chownSync(dir, st.uid, st.gid) + fs.chownSync(file, st.uid, st.gid) // truncate once it's been written. log.record.length = 0 @@ -226,3 +245,6 @@ const writeLogFile = () => { module.exports = errorHandler module.exports.exit = exit +module.exports.setNpm = (n) => { + npm = n +} diff --git a/lib/utils/error-message.js b/lib/utils/error-message.js index ac5a935dc8770..125cdf8c53581 100644 --- a/lib/utils/error-message.js +++ b/lib/utils/error-message.js @@ -1,12 +1,11 @@ -const npm = require('../npm.js') const { format } = require('util') const { resolve } = require('path') const nameValidator = require('validate-npm-package-name') const npmlog = require('npmlog') const replaceInfo = require('./replace-info.js') -const { report: explainEresolve } = require('./explain-eresolve.js') +const { report } = require('./explain-eresolve.js') -module.exports = (er) => { +module.exports = (er, npm) => { const short = [] const detail = [] @@ -19,7 +18,7 @@ module.exports = (er) => { case 'ERESOLVE': short.push(['ERESOLVE', er.message]) detail.push(['', '']) - detail.push(['', explainEresolve(er)]) + detail.push(['', report(er, npm.color, resolve(npm.cache, 'eresolve-report.txt'))]) break case 'ENOLOCK': { diff --git a/lib/utils/explain-eresolve.js b/lib/utils/explain-eresolve.js index cda77aff94113..b35a32c6f935d 100644 --- a/lib/utils/explain-eresolve.js +++ b/lib/utils/explain-eresolve.js @@ -1,20 +1,14 @@ // this is called when an ERESOLVE error is caught in the error-handler, // or when there's a log.warn('eresolve', msg, explanation), to turn it // into a human-intelligible explanation of what's wrong and how to fix. -// -// TODO: abstract out the explainNode methods into a separate util for -// use by a future `npm explain ` command. - -const npm = require('../npm.js') const { writeFileSync } = require('fs') -const { resolve } = require('path') const { explainEdge, explainNode, printNode } = require('./explain-dep.js') // expl is an explanation object that comes from Arborist. It looks like: // Depth is how far we want to want to descend into the object making a report. // The full report (ie, depth=Infinity) is always written to the cache folder // at ${cache}/eresolve-report.txt along with full json. -const explainEresolve = (expl, color, depth) => { +const explain = (expl, color, depth) => { const { edge, current, peerConflict, currentEdge } = expl const out = [] @@ -42,9 +36,7 @@ const explainEresolve = (expl, color, depth) => { } // generate a full verbose report and tell the user how to fix it -const report = (expl, depth = 4) => { - const fullReport = resolve(npm.cache, 'eresolve-report.txt') - +const report = (expl, color, fullReport) => { const orNoStrict = expl.strictPeerDeps ? '--no-strict-peer-deps, ' : '' const fix = `Fix the upstream dependency conflict, or retry this command with ${orNoStrict}--force, or --legacy-peer-deps @@ -54,7 +46,7 @@ to accept an incorrect (and potentially broken) dependency resolution.` ${new Date().toISOString()} -${explainEresolve(expl, false, Infinity)} +${explain(expl, false, Infinity)} ${fix} @@ -63,13 +55,10 @@ Raw JSON explanation object: ${JSON.stringify(expl, null, 2)} `, 'utf8') - return explainEresolve(expl, npm.color, depth) + + return explain(expl, color, 4) + `\n\n${fix}\n\nSee ${fullReport} for a full report.` } -// the terser explain method for the warning when using --force -const explain = (expl, depth = 2) => explainEresolve(expl, npm.color, depth) - module.exports = { explain, report, diff --git a/lib/utils/setup-log.js b/lib/utils/setup-log.js index 44e612d50dc9f..9ee79d192d9ea 100644 --- a/lib/utils/setup-log.js +++ b/lib/utils/setup-log.js @@ -14,10 +14,22 @@ module.exports = (config) => { const { warn } = log + const stdoutTTY = process.stdout.isTTY + const stderrTTY = process.stderr.isTTY + const dumbTerm = process.env.TERM === 'dumb' + const stderrNotDumb = stderrTTY && !dumbTerm + const enableColorStderr = color === 'always' ? true + : color === false ? false + : stderrTTY + + const enableColorStdout = color === 'always' ? true + : color === false ? false + : stdoutTTY + log.warn = (heading, ...args) => { if (heading === 'ERESOLVE' && args[1] && typeof args[1] === 'object') { warn(heading, args[0]) - return warn('', explain(args[1])) + return warn('', explain(args[1], enableColorStdout, 2)) } return warn(heading, ...args) } @@ -29,19 +41,6 @@ module.exports = (config) => { log.heading = config.get('heading') || 'npm' - const stdoutTTY = process.stdout.isTTY - const stderrTTY = process.stderr.isTTY - const dumbTerm = process.env.TERM === 'dumb' - const stderrNotDumb = stderrTTY && !dumbTerm - - const enableColorStderr = color === 'always' ? true - : color === false ? false - : stderrTTY - - const enableColorStdout = color === 'always' ? true - : color === false ? false - : stdoutTTY - if (enableColorStderr) log.enableColor() else diff --git a/tap-snapshots/test/lib/utils/error-handler.js.test.cjs b/tap-snapshots/test/lib/utils/error-handler.js.test.cjs index 909051cdab506..78a9eef217f35 100644 --- a/tap-snapshots/test/lib/utils/error-handler.js.test.cjs +++ b/tap-snapshots/test/lib/utils/error-handler.js.test.cjs @@ -8,7 +8,7 @@ exports[`test/lib/utils/error-handler.js TAP handles unknown error > should have expected log contents for unknown error 1`] = ` 0 verbose code 1 1 error foo A complete log of this run can be found in: -1 error foo {CWD}/cachefolder/_logs/expecteddate-debug.log +1 error foo {CWD}/test/lib/utils/tap-testdir-error-handler/_logs/expecteddate-debug.log 2 verbose stack Error: ERROR 3 verbose cwd {CWD} 4 verbose Foo 1.0.0 diff --git a/tap-snapshots/test/lib/utils/explain-eresolve.js.test.cjs b/tap-snapshots/test/lib/utils/explain-eresolve.js.test.cjs index e066680153021..4c84aaca4ceeb 100644 --- a/tap-snapshots/test/lib/utils/explain-eresolve.js.test.cjs +++ b/tap-snapshots/test/lib/utils/explain-eresolve.js.test.cjs @@ -5,7 +5,7 @@ * Make sure to inspect the output below. Do not ignore changes! */ 'use strict' -exports[`test/lib/utils/explain-eresolve.js TAP chain-conflict > explain with color 1`] = ` +exports[`test/lib/utils/explain-eresolve.js TAP chain-conflict > explain with color, depth of 2 1`] = ` While resolving: project@1.2.3 Found: @isaacs/testing-peer-dep-conflict-chain-d@2.0.0 node_modules/@isaacs/testing-peer-dep-conflict-chain-d @@ -75,25 +75,7 @@ to accept an incorrect (and potentially broken) dependency resolution. See \${REPORT} for a full report. ` -exports[`test/lib/utils/explain-eresolve.js TAP chain-conflict > report with color, depth only 2 1`] = ` -While resolving: project@1.2.3 -Found: @isaacs/testing-peer-dep-conflict-chain-d@2.0.0 -node_modules/@isaacs/testing-peer-dep-conflict-chain-d - @isaacs/testing-peer-dep-conflict-chain-d@"2" from the root project - -Could not resolve dependency: -peer @isaacs/testing-peer-dep-conflict-chain-d@"1" from @isaacs/testing-peer-dep-conflict-chain-c@1.0.0 -node_modules/@isaacs/testing-peer-dep-conflict-chain-c - @isaacs/testing-peer-dep-conflict-chain-c@"1" from the root project - -Fix the upstream dependency conflict, or retry -this command with --force, or --legacy-peer-deps -to accept an incorrect (and potentially broken) dependency resolution. - -See \${REPORT} for a full report. -` - -exports[`test/lib/utils/explain-eresolve.js TAP chain-conflict > report with no color, depth of 6 1`] = ` +exports[`test/lib/utils/explain-eresolve.js TAP chain-conflict > report with no color 1`] = ` While resolving: project@1.2.3 Found: @isaacs/testing-peer-dep-conflict-chain-d@2.0.0 node_modules/@isaacs/testing-peer-dep-conflict-chain-d @@ -111,7 +93,7 @@ to accept an incorrect (and potentially broken) dependency resolution. See \${REPORT} for a full report. ` -exports[`test/lib/utils/explain-eresolve.js TAP cycleNested > explain with color 1`] = ` +exports[`test/lib/utils/explain-eresolve.js TAP cycleNested > explain with color, depth of 2 1`] = ` Found: @isaacs/peer-dep-cycle-c@2.0.0 node_modules/@isaacs/peer-dep-cycle-c @isaacs/peer-dep-cycle-c@"2.x" from the root project @@ -208,31 +190,7 @@ to accept an incorrect (and potentially broken) dependency resolution. See \${REPORT} for a full report. ` -exports[`test/lib/utils/explain-eresolve.js TAP cycleNested > report with color, depth only 2 1`] = ` -Found: @isaacs/peer-dep-cycle-c@2.0.0 -node_modules/@isaacs/peer-dep-cycle-c - @isaacs/peer-dep-cycle-c@"2.x" from the root project - -Could not resolve dependency: -peer @isaacs/peer-dep-cycle-b@"1" from @isaacs/peer-dep-cycle-a@1.0.0 -node_modules/@isaacs/peer-dep-cycle-a - @isaacs/peer-dep-cycle-a@"1.x" from the root project - -Conflicting peer dependency: @isaacs/peer-dep-cycle-c@1.0.0 -node_modules/@isaacs/peer-dep-cycle-c - peer @isaacs/peer-dep-cycle-c@"1" from @isaacs/peer-dep-cycle-b@1.0.0 - node_modules/@isaacs/peer-dep-cycle-b - peer @isaacs/peer-dep-cycle-b@"1" from @isaacs/peer-dep-cycle-a@1.0.0 - node_modules/@isaacs/peer-dep-cycle-a - -Fix the upstream dependency conflict, or retry -this command with --no-strict-peer-deps, --force, or --legacy-peer-deps -to accept an incorrect (and potentially broken) dependency resolution. - -See \${REPORT} for a full report. -` - -exports[`test/lib/utils/explain-eresolve.js TAP cycleNested > report with no color, depth of 6 1`] = ` +exports[`test/lib/utils/explain-eresolve.js TAP cycleNested > report with no color 1`] = ` Found: @isaacs/peer-dep-cycle-c@2.0.0 node_modules/@isaacs/peer-dep-cycle-c @isaacs/peer-dep-cycle-c@"2.x" from the root project @@ -257,7 +215,7 @@ to accept an incorrect (and potentially broken) dependency resolution. See \${REPORT} for a full report. ` -exports[`test/lib/utils/explain-eresolve.js TAP gatsby > explain with color 1`] = ` +exports[`test/lib/utils/explain-eresolve.js TAP gatsby > explain with color, depth of 2 1`] = ` While resolving: gatsby-recipes@0.2.31 Found: ink@3.0.0-7 node_modules/ink @@ -366,29 +324,7 @@ to accept an incorrect (and potentially broken) dependency resolution. See \${REPORT} for a full report. ` -exports[`test/lib/utils/explain-eresolve.js TAP gatsby > report with color, depth only 2 1`] = ` -While resolving: gatsby-recipes@0.2.31 -Found: ink@3.0.0-7 -node_modules/ink - dev ink@"next" from gatsby-recipes@0.2.31 - node_modules/gatsby-recipes - gatsby-recipes@"^0.2.31" from gatsby-cli@2.12.107 - node_modules/gatsby-cli - -Could not resolve dependency: -peer ink@">=2.0.0" from ink-box@1.0.0 -node_modules/ink-box - ink-box@"^1.0.0" from gatsby-recipes@0.2.31 - node_modules/gatsby-recipes - -Fix the upstream dependency conflict, or retry -this command with --no-strict-peer-deps, --force, or --legacy-peer-deps -to accept an incorrect (and potentially broken) dependency resolution. - -See \${REPORT} for a full report. -` - -exports[`test/lib/utils/explain-eresolve.js TAP gatsby > report with no color, depth of 6 1`] = ` +exports[`test/lib/utils/explain-eresolve.js TAP gatsby > report with no color 1`] = ` While resolving: gatsby-recipes@0.2.31 Found: ink@3.0.0-7 node_modules/ink @@ -409,7 +345,6 @@ node_modules/ink-box node_modules/gatsby-cli gatsby-cli@"^2.12.107" from gatsby@2.24.74 node_modules/gatsby - gatsby@"" from the root project Fix the upstream dependency conflict, or retry this command with --no-strict-peer-deps, --force, or --legacy-peer-deps @@ -418,7 +353,7 @@ to accept an incorrect (and potentially broken) dependency resolution. See \${REPORT} for a full report. ` -exports[`test/lib/utils/explain-eresolve.js TAP no current node, but has current edge > explain with color 1`] = ` +exports[`test/lib/utils/explain-eresolve.js TAP no current node, but has current edge > explain with color, depth of 2 1`] = ` While resolving: eslint@7.22.0 Found: dev eslint@"file:." from the root project @@ -480,23 +415,7 @@ to accept an incorrect (and potentially broken) dependency resolution. See \${REPORT} for a full report. ` -exports[`test/lib/utils/explain-eresolve.js TAP no current node, but has current edge > report with color, depth only 2 1`] = ` -While resolving: eslint@7.22.0 -Found: dev eslint@"file:." from the root project - -Could not resolve dependency: -peer eslint@"^6.0.0" from eslint-plugin-jsdoc@22.2.0 -node_modules/eslint-plugin-jsdoc - dev eslint-plugin-jsdoc@"^22.1.0" from the root project - -Fix the upstream dependency conflict, or retry -this command with --force, or --legacy-peer-deps -to accept an incorrect (and potentially broken) dependency resolution. - -See \${REPORT} for a full report. -` - -exports[`test/lib/utils/explain-eresolve.js TAP no current node, but has current edge > report with no color, depth of 6 1`] = ` +exports[`test/lib/utils/explain-eresolve.js TAP no current node, but has current edge > report with no color 1`] = ` While resolving: eslint@7.22.0 Found: dev eslint@"file:." from the root project @@ -512,7 +431,7 @@ to accept an incorrect (and potentially broken) dependency resolution. See \${REPORT} for a full report. ` -exports[`test/lib/utils/explain-eresolve.js TAP no current node, no current edge, idk > explain with color 1`] = ` +exports[`test/lib/utils/explain-eresolve.js TAP no current node, no current edge, idk > explain with color, depth of 2 1`] = ` While resolving: eslint@7.22.0 Could not resolve dependency: @@ -570,22 +489,7 @@ to accept an incorrect (and potentially broken) dependency resolution. See \${REPORT} for a full report. ` -exports[`test/lib/utils/explain-eresolve.js TAP no current node, no current edge, idk > report with color, depth only 2 1`] = ` -While resolving: eslint@7.22.0 - -Could not resolve dependency: -peer eslint@"^6.0.0" from eslint-plugin-jsdoc@22.2.0 -node_modules/eslint-plugin-jsdoc - dev eslint-plugin-jsdoc@"^22.1.0" from the root project - -Fix the upstream dependency conflict, or retry -this command with --force, or --legacy-peer-deps -to accept an incorrect (and potentially broken) dependency resolution. - -See \${REPORT} for a full report. -` - -exports[`test/lib/utils/explain-eresolve.js TAP no current node, no current edge, idk > report with no color, depth of 6 1`] = ` +exports[`test/lib/utils/explain-eresolve.js TAP no current node, no current edge, idk > report with no color 1`] = ` While resolving: eslint@7.22.0 Could not resolve dependency: @@ -600,7 +504,7 @@ to accept an incorrect (and potentially broken) dependency resolution. See \${REPORT} for a full report. ` -exports[`test/lib/utils/explain-eresolve.js TAP withShrinkwrap > explain with color 1`] = ` +exports[`test/lib/utils/explain-eresolve.js TAP withShrinkwrap > explain with color, depth of 2 1`] = ` While resolving: @isaacs/peer-dep-cycle-b@1.0.0 Found: @isaacs/peer-dep-cycle-c@2.0.0 node_modules/@isaacs/peer-dep-cycle-c @@ -677,26 +581,7 @@ to accept an incorrect (and potentially broken) dependency resolution. See \${REPORT} for a full report. ` -exports[`test/lib/utils/explain-eresolve.js TAP withShrinkwrap > report with color, depth only 2 1`] = ` -While resolving: @isaacs/peer-dep-cycle-b@1.0.0 -Found: @isaacs/peer-dep-cycle-c@2.0.0 -node_modules/@isaacs/peer-dep-cycle-c - @isaacs/peer-dep-cycle-c@"2.x" from the root project - -Could not resolve dependency: -peer @isaacs/peer-dep-cycle-c@"1" from @isaacs/peer-dep-cycle-b@1.0.0 -node_modules/@isaacs/peer-dep-cycle-b - peer @isaacs/peer-dep-cycle-b@"1" from @isaacs/peer-dep-cycle-a@1.0.0 - node_modules/@isaacs/peer-dep-cycle-a - -Fix the upstream dependency conflict, or retry -this command with --no-strict-peer-deps, --force, or --legacy-peer-deps -to accept an incorrect (and potentially broken) dependency resolution. - -See \${REPORT} for a full report. -` - -exports[`test/lib/utils/explain-eresolve.js TAP withShrinkwrap > report with no color, depth of 6 1`] = ` +exports[`test/lib/utils/explain-eresolve.js TAP withShrinkwrap > report with no color 1`] = ` While resolving: @isaacs/peer-dep-cycle-b@1.0.0 Found: @isaacs/peer-dep-cycle-c@2.0.0 node_modules/@isaacs/peer-dep-cycle-c diff --git a/test/lib/cli.js b/test/lib/cli.js index 42e05cc5d14c3..d0a9e0bd49200 100644 --- a/test/lib/cli.js +++ b/test/lib/cli.js @@ -25,6 +25,7 @@ const unsupportedMock = { } let errorHandlerCalled = null +let errorHandlerNpm = null let errorHandlerCb const errorHandlerMock = (...args) => { errorHandlerCalled = args @@ -35,6 +36,9 @@ let errorHandlerExitCalled = null errorHandlerMock.exit = code => { errorHandlerExitCalled = code } +errorHandlerMock.setNpm = npm => { + errorHandlerNpm = npm +} const logs = [] const npmlogMock = { @@ -181,6 +185,7 @@ t.test('gracefully handles error printing usage', t => { npmock.argv = [] errorHandlerCb = () => { t.match(errorHandlerCalled, [], 'should call errorHandler with no args') + t.match(errorHandlerNpm, npmock, 'errorHandler npm is set') t.end() } cli(proc) diff --git a/test/lib/load-all.js b/test/lib/load-all.js index 02736c18ccc38..4a975d49a490e 100644 --- a/test/lib/load-all.js +++ b/test/lib/load-all.js @@ -24,6 +24,7 @@ else { t.test('call the error handle so we dont freak out', t => { const errorHandler = require('../../lib/utils/error-handler.js') + errorHandler.setNpm(npm) errorHandler() t.end() }) diff --git a/test/lib/utils/error-handler.js b/test/lib/utils/error-handler.js index a00bac76e11c2..9a681e52ce5db 100644 --- a/test/lib/utils/error-handler.js +++ b/test/lib/utils/error-handler.js @@ -1,6 +1,7 @@ /* eslint-disable no-extend-native */ /* eslint-disable no-global-assign */ const EventEmitter = require('events') +const writeFileAtomic = require('write-file-atomic') const t = require('tap') // NOTE: Although these unit tests may look like the rest on the surface, @@ -23,13 +24,10 @@ const redactCwd = (path) => { t.cleanSnapshot = (str) => redactCwd(str) // internal modules mocks -const cacheFile = { - append: () => null, - write: () => null, -} +const cacheFolder = t.testdir({}) const config = { values: { - cache: 'cachefolder', + cache: cacheFolder, timing: true, }, loaded: true, @@ -111,20 +109,20 @@ process = Object.assign( // in order for tap to exit properly t.teardown(() => { process = _process + npmlog.record.length = 0 }) const mocks = { npmlog, - '../../../lib/npm.js': npm, '../../../lib/utils/error-message.js': (err) => ({ ...err, summary: [['ERR', err.message]], detail: [['ERR', err.message]], }), - '../../../lib/utils/cache-file.js': cacheFile, } let errorHandler = t.mock('../../../lib/utils/error-handler.js', mocks) +errorHandler.setNpm(npm) t.test('default exit code', (t) => { t.plan(1) @@ -160,10 +158,14 @@ t.test('default exit code', (t) => { t.test('handles unknown error', (t) => { t.plan(2) - cacheFile.write = (filename, content) => { + const _toISOString = Date.prototype.toISOString + Date.prototype.toISOString = () => 'expecteddate' + + const sync = writeFileAtomic.sync + writeFileAtomic.sync = (filename, content) => { t.equal( redactCwd(filename), - '{CWD}/cachefolder/_logs/expecteddate-debug.log', + '{CWD}/test/lib/utils/tap-testdir-error-handler/_logs/expecteddate-debug.log', 'should use expected log filename' ) t.matchSnapshot( @@ -175,7 +177,8 @@ t.test('handles unknown error', (t) => { errorHandler(err) t.teardown(() => { - cacheFile.write = () => null + writeFileAtomic.sync = sync + Date.prototype.toISOString = _toISOString }) t.end() }) @@ -205,11 +208,14 @@ t.test('npm.config not ready', (t) => { t.test('fail to write logfile', (t) => { t.plan(1) - cacheFile.write = () => { - throw err - } + const badDir = t.testdir({ + _logs: 'is a file', + }) + + config.values.cache = badDir + t.teardown(() => { - cacheFile.write = () => null + config.values.cache = cacheFolder }) t.doesNotThrow( @@ -372,6 +378,7 @@ t.test('uses code from errno', (t) => { t.plan(1) errorHandler = t.mock('../../../lib/utils/error-handler.js', mocks) + errorHandler.setNpm(npm) npmlog.level = 'silent' const _exit = process.exit @@ -396,6 +403,7 @@ t.test('uses exitCode as code if using a number', (t) => { t.plan(1) errorHandler = t.mock('../../../lib/utils/error-handler.js', mocks) + errorHandler.setNpm(npm) npmlog.level = 'silent' const _exit = process.exit @@ -420,6 +428,7 @@ t.test('call errorHandler with no error', (t) => { t.plan(1) errorHandler = t.mock('../../../lib/utils/error-handler.js', mocks) + errorHandler.setNpm(npm) const _exit = process.exit process.exit = (code) => { @@ -478,6 +487,7 @@ t.test('set it worked', (t) => { t.plan(1) errorHandler = t.mock('../../../lib/utils/error-handler.js', mocks) + errorHandler.setNpm(npm) const _exit = process.exit process.exit = () => { diff --git a/test/lib/utils/error-message.js b/test/lib/utils/error-message.js index 7529aac2d4a4b..4f94645a4542d 100644 --- a/test/lib/utils/error-message.js +++ b/test/lib/utils/error-message.js @@ -1,4 +1,5 @@ const t = require('tap') +const path = require('path') // make a bunch of stuff consistent for snapshots @@ -48,7 +49,7 @@ const mocks = { return 'explanation' }, }, - '../../../lib/npm.js': require('../../../lib/npm.js'), + // XXX ??? get '../../../lib/utils/is-windows.js' () { return process.platform === 'win32' }, @@ -110,7 +111,7 @@ t.test('just simple messages', t => { file, stack, }) - t.matchSnapshot(errorMessage(er)) + t.matchSnapshot(errorMessage(er, npm)) }) }) @@ -128,7 +129,7 @@ t.test('replace message/stack sensistive info', t => { file, stack, }) - t.matchSnapshot(errorMessage(er)) + t.matchSnapshot(errorMessage(er, npm)) t.end() }) @@ -148,7 +149,7 @@ t.test('bad engine with config loaded', t => { file, stack, }) - t.matchSnapshot(errorMessage(er)) + t.matchSnapshot(errorMessage(er, npm)) t.end() }) @@ -162,7 +163,7 @@ t.test('enoent without a file', t => { pkgid, stack, }) - t.matchSnapshot(errorMessage(er)) + t.matchSnapshot(errorMessage(er, npm)) t.end() }) @@ -179,20 +180,20 @@ t.test('enolock without a command', t => { file, stack, }) - t.matchSnapshot(errorMessage(er)) + t.matchSnapshot(errorMessage(er, npm)) t.end() }) t.test('default message', t => { - t.matchSnapshot(errorMessage(new Error('error object'))) - t.matchSnapshot(errorMessage('error string')) + t.matchSnapshot(errorMessage(new Error('error object'), npm)) + t.matchSnapshot(errorMessage('error string'), npm) t.matchSnapshot(errorMessage(Object.assign(new Error('cmd err'), { cmd: 'some command', signal: 'SIGYOLO', args: ['a', 'r', 'g', 's'], stdout: 'stdout', stderr: 'stderr', - }))) + }), npm)) t.end() }) @@ -213,7 +214,7 @@ t.test('eacces/eperm', t => { stack: 'dummy stack trace', }) verboseLogs.length = 0 - t.matchSnapshot(errorMessage(er)) + t.matchSnapshot(errorMessage(er, npm)) t.matchSnapshot(verboseLogs) t.end() verboseLogs.length = 0 @@ -288,7 +289,7 @@ t.test('json parse', t => { t.matchSnapshot(errorMessage(Object.assign(new Error('conflicted'), { code: 'EJSONPARSE', file: resolve(dir, 'package.json'), - }))) + }), npm)) t.end() }) @@ -310,7 +311,7 @@ t.test('json parse', t => { t.matchSnapshot(errorMessage(Object.assign(new Error('not json'), { code: 'EJSONPARSE', file: resolve(dir, 'package.json'), - }))) + }), npm)) t.end() }) @@ -326,7 +327,7 @@ t.test('json parse', t => { t.matchSnapshot(errorMessage(Object.assign(new Error('not json'), { code: 'EJSONPARSE', file: `${dir}/blerg.json`, - }))) + }), npm)) t.end() }) @@ -337,21 +338,21 @@ t.test('eotp/e401', t => { t.test('401, no auth headers', t => { t.matchSnapshot(errorMessage(Object.assign(new Error('nope'), { code: 'E401', - }))) + }), npm)) t.end() }) t.test('401, no message', t => { t.matchSnapshot(errorMessage({ code: 'E401', - })) + }, npm)) t.end() }) t.test('one-time pass challenge code', t => { t.matchSnapshot(errorMessage(Object.assign(new Error('nope'), { code: 'EOTP', - }))) + }), npm)) t.end() }) @@ -359,7 +360,7 @@ t.test('eotp/e401', t => { const message = 'one-time pass' t.matchSnapshot(errorMessage(Object.assign(new Error(message), { code: 'E401', - }))) + }), npm)) t.end() }) @@ -379,7 +380,7 @@ t.test('eotp/e401', t => { }, code: 'E401', }) - t.matchSnapshot(errorMessage(er)) + t.matchSnapshot(errorMessage(er, npm)) t.end() }) } @@ -391,7 +392,7 @@ t.test('eotp/e401', t => { t.test('404', t => { t.test('no package id', t => { const er = Object.assign(new Error('404 not found'), { code: 'E404' }) - t.matchSnapshot(errorMessage(er)) + t.matchSnapshot(errorMessage(er, npm)) t.end() }) t.test('you should publish it', t => { @@ -399,7 +400,7 @@ t.test('404', t => { pkgid: 'yolo', code: 'E404', }) - t.matchSnapshot(errorMessage(er)) + t.matchSnapshot(errorMessage(er, npm)) t.end() }) t.test('name with warning', t => { @@ -407,7 +408,7 @@ t.test('404', t => { pkgid: new Array(215).fill('x').join(''), code: 'E404', }) - t.matchSnapshot(errorMessage(er)) + t.matchSnapshot(errorMessage(er, npm)) t.end() }) t.test('name with error', t => { @@ -415,7 +416,7 @@ t.test('404', t => { pkgid: 'node_modules', code: 'E404', }) - t.matchSnapshot(errorMessage(er)) + t.matchSnapshot(errorMessage(er, npm)) t.end() }) t.end() @@ -435,7 +436,7 @@ t.test('bad platform', t => { }, code: 'EBADPLATFORM', }) - t.matchSnapshot(errorMessage(er)) + t.matchSnapshot(errorMessage(er, npm)) t.end() }) t.test('array os/arch', t => { @@ -451,7 +452,7 @@ t.test('bad platform', t => { }, code: 'EBADPLATFORM', }) - t.matchSnapshot(errorMessage(er)) + t.matchSnapshot(errorMessage(er, npm)) t.end() }) @@ -462,7 +463,11 @@ t.test('explain ERESOLVE errors', t => { const er = Object.assign(new Error('could not resolve'), { code: 'ERESOLVE', }) - t.matchSnapshot(errorMessage(er)) - t.strictSame(EXPLAIN_CALLED, [[er]]) + t.matchSnapshot(errorMessage(er, npm)) + t.match(EXPLAIN_CALLED, [[ + er, + undefined, + path.resolve(npm.cache, 'eresolve-report.txt'), + ]]) t.end() }) diff --git a/test/lib/utils/explain-eresolve.js b/test/lib/utils/explain-eresolve.js index 90795bb4470b2..f9710ee889ab1 100644 --- a/test/lib/utils/explain-eresolve.js +++ b/test/lib/utils/explain-eresolve.js @@ -1,8 +1,6 @@ const t = require('tap') const npm = {} -const { explain, report } = t.mock('../../../lib/utils/explain-eresolve.js', { - '../../../lib/npm.js': npm, -}) +const { explain, report } = require('../../../lib/utils/explain-eresolve.js') const { statSync, readFileSync, unlinkSync } = require('fs') // strip out timestamps from reports const read = f => readFileSync(f, 'utf8') @@ -25,23 +23,18 @@ for (const [name, expl] of Object.entries(cases)) { t.cleanSnapshot = str => str.split(reportFile).join('${REPORT}') npm.color = true - t.matchSnapshot(report(expl), 'report with color') + t.matchSnapshot(report(expl, true, reportFile), 'report with color') const reportData = read(reportFile) t.matchSnapshot(reportData, 'report') unlinkSync(reportFile) - t.matchSnapshot(report(expl, 2), 'report with color, depth only 2') + + t.matchSnapshot(report(expl, false, reportFile), 'report with no color') t.equal(read(reportFile), reportData, 'same report written for object') unlinkSync(reportFile) - npm.color = false - t.matchSnapshot(report(expl, 6), 'report with no color, depth of 6') - t.equal(read(reportFile), reportData, 'same report written for object') - unlinkSync(reportFile) - npm.color = true - t.matchSnapshot(explain(expl), 'explain with color') + t.matchSnapshot(explain(expl, true, 2), 'explain with color, depth of 2') t.throws(() => statSync(reportFile), { code: 'ENOENT' }, 'no report') - npm.color = false - t.matchSnapshot(explain(expl, 6), 'explain with no color, depth of 6') + t.matchSnapshot(explain(expl, false, 6), 'explain with no color, depth of 6') t.throws(() => statSync(reportFile), { code: 'ENOENT' }, 'no report') t.end() diff --git a/test/lib/utils/setup-log.js b/test/lib/utils/setup-log.js index 3daf3b8a52f53..86befe6e29297 100644 --- a/test/lib/utils/setup-log.js +++ b/test/lib/utils/setup-log.js @@ -92,7 +92,7 @@ t.test('setup with color=always and unicode', t => { })), true) npmlog.warn('ERESOLVE', 'hello', { some: { other: 'object' } }) - t.strictSame(EXPLAIN_CALLED, [[{ some: { other: 'object' } }]], + t.strictSame(EXPLAIN_CALLED, [[{ some: { other: 'object' } }, true, 2]], 'log.warn(ERESOLVE) patched to call explainEresolve()') t.strictSame(WARN_CALLED, [ ['ERESOLVE', 'hello'],