From 5c4483c6de0f8fafa01088c1fa9bb6cf8b841700 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 23 Mar 2023 16:12:52 -0700 Subject: [PATCH] fix: make all color output use an npm instance of chalk --- lib/commands/explain.js | 2 +- lib/commands/fund.js | 6 +- lib/commands/help-search.js | 7 +- lib/commands/ls.js | 28 +- lib/commands/outdated.js | 17 +- lib/commands/profile.js | 3 +- lib/commands/run-script.js | 12 +- lib/commands/token.js | 3 +- lib/commands/view.js | 2 +- lib/npm.js | 24 +- lib/utils/display.js | 7 +- lib/utils/error-message.js | 2 +- lib/utils/explain-dep.js | 69 ++-- lib/utils/explain-eresolve.js | 22 +- lib/utils/update-notifier.js | 19 +- .../test/lib/commands/profile.js.test.cjs | 95 ++--- .../lib/utils/update-notifier.js.test.cjs | 72 ++-- test/lib/commands/explain.js | 3 +- test/lib/commands/profile.js | 16 +- test/lib/commands/view.js | 6 +- test/lib/utils/display.js | 2 +- test/lib/utils/error-message.js | 10 +- test/lib/utils/explain-dep.js | 15 +- test/lib/utils/explain-eresolve.js | 28 +- test/lib/utils/update-notifier.js | 324 ++++++++---------- 25 files changed, 381 insertions(+), 413 deletions(-) diff --git a/lib/commands/explain.js b/lib/commands/explain.js index a06ad24152a1e..44c32c0e5663a 100644 --- a/lib/commands/explain.js +++ b/lib/commands/explain.js @@ -78,7 +78,7 @@ class Explain extends ArboristWorkspaceCmd { this.npm.output(JSON.stringify(expls, null, 2)) } else { this.npm.output(expls.map(expl => { - return explainNode(expl, Infinity, this.npm.color) + return explainNode(expl, Infinity, this.npm.chalk) }).join('\n\n')) } } diff --git a/lib/commands/fund.js b/lib/commands/fund.js index 12762533c123e..479ff487ec8b0 100644 --- a/lib/commands/fund.js +++ b/lib/commands/fund.js @@ -1,6 +1,5 @@ const archy = require('archy') const Arborist = require('@npmcli/arborist') -const chalk = require('chalk') const pacote = require('pacote') const semver = require('semver') const npa = require('npm-package-arg') @@ -96,7 +95,6 @@ class Fund extends ArboristWorkspaceCmd { } printHuman (fundingInfo) { - const color = this.npm.color const unicode = this.npm.config.get('unicode') const seenUrls = new Map() @@ -117,7 +115,7 @@ class Fund extends ArboristWorkspaceCmd { if (url) { item.label = tree({ - label: color ? chalk.bgBlack.white(url) : url, + label: this.npm.chalk.bgBlack.white(url), nodes: [pkgRef], }).trim() @@ -154,7 +152,7 @@ class Fund extends ArboristWorkspaceCmd { }) const res = tree(result) - return color ? chalk.reset(res) : res + return this.npm.chalk.reset(res) } async openFundingUrl ({ path, tree, spec, fundingSourceNumber }) { diff --git a/lib/commands/help-search.js b/lib/commands/help-search.js index 1e69b9784e8f4..0a706be70a067 100644 --- a/lib/commands/help-search.js +++ b/lib/commands/help-search.js @@ -1,6 +1,5 @@ const { readFile } = require('fs/promises') const path = require('path') -const chalk = require('chalk') const glob = require('glob') const BaseCommand = require('../base-command.js') @@ -163,10 +162,6 @@ class HelpSearch extends BaseCommand { return } - if (!this.npm.color) { - out.push(line + '\n') - return - } const hilitLine = [] for (const arg of args) { const finder = line.toLowerCase().split(arg.toLowerCase()) @@ -174,7 +169,7 @@ class HelpSearch extends BaseCommand { for (const f of finder) { hilitLine.push(line.slice(p, p + f.length)) const word = line.slice(p + f.length, p + f.length + arg.length) - const hilit = chalk.bgBlack.red(word) + const hilit = this.npm.chalk.bgBlack.red(word) hilitLine.push(hilit) p += f.length + arg.length } diff --git a/lib/commands/ls.js b/lib/commands/ls.js index 2213e7937407a..a737f44b73b29 100644 --- a/lib/commands/ls.js +++ b/lib/commands/ls.js @@ -3,7 +3,6 @@ const relativePrefix = `.${sep}` const { EOL } = require('os') const archy = require('archy') -const chalk = require('chalk') const Arborist = require('@npmcli/arborist') const { breadth } = require('treeverse') const npa = require('npm-package-arg') @@ -50,7 +49,7 @@ class LS extends ArboristWorkspaceCmd { async exec (args) { const all = this.npm.config.get('all') - const color = this.npm.color + const chalk = this.npm.chalk const depth = this.npm.config.get('depth') const global = this.npm.global const json = this.npm.config.get('json') @@ -157,7 +156,7 @@ class LS extends ArboristWorkspaceCmd { ? getJsonOutputItem(node, { global, long }) : parseable ? null - : getHumanOutputItem(node, { args, color, global, long }) + : getHumanOutputItem(node, { args, chalk, global, long }) // loop through list of node problems to add them to global list if (node[_include]) { @@ -180,7 +179,7 @@ class LS extends ArboristWorkspaceCmd { this.npm.outputBuffer( json ? jsonOutput({ path, problems, result, rootError, seenItems }) : parseable ? parseableOutput({ seenNodes, global, long }) : - humanOutput({ color, result, seenItems, unicode }) + humanOutput({ chalk, result, seenItems, unicode }) ) // if filtering items, should exit with error code on no results @@ -278,9 +277,9 @@ const augmentItemWithIncludeMetadata = (node, item) => { return item } -const getHumanOutputItem = (node, { args, color, global, long }) => { +const getHumanOutputItem = (node, { args, chalk, global, long }) => { const { pkgid, path } = node - const workspacePkgId = color ? chalk.green(pkgid) : pkgid + const workspacePkgId = chalk.green(pkgid) let printable = node.isWorkspace ? workspacePkgId : pkgid // special formatting for top-level package name @@ -293,8 +292,7 @@ const getHumanOutputItem = (node, { args, color, global, long }) => { } } - const highlightDepName = - color && args.length && node[_filteredBy] + const highlightDepName = args.length && node[_filteredBy] const missingColor = isOptional(node) ? chalk.yellow.bgBlack : chalk.red.bgBlack @@ -308,28 +306,28 @@ const getHumanOutputItem = (node, { args, color, global, long }) => { const label = ( node[_missing] - ? (color ? missingColor(missingMsg) : missingMsg) + ' ' + ? missingColor(missingMsg) + ' ' : '' ) + `${highlightDepName ? chalk.yellow.bgBlack(printable) : printable}` + ( node[_dedupe] - ? ' ' + (color ? chalk.gray('deduped') : 'deduped') + ? ' ' + chalk.gray('deduped') : '' ) + ( invalid - ? ' ' + (color ? chalk.red.bgBlack(invalid) : invalid) + ? ' ' + chalk.red.bgBlack(invalid) : '' ) + ( isExtraneous(node, { global }) - ? ' ' + (color ? chalk.green.bgBlack('extraneous') : 'extraneous') + ? ' ' + chalk.green.bgBlack('extraneous') : '' ) + ( node.overridden - ? ' ' + (color ? chalk.gray('overridden') : 'overridden') + ? ' ' + chalk.gray('overridden') : '' ) + (isGitNode(node) ? ` (${node.resolved})` : '') + @@ -504,7 +502,7 @@ const augmentNodesWithMetadata = ({ const sortAlphabetically = ({ pkgid: a }, { pkgid: b }) => localeCompare(a, b) -const humanOutput = ({ color, result, seenItems, unicode }) => { +const humanOutput = ({ chalk, result, seenItems, unicode }) => { // we need to traverse the entire tree in order to determine which items // should be included (since a nested transitive included dep will make it // so that all its ancestors should be displayed) @@ -520,7 +518,7 @@ const humanOutput = ({ color, result, seenItems, unicode }) => { } const archyOutput = archy(result, '', { unicode }) - return color ? chalk.reset(archyOutput) : archyOutput + return chalk.reset(archyOutput) } const jsonOutput = ({ path, problems, result, rootError, seenItems }) => { diff --git a/lib/commands/outdated.js b/lib/commands/outdated.js index 5e8a4e0d2168c..49626ebd5e20e 100644 --- a/lib/commands/outdated.js +++ b/lib/commands/outdated.js @@ -2,7 +2,6 @@ const os = require('os') const { resolve } = require('path') const pacote = require('pacote') const table = require('text-table') -const chalk = require('chalk') const npa = require('npm-package-arg') const pickManifest = require('npm-pick-manifest') const localeCompare = require('@isaacs/string-locale-compare')('en') @@ -104,9 +103,7 @@ class Outdated extends ArboristWorkspaceCmd { } const outTable = [outHead].concat(outList) - if (this.npm.color) { - outTable[0] = outTable[0].map(heading => chalk.underline(heading)) - } + outTable[0] = outTable[0].map(heading => this.npm.chalk.underline(heading)) const tableOpts = { align: ['l', 'r', 'r', 'r', 'l'], @@ -281,8 +278,8 @@ class Outdated extends ArboristWorkspaceCmd { ? node.pkgid : node.name - return this.npm.color && humanOutput - ? chalk.green(workspaceName) + return humanOutput + ? this.npm.chalk.green(workspaceName) : workspaceName } @@ -306,11 +303,9 @@ class Outdated extends ArboristWorkspaceCmd { columns[7] = homepage } - if (this.npm.color) { - columns[0] = chalk[current === wanted ? 'yellow' : 'red'](columns[0]) // current - columns[2] = chalk.green(columns[2]) // wanted - columns[3] = chalk.magenta(columns[3]) // latest - } + columns[0] = this.npm.chalk[current === wanted ? 'yellow' : 'red'](columns[0]) // current + columns[2] = this.npm.chalk.green(columns[2]) // wanted + columns[3] = this.npm.chalk.magenta(columns[3]) // latest return columns } diff --git a/lib/commands/profile.js b/lib/commands/profile.js index e42ebb276d202..4fba1209e0335 100644 --- a/lib/commands/profile.js +++ b/lib/commands/profile.js @@ -1,6 +1,5 @@ const inspect = require('util').inspect const { URL } = require('url') -const chalk = require('chalk') const log = require('../utils/log-shim.js') const npmProfile = require('npm-profile') const qrcodeTerminal = require('qrcode-terminal') @@ -161,7 +160,7 @@ class Profile extends BaseCommand { } else { const table = new Table() for (const key of Object.keys(cleaned)) { - table.push({ [chalk.bold(key)]: cleaned[key] }) + table.push({ [this.npm.chalk.bold(key)]: cleaned[key] }) } this.npm.output(table.toString()) diff --git a/lib/commands/run-script.js b/lib/commands/run-script.js index 40e18e1ea0644..e1bce0e52a513 100644 --- a/lib/commands/run-script.js +++ b/lib/commands/run-script.js @@ -1,5 +1,4 @@ const { resolve } = require('path') -const chalk = require('chalk') const runScript = require('@npmcli/run-script') const { isServerPackage } = runScript const rpj = require('read-package-json-fast') @@ -18,14 +17,6 @@ const cmdList = [ 'version', ].reduce((l, p) => l.concat(['pre' + p, p, 'post' + p]), []) -const nocolor = { - reset: s => s, - bold: s => s, - dim: s => s, - blue: s => s, - green: s => s, -} - const BaseCommand = require('../base-command.js') class RunScript extends BaseCommand { static description = 'Run arbitrary package scripts' @@ -138,7 +129,6 @@ class RunScript extends BaseCommand { path = path || this.npm.localPrefix const { scripts, name, _id } = await rpj(`${path}/package.json`) const pkgid = _id || name - const color = this.npm.color if (!scripts) { return [] @@ -170,7 +160,7 @@ class RunScript extends BaseCommand { const list = cmdList.includes(script) ? cmds : runScripts list.push(script) } - const colorize = color ? chalk : nocolor + const colorize = this.npm.chalk if (cmds.length) { this.npm.output( diff --git a/lib/commands/token.js b/lib/commands/token.js index 8da8311875714..bc2e4f3796364 100644 --- a/lib/commands/token.js +++ b/lib/commands/token.js @@ -1,5 +1,4 @@ const Table = require('cli-table3') -const chalk = require('chalk') const { v4: isCidrV4, v6: isCidrV6 } = require('is-cidr') const log = require('../utils/log-shim.js') const profile = require('npm-profile') @@ -152,7 +151,7 @@ class Token extends BaseCommand { } else { const table = new Table() for (const k of Object.keys(result)) { - table.push({ [chalk.bold(k)]: String(result[k]) }) + table.push({ [this.npm.chalk.bold(k)]: String(result[k]) }) } this.npm.output(table.toString()) } diff --git a/lib/commands/view.js b/lib/commands/view.js index 855b37b81d42f..bbe7dcdd18bbf 100644 --- a/lib/commands/view.js +++ b/lib/commands/view.js @@ -1,4 +1,3 @@ -const chalk = require('chalk') const columns = require('cli-columns') const fs = require('fs') const jsonParse = require('json-parse-even-better-errors') @@ -315,6 +314,7 @@ class View extends BaseCommand { prettyView (packu, manifest) { // More modern, pretty printing of default view const unicode = this.npm.config.get('unicode') + const chalk = this.npm.chalk const tags = [] Object.keys(packu['dist-tags']).forEach((t) => { diff --git a/lib/npm.js b/lib/npm.js index 841d145ddcbad..155a768496550 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -36,6 +36,8 @@ class Npm extends EventEmitter { #title = 'npm' #argvClean = [] #chalk = null + #logChalk = null + #noColorChalk = new chalk.Instance({ level: 0 }) #npmRoot = null #warnedNonDashArg = false @@ -248,6 +250,7 @@ class Npm extends EventEmitter { this.#display.load({ // Use logColor since that is based on stderr color: this.logColor, + chalk: this.logChalk, progress: this.flatOptions.progress, silent: this.silent, timing: this.config.get('timing'), @@ -317,17 +320,28 @@ class Npm extends EventEmitter { return this.flatOptions.logColor } + get noColorChalk () { + return this.#noColorChalk + } + get chalk () { if (!this.#chalk) { - let level = chalk.level - if (!this.color) { - level = 0 - } - this.#chalk = new chalk.Instance({ level }) + this.#chalk = new chalk.Instance({ + level: this.color ? chalk.level : 0, + }) } return this.#chalk } + get logChalk () { + if (!this.#logChalk) { + this.#logChalk = new chalk.Instance({ + level: this.logColor ? chalk.stderr.level : 0, + }) + } + return this.#logChalk + } + get global () { return this.config.get('global') || this.config.get('location') === 'global' } diff --git a/lib/utils/display.js b/lib/utils/display.js index 35d221c09cae8..a41bf903e9a8f 100644 --- a/lib/utils/display.js +++ b/lib/utils/display.js @@ -4,6 +4,8 @@ const log = require('./log-shim.js') const { explain } = require('./explain-eresolve.js') class Display { + #chalk = null + constructor () { // pause by default until config is loaded this.on() @@ -26,6 +28,7 @@ class Display { load (config) { const { color, + chalk, timing, loglevel, unicode, @@ -34,6 +37,8 @@ class Display { heading = 'npm', } = config + this.#chalk = chalk + // npmlog is still going away someday, so this is a hack to dynamically // set the loglevel of timing based on the timing flag, instead of making // a breaking change to npmlog. The result is that timing logs are never @@ -111,7 +116,7 @@ class Display { expl && typeof expl === 'object' ) { this.#npmlog(level, heading, message) - this.#npmlog(level, '', explain(expl, log.useColor(), 2)) + this.#npmlog(level, '', explain(expl, this.#chalk, 2)) // Return true to short circuit other log in chain return true } diff --git a/lib/utils/error-message.js b/lib/utils/error-message.js index 85a40bb32e0d7..a2cdb0aa48068 100644 --- a/lib/utils/error-message.js +++ b/lib/utils/error-message.js @@ -38,7 +38,7 @@ const errorMessage = (er, npm) => { // XXX(display): error messages are logged so we use the logColor since that is based // on stderr. This should be handled solely by the display layer so it could also be // printed to stdout if necessary. - const { explanation, file } = report(er, !!npm.logColor) + const { explanation, file } = report(er, npm.logChalk, npm.noColorChalk) detail.push(['', explanation]) files.push(['eresolve-report.txt', file]) break diff --git a/lib/utils/explain-dep.js b/lib/utils/explain-dep.js index 58258026491dc..86660d5d3ad4b 100644 --- a/lib/utils/explain-dep.js +++ b/lib/utils/explain-dep.js @@ -1,25 +1,12 @@ -const chalk = require('chalk') -const nocolor = { - bold: s => s, - dim: s => s, - red: s => s, - yellow: s => s, - cyan: s => s, - magenta: s => s, - blue: s => s, - green: s => s, - gray: s => s, -} - const { relative } = require('path') -const explainNode = (node, depth, color) => - printNode(node, color) + - explainDependents(node, depth, color) + - explainLinksIn(node, depth, color) +const explainNode = (node, depth, chalk) => + printNode(node, chalk) + + explainDependents(node, depth, chalk) + + explainLinksIn(node, depth, chalk) -const colorType = (type, color) => { - const { red, yellow, cyan, magenta, blue, green, gray } = color ? chalk : nocolor +const colorType = (type, chalk) => { + const { red, yellow, cyan, magenta, blue, green, gray } = chalk const style = type === 'extraneous' ? red : type === 'dev' ? yellow : type === 'optional' ? cyan @@ -31,7 +18,7 @@ const colorType = (type, color) => { return style(type) } -const printNode = (node, color) => { +const printNode = (node, chalk) => { const { name, version, @@ -44,30 +31,30 @@ const printNode = (node, color) => { isWorkspace, overridden, } = node - const { bold, dim, green } = color ? chalk : nocolor + const { bold, dim, green } = chalk const extra = [] if (extraneous) { - extra.push(' ' + bold(colorType('extraneous', color))) + extra.push(' ' + bold(colorType('extraneous', chalk))) } if (dev) { - extra.push(' ' + bold(colorType('dev', color))) + extra.push(' ' + bold(colorType('dev', chalk))) } if (optional) { - extra.push(' ' + bold(colorType('optional', color))) + extra.push(' ' + bold(colorType('optional', chalk))) } if (peer) { - extra.push(' ' + bold(colorType('peer', color))) + extra.push(' ' + bold(colorType('peer', chalk))) } if (bundled) { - extra.push(' ' + bold(colorType('bundled', color))) + extra.push(' ' + bold(colorType('bundled', chalk))) } if (overridden) { - extra.push(' ' + bold(colorType('overridden', color))) + extra.push(' ' + bold(colorType('overridden', chalk))) } const pkgid = isWorkspace @@ -78,24 +65,24 @@ const printNode = (node, color) => { (location ? dim(`\n${location}`) : '') } -const explainLinksIn = ({ linksIn }, depth, color) => { +const explainLinksIn = ({ linksIn }, depth, chalk) => { if (!linksIn || !linksIn.length || depth <= 0) { return '' } - const messages = linksIn.map(link => explainNode(link, depth - 1, color)) + const messages = linksIn.map(link => explainNode(link, depth - 1, chalk)) const str = '\n' + messages.join('\n') return str.split('\n').join('\n ') } -const explainDependents = ({ name, dependents }, depth, color) => { +const explainDependents = ({ name, dependents }, depth, chalk) => { if (!dependents || !dependents.length || depth <= 0) { return '' } const max = Math.ceil(depth / 2) const messages = dependents.slice(0, max) - .map(edge => explainEdge(edge, depth, color)) + .map(edge => explainEdge(edge, depth, chalk)) // show just the names of the first 5 deps that overflowed the list if (dependents.length > max) { @@ -119,30 +106,30 @@ const explainDependents = ({ name, dependents }, depth, color) => { return str.split('\n').join('\n ') } -const explainEdge = ({ name, type, bundled, from, spec, rawSpec, overridden }, depth, color) => { - const { bold } = color ? chalk : nocolor +const explainEdge = ({ name, type, bundled, from, spec, rawSpec, overridden }, depth, chalk) => { + const { bold } = chalk let dep = type === 'workspace' ? bold(relative(from.location, spec.slice('file:'.length))) : `${bold(name)}@"${bold(spec)}"` if (overridden) { - dep = `${colorType('overridden', color)} ${dep} (was "${rawSpec}")` + dep = `${colorType('overridden', chalk)} ${dep} (was "${rawSpec}")` } - const fromMsg = ` from ${explainFrom(from, depth, color)}` + const fromMsg = ` from ${explainFrom(from, depth, chalk)}` - return (type === 'prod' ? '' : `${colorType(type, color)} `) + - (bundled ? `${colorType('bundled', color)} ` : '') + + return (type === 'prod' ? '' : `${colorType(type, chalk)} `) + + (bundled ? `${colorType('bundled', chalk)} ` : '') + `${dep}${fromMsg}` } -const explainFrom = (from, depth, color) => { +const explainFrom = (from, depth, chalk) => { if (!from.name && !from.version) { return 'the root project' } - return printNode(from, color) + - explainDependents(from, depth - 1, color) + - explainLinksIn(from, depth - 1, color) + return printNode(from, chalk) + + explainDependents(from, depth - 1, chalk) + + explainLinksIn(from, depth - 1, chalk) } module.exports = { explainNode, printNode, explainEdge } diff --git a/lib/utils/explain-eresolve.js b/lib/utils/explain-eresolve.js index 480cd8e5cd4e6..ba46f3480adb3 100644 --- a/lib/utils/explain-eresolve.js +++ b/lib/utils/explain-eresolve.js @@ -7,7 +7,7 @@ const { explainEdge, explainNode, printNode } = require('./explain-dep.js') // 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 explain = (expl, color, depth) => { +const explain = (expl, chalk, depth) => { const { edge, dep, current, peerConflict, currentEdge } = expl const out = [] @@ -15,28 +15,28 @@ const explain = (expl, color, depth) => { current && current.whileInstalling || edge && edge.from && edge.from.whileInstalling if (whileInstalling) { - out.push('While resolving: ' + printNode(whileInstalling, color)) + out.push('While resolving: ' + printNode(whileInstalling, chalk)) } // it "should" be impossible for an ERESOLVE explanation to lack both // current and currentEdge, but better to have a less helpful error // than a crashing failure. if (current) { - out.push('Found: ' + explainNode(current, depth, color)) + out.push('Found: ' + explainNode(current, depth, chalk)) } else if (peerConflict && peerConflict.current) { - out.push('Found: ' + explainNode(peerConflict.current, depth, color)) + out.push('Found: ' + explainNode(peerConflict.current, depth, chalk)) } else if (currentEdge) { - out.push('Found: ' + explainEdge(currentEdge, depth, color)) + out.push('Found: ' + explainEdge(currentEdge, depth, chalk)) } else /* istanbul ignore else - should always have one */ if (edge) { - out.push('Found: ' + explainEdge(edge, depth, color)) + out.push('Found: ' + explainEdge(edge, depth, chalk)) } out.push('\nCould not resolve dependency:\n' + - explainEdge(edge, depth, color)) + explainEdge(edge, depth, chalk)) if (peerConflict) { const heading = '\nConflicting peer dependency:' - const pc = explainNode(peerConflict.peer, depth, color) + const pc = explainNode(peerConflict.peer, depth, chalk) out.push(heading + ' ' + pc) } @@ -44,7 +44,7 @@ const explain = (expl, color, depth) => { } // generate a full verbose report and tell the user how to fix it -const report = (expl, color) => { +const report = (expl, chalk, noColor) => { const flags = [ expl.strictPeerDeps ? '--no-strict-peer-deps' : '', '--force', @@ -60,8 +60,8 @@ this command with ${or(flags)} to accept an incorrect (and potentially broken) dependency resolution.` return { - explanation: `${explain(expl, color, 4)}\n\n${fix}`, - file: `# npm resolution error report\n\n${explain(expl, false, Infinity)}\n\n${fix}`, + explanation: `${explain(expl, chalk, 4)}\n\n${fix}`, + file: `# npm resolution error report\n\n${explain(expl, noColor, Infinity)}\n\n${fix}`, } } diff --git a/lib/utils/update-notifier.js b/lib/utils/update-notifier.js index a7eaaca64747f..cb9184bcdb41d 100644 --- a/lib/utils/update-notifier.js +++ b/lib/utils/update-notifier.js @@ -5,10 +5,7 @@ const pacote = require('pacote') const ciInfo = require('ci-info') const semver = require('semver') -const chalk = require('chalk') -const { promisify } = require('util') -const stat = promisify(require('fs').stat) -const writeFile = promisify(require('fs').writeFile) +const { stat, writeFile } = require('fs/promises') const { resolve } = require('path') const SKIP = Symbol('SKIP') @@ -61,10 +58,6 @@ const updateNotifier = async (npm, spec = 'latest') => { return null } - // if they're currently using a prerelease, nudge to the next prerelease - // otherwise, nudge to latest. - const useColor = npm.logColor - const mani = await pacote.manifest(`npm@${spec}`, { // always prefer latest, even if doing --tag=whatever on the cmd defaultTag: 'latest', @@ -91,6 +84,9 @@ const updateNotifier = async (npm, spec = 'latest') => { return null } + const useColor = npm.logColor + const chalk = npm.logChalk + // ok! notify the user about this update they should get. // The message is saved for printing at process exit so it will not get // lost in any other messages being printed as part of the command. @@ -99,12 +95,11 @@ const updateNotifier = async (npm, spec = 'latest') => { : update.minor !== current.minor ? 'minor' : update.patch !== current.patch ? 'patch' : 'prerelease' - const typec = !useColor ? type - : type === 'major' ? chalk.red(type) + const typec = type === 'major' ? chalk.red(type) : type === 'minor' ? chalk.yellow(type) : chalk.green(type) - const oldc = !useColor ? current : chalk.red(current) - const latestc = !useColor ? latest : chalk.green(latest) + const oldc = chalk.red(current) + const latestc = chalk.green(latest) const changelog = `https://github.com/npm/cli/releases/tag/v${latest}` const changelogc = !useColor ? `<${changelog}>` : chalk.cyan(changelog) const cmd = `npm install -g npm@${latest}` diff --git a/tap-snapshots/test/lib/commands/profile.js.test.cjs b/tap-snapshots/test/lib/commands/profile.js.test.cjs index 4959f7cdd2cc3..4530dbf95cec2 100644 --- a/tap-snapshots/test/lib/commands/profile.js.test.cjs +++ b/tap-snapshots/test/lib/commands/profile.js.test.cjs @@ -31,6 +31,19 @@ exports[`test/lib/commands/profile.js TAP profile get multiple args default outp foo foo@github.com (verified) https://github.com/npm ` +exports[`test/lib/commands/profile.js TAP profile get no args --color > should output all profile info with color result 1`] = ` +name: foo +email: foo@github.com (verified) +two-factor auth: auth-and-writes +fullname: Foo Bar +homepage: https://github.com +freenode: foobar +twitter: https://twitter.com/npmjs +github: https://github.com/npm +created: 2015-02-26T01:26:37.384Z +updated: 2020-08-12T16:19:35.326Z +` + exports[`test/lib/commands/profile.js TAP profile get no args --parseable > should output all profile info as parseable result 1`] = ` tfa auth-and-writes name foo @@ -46,56 +59,56 @@ github https://github.com/npm ` exports[`test/lib/commands/profile.js TAP profile get no args default output > should output table with contents 1`] = ` -name: foo -email: foo@github.com (verified) -two-factor auth: auth-and-writes -fullname: Foo Bar -homepage: https://github.com -freenode: foobar -twitter: https://twitter.com/npmjs -github: https://github.com/npm -created: 2015-02-26T01:26:37.384Z -updated: 2020-08-12T16:19:35.326Z +name: foo +email: foo@github.com (verified) +two-factor auth: auth-and-writes +fullname: Foo Bar +homepage: https://github.com +freenode: foobar +twitter: https://twitter.com/npmjs +github: https://github.com/npm +created: 2015-02-26T01:26:37.384Z +updated: 2020-08-12T16:19:35.326Z ` exports[`test/lib/commands/profile.js TAP profile get no args no tfa enabled > should output expected profile values 1`] = ` -name: foo -email: foo@github.com (verified) -two-factor auth: disabled -fullname: Foo Bar -homepage: https://github.com -freenode: foobar -twitter: https://twitter.com/npmjs -github: https://github.com/npm -created: 2015-02-26T01:26:37.384Z -updated: 2020-08-12T16:19:35.326Z +name: foo +email: foo@github.com (verified) +two-factor auth: disabled +fullname: Foo Bar +homepage: https://github.com +freenode: foobar +twitter: https://twitter.com/npmjs +github: https://github.com/npm +created: 2015-02-26T01:26:37.384Z +updated: 2020-08-12T16:19:35.326Z ` exports[`test/lib/commands/profile.js TAP profile get no args profile has cidr_whitelist item > should output table with contents 1`] = ` -name: foo -email: foo@github.com (verified) -two-factor auth: auth-and-writes -fullname: Foo Bar -homepage: https://github.com -freenode: foobar -twitter: https://twitter.com/npmjs -github: https://github.com/npm -created: 2015-02-26T01:26:37.384Z -updated: 2020-08-12T16:19:35.326Z -cidr_whitelist: 192.168.1.1 +name: foo +email: foo@github.com (verified) +two-factor auth: auth-and-writes +fullname: Foo Bar +homepage: https://github.com +freenode: foobar +twitter: https://twitter.com/npmjs +github: https://github.com/npm +created: 2015-02-26T01:26:37.384Z +updated: 2020-08-12T16:19:35.326Z +cidr_whitelist: 192.168.1.1 ` exports[`test/lib/commands/profile.js TAP profile get no args unverified email > should output table with contents 1`] = ` -name: foo -email: foo@github.com(unverified) -two-factor auth: auth-and-writes -fullname: Foo Bar -homepage: https://github.com -freenode: foobar -twitter: https://twitter.com/npmjs -github: https://github.com/npm -created: 2015-02-26T01:26:37.384Z -updated: 2020-08-12T16:19:35.326Z +name: foo +email: foo@github.com(unverified) +two-factor auth: auth-and-writes +fullname: Foo Bar +homepage: https://github.com +freenode: foobar +twitter: https://twitter.com/npmjs +github: https://github.com/npm +created: 2015-02-26T01:26:37.384Z +updated: 2020-08-12T16:19:35.326Z ` exports[`test/lib/commands/profile.js TAP profile set writable key --parseable > should output parseable set key success msg 1`] = ` diff --git a/tap-snapshots/test/lib/utils/update-notifier.js.test.cjs b/tap-snapshots/test/lib/utils/update-notifier.js.test.cjs index 157390997d793..e5e9dd77569e0 100644 --- a/tap-snapshots/test/lib/utils/update-notifier.js.test.cjs +++ b/tap-snapshots/test/lib/utils/update-notifier.js.test.cjs @@ -5,7 +5,7 @@ * Make sure to inspect the output below. Do not ignore changes! */ 'use strict' -exports[`test/lib/utils/update-notifier.js TAP notification situations major to current > color 1`] = ` +exports[`test/lib/utils/update-notifier.js TAP notification situations 122.420.69 - color=always > must match snapshot 1`] = ` New major version of npm available! 122.420.69 -> 123.420.69 Changelog: https://github.com/npm/cli/releases/tag/v123.420.69 @@ -13,7 +13,7 @@ Run npm install -g npm@123.420.69 to update! ` -exports[`test/lib/utils/update-notifier.js TAP notification situations major to current > no color 1`] = ` +exports[`test/lib/utils/update-notifier.js TAP notification situations 122.420.69 - color=false > must match snapshot 1`] = ` New major version of npm available! 122.420.69 -> 123.420.69 Changelog: @@ -21,7 +21,7 @@ Run \`npm install -g npm@123.420.69\` to update! ` -exports[`test/lib/utils/update-notifier.js TAP notification situations minor to current > color 1`] = ` +exports[`test/lib/utils/update-notifier.js TAP notification situations 123.419.69 - color=always > must match snapshot 1`] = ` New minor version of npm available! 123.419.69 -> 123.420.69 Changelog: https://github.com/npm/cli/releases/tag/v123.420.69 @@ -29,7 +29,7 @@ Run npm install -g npm@123.420.69 to update! ` -exports[`test/lib/utils/update-notifier.js TAP notification situations minor to current > no color 1`] = ` +exports[`test/lib/utils/update-notifier.js TAP notification situations 123.419.69 - color=false > must match snapshot 1`] = ` New minor version of npm available! 123.419.69 -> 123.420.69 Changelog: @@ -37,66 +37,66 @@ Run \`npm install -g npm@123.420.69\` to update! ` -exports[`test/lib/utils/update-notifier.js TAP notification situations minor to next version > color 1`] = ` +exports[`test/lib/utils/update-notifier.js TAP notification situations 123.420.68 - color=always > must match snapshot 1`] = ` -New minor version of npm available! 123.420.70 -> 123.421.70 -Changelog: https://github.com/npm/cli/releases/tag/v123.421.70 -Run npm install -g npm@123.421.70 to update! +New patch version of npm available! 123.420.68 -> 123.420.69 +Changelog: https://github.com/npm/cli/releases/tag/v123.420.69 +Run npm install -g npm@123.420.69 to update! ` -exports[`test/lib/utils/update-notifier.js TAP notification situations minor to next version > no color 1`] = ` +exports[`test/lib/utils/update-notifier.js TAP notification situations 123.420.68 - color=false > must match snapshot 1`] = ` -New minor version of npm available! 123.420.70 -> 123.421.70 -Changelog: -Run \`npm install -g npm@123.421.70\` to update! +New patch version of npm available! 123.420.68 -> 123.420.69 +Changelog: +Run \`npm install -g npm@123.420.69\` to update! ` -exports[`test/lib/utils/update-notifier.js TAP notification situations new beta available > color 1`] = ` +exports[`test/lib/utils/update-notifier.js TAP notification situations 123.420.70 - color=always > must match snapshot 1`] = ` -New prerelease version of npm available! 124.0.0-beta.0 -> 124.0.0-beta.99999 -Changelog: https://github.com/npm/cli/releases/tag/v124.0.0-beta.99999 -Run npm install -g npm@124.0.0-beta.99999 to update! +New minor version of npm available! 123.420.70 -> 123.421.70 +Changelog: https://github.com/npm/cli/releases/tag/v123.421.70 +Run npm install -g npm@123.421.70 to update! ` -exports[`test/lib/utils/update-notifier.js TAP notification situations new beta available > no color 1`] = ` +exports[`test/lib/utils/update-notifier.js TAP notification situations 123.420.70 - color=false > must match snapshot 1`] = ` -New prerelease version of npm available! 124.0.0-beta.0 -> 124.0.0-beta.99999 -Changelog: -Run \`npm install -g npm@124.0.0-beta.99999\` to update! +New minor version of npm available! 123.420.70 -> 123.421.70 +Changelog: +Run \`npm install -g npm@123.421.70\` to update! ` -exports[`test/lib/utils/update-notifier.js TAP notification situations patch to current > color 1`] = ` +exports[`test/lib/utils/update-notifier.js TAP notification situations 123.421.69 - color=always > must match snapshot 1`] = ` -New patch version of npm available! 123.420.68 -> 123.420.69 -Changelog: https://github.com/npm/cli/releases/tag/v123.420.69 -Run npm install -g npm@123.420.69 to update! +New patch version of npm available! 123.421.69 -> 123.421.70 +Changelog: https://github.com/npm/cli/releases/tag/v123.421.70 +Run npm install -g npm@123.421.70 to update! ` -exports[`test/lib/utils/update-notifier.js TAP notification situations patch to current > no color 1`] = ` +exports[`test/lib/utils/update-notifier.js TAP notification situations 123.421.69 - color=false > must match snapshot 1`] = ` -New patch version of npm available! 123.420.68 -> 123.420.69 -Changelog: -Run \`npm install -g npm@123.420.69\` to update! +New patch version of npm available! 123.421.69 -> 123.421.70 +Changelog: +Run \`npm install -g npm@123.421.70\` to update! ` -exports[`test/lib/utils/update-notifier.js TAP notification situations patch to next version > color 1`] = ` +exports[`test/lib/utils/update-notifier.js TAP notification situations 124.0.0-beta.0 - color=always > must match snapshot 1`] = ` -New patch version of npm available! 123.421.69 -> 123.421.70 -Changelog: https://github.com/npm/cli/releases/tag/v123.421.70 -Run npm install -g npm@123.421.70 to update! +New prerelease version of npm available! 124.0.0-beta.0 -> 124.0.0-beta.99999 +Changelog: https://github.com/npm/cli/releases/tag/v124.0.0-beta.99999 +Run npm install -g npm@124.0.0-beta.99999 to update! ` -exports[`test/lib/utils/update-notifier.js TAP notification situations patch to next version > no color 1`] = ` +exports[`test/lib/utils/update-notifier.js TAP notification situations 124.0.0-beta.0 - color=false > must match snapshot 1`] = ` -New patch version of npm available! 123.421.69 -> 123.421.70 -Changelog: -Run \`npm install -g npm@123.421.70\` to update! +New prerelease version of npm available! 124.0.0-beta.0 -> 124.0.0-beta.99999 +Changelog: +Run \`npm install -g npm@124.0.0-beta.99999\` to update! ` diff --git a/test/lib/commands/explain.js b/test/lib/commands/explain.js index 3262dfdce87af..79c917a1cd452 100644 --- a/test/lib/commands/explain.js +++ b/test/lib/commands/explain.js @@ -7,7 +7,8 @@ const mockExplain = async (t, opts) => { mocks: { // keep the snapshots pared down a bit, since this has its own tests. '{LIB}/utils/explain-dep.js': { - explainNode: (expl, depth, color) => { + explainNode: (expl, depth, chalk) => { + const color = chalk.level !== 0 return `${expl.name}@${expl.version} depth=${depth} color=${color}` }, }, diff --git a/test/lib/commands/profile.js b/test/lib/commands/profile.js index 00ccf2607524a..1152acfdc5c46 100644 --- a/test/lib/commands/profile.js +++ b/test/lib/commands/profile.js @@ -1,7 +1,7 @@ const t = require('tap') const mockNpm = require('../../fixtures/mock-npm') -const mockProfile = async (t, { npmProfile, readUserInfo, qrcode, ...opts } = {}) => { +const mockProfile = async (t, { npmProfile, readUserInfo, qrcode, config, ...opts } = {}) => { const mocks = { 'npm-profile': npmProfile || { async get () {}, @@ -24,6 +24,10 @@ const mockProfile = async (t, { npmProfile, readUserInfo, qrcode, ...opts } = {} const mock = await mockNpm(t, { ...opts, + config: { + color: false, + ...config, + }, mocks: { ...mocks, ...opts.mocks, @@ -95,6 +99,16 @@ t.test('profile get no args', async t => { t.matchSnapshot(result(), 'should output all profile info as parseable result') }) + t.test('--color', async t => { + const { profile, result } = await mockProfile(t, { + npmProfile: defaultNpmProfile, + config: { color: 'always' }, + }) + + await profile.exec(['get']) + t.matchSnapshot(result(), 'should output all profile info with color result') + }) + t.test('no tfa enabled', async t => { const npmProfile = { async get () { diff --git a/test/lib/commands/view.js b/test/lib/commands/view.js index c6a4bf8fb79f4..51bc130df24e5 100644 --- a/test/lib/commands/view.js +++ b/test/lib/commands/view.js @@ -270,6 +270,10 @@ const loadMockNpm = async function (t, opts = {}) { }, }, ...opts, + config: { + color: 'always', + ...opts.config, + }, }) return mockNpm } @@ -374,7 +378,7 @@ t.test('package in cwd', async t => { }) t.test('specific field names', async t => { - const { view, outputs } = await loadMockNpm(t) + const { view, outputs } = await loadMockNpm(t, { config: { color: false } }) t.afterEach(() => outputs.length = 0) t.test('readme', async t => { diff --git a/test/lib/utils/display.js b/test/lib/utils/display.js index cfe0181e23e79..7a99dcb679c09 100644 --- a/test/lib/utils/display.js +++ b/test/lib/utils/display.js @@ -58,7 +58,7 @@ t.test('can log', async (t) => { display.log('warn', 'ERESOLVE', 'hello', { some: 'object' }) t.match(logs.warn, [['ERESOLVE', 'hello']]) - t.match(explains, [[{ some: 'object' }, false, 2]]) + t.match(explains, [[{ some: 'object' }, null, 2]]) }) t.test('handles log throwing', async (t) => { diff --git a/test/lib/utils/error-message.js b/test/lib/utils/error-message.js index 8a52b97f528e7..37b3bc6afeddc 100644 --- a/test/lib/utils/error-message.js +++ b/test/lib/utils/error-message.js @@ -414,11 +414,14 @@ t.test('explain ERESOLVE errors', async t => { errorMocks: { '{LIB}/utils/explain-eresolve.js': { report: (...args) => { - EXPLAIN_CALLED.push(args) + EXPLAIN_CALLED.push(...args) return { explanation: 'explanation', file: 'report' } }, }, }, + config: { + color: 'always', + }, }) const er = Object.assign(new Error('could not resolve'), { @@ -426,5 +429,8 @@ t.test('explain ERESOLVE errors', async t => { }) t.matchSnapshot(errorMessage(er)) - t.match(EXPLAIN_CALLED, [[er, false]]) + t.equal(EXPLAIN_CALLED.length, 3) + t.match(EXPLAIN_CALLED, [er, Function, Function]) + t.not(EXPLAIN_CALLED[1].level, 0, 'color chalk level is not 0') + t.equal(EXPLAIN_CALLED[2].level, 0, 'colorless chalk level is 0') }) diff --git a/test/lib/utils/explain-dep.js b/test/lib/utils/explain-dep.js index 514f28d125a0d..e5389fd26d796 100644 --- a/test/lib/utils/explain-dep.js +++ b/test/lib/utils/explain-dep.js @@ -1,9 +1,12 @@ const { resolve } = require('path') const t = require('tap') +const Chalk = require('chalk') const { explainNode, printNode } = require('../../../lib/utils/explain-dep.js') const { cleanCwd } = require('../../fixtures/clean-snapshot') const testdir = t.testdirName +const color = new Chalk.Instance({ level: Chalk.level }) +const noColor = new Chalk.Instance({ level: 0 }) t.cleanSnapshot = (str) => cleanCwd(str) @@ -250,10 +253,10 @@ cases.workspaces = { for (const [name, expl] of Object.entries(cases)) { t.test(name, t => { - t.matchSnapshot(printNode(expl, true), 'print color') - t.matchSnapshot(printNode(expl, false), 'print nocolor') - t.matchSnapshot(explainNode(expl, Infinity, true), 'explain color deep') - t.matchSnapshot(explainNode(expl, 2, false), 'explain nocolor shallow') + t.matchSnapshot(printNode(expl, color), 'print color') + t.matchSnapshot(printNode(expl, noColor), 'print nocolor') + t.matchSnapshot(explainNode(expl, Infinity, color), 'explain color deep') + t.matchSnapshot(explainNode(expl, 2, noColor), 'explain nocolor shallow') t.end() }) } @@ -261,6 +264,6 @@ for (const [name, expl] of Object.entries(cases)) { // make sure that we show the last one if it's the only one that would // hit the ... cases.manyDeps.dependents.pop() -t.matchSnapshot(explainNode(cases.manyDeps, 2, false), 'ellipses test one') +t.matchSnapshot(explainNode(cases.manyDeps, 2, noColor), 'ellipses test one') cases.manyDeps.dependents.pop() -t.matchSnapshot(explainNode(cases.manyDeps, 2, false), 'ellipses test two') +t.matchSnapshot(explainNode(cases.manyDeps, 2, noColor), 'ellipses test two') diff --git a/test/lib/utils/explain-eresolve.js b/test/lib/utils/explain-eresolve.js index 2c1fed77899e9..0f60556ef2ac9 100644 --- a/test/lib/utils/explain-eresolve.js +++ b/test/lib/utils/explain-eresolve.js @@ -1,9 +1,11 @@ const t = require('tap') -const { resolve } = require('path') +const Chalk = require('chalk') const { explain, report } = require('../../../lib/utils/explain-eresolve.js') const cases = require('../../fixtures/eresolve-explanations.js') -const { cleanDate } = require('../../fixtures/clean-snapshot.js') + +const color = new Chalk.Instance({ level: Chalk.level }) +const noColor = new Chalk.Instance({ level: 0 }) for (const [name, expl] of Object.entries(cases)) { // no sense storing the whole contents of each object in the snapshot @@ -11,22 +13,16 @@ for (const [name, expl] of Object.entries(cases)) { expl.toJSON = () => ({ name, json: true }) t.test(name, t => { - const dir = t.testdir() - const fileReport = resolve(dir, 'eresolve-report.txt') - const opts = { file: fileReport, date: new Date().toISOString() } - - t.cleanSnapshot = str => cleanDate(str.split(fileReport).join('${REPORT}')) - - const color = report(expl, true, opts) - t.matchSnapshot(color.explanation, 'report with color') - t.matchSnapshot(color.file, 'report from color') + const colorReport = report(expl, color, noColor) + t.matchSnapshot(colorReport.explanation, 'report with color') + t.matchSnapshot(colorReport.file, 'report from color') - const noColor = report(expl, false, opts) - t.matchSnapshot(noColor.explanation, 'report with no color') - t.equal(noColor.file, color.file, 'same report written for object') + const noColorReport = report(expl, noColor, noColor) + t.matchSnapshot(noColorReport.explanation, 'report with no color') + t.equal(noColorReport.file, colorReport.file, 'same report written for object') - t.matchSnapshot(explain(expl, true, 2), 'explain with color, depth of 2') - t.matchSnapshot(explain(expl, false, 6), 'explain with no color, depth of 6') + t.matchSnapshot(explain(expl, color, 2), 'explain with color, depth of 2') + t.matchSnapshot(explain(expl, noColor, 6), 'explain with no color, depth of 6') t.end() }) diff --git a/test/lib/utils/update-notifier.js b/test/lib/utils/update-notifier.js index e7830e6d9d66e..9c12433a2d117 100644 --- a/test/lib/utils/update-notifier.js +++ b/test/lib/utils/update-notifier.js @@ -1,10 +1,8 @@ const t = require('tap') +const { basename } = require('path') const tmock = require('../../fixtures/tmock') +const mockNpm = require('../../fixtures/mock-npm') -let ciMock = {} -const flatOptions = { global: false, cache: t.testdir() + '/_cacache' } - -const MANIFEST_REQUEST = [] const CURRENT_VERSION = '123.420.69' const CURRENT_MAJOR = '122.420.69' const CURRENT_MINOR = '123.419.69' @@ -15,238 +13,196 @@ const NEXT_PATCH = '123.421.69' const CURRENT_BETA = '124.0.0-beta.99999' const HAVE_BETA = '124.0.0-beta.0' -let PACOTE_ERROR = null -const pacote = { - manifest: async (spec, opts) => { - if (!spec.match(/^npm@/)) { - process.exit(1) - } - MANIFEST_REQUEST.push(spec) - if (PACOTE_ERROR) { - throw PACOTE_ERROR - } - - return { - version: - spec === 'npm@latest' - ? CURRENT_VERSION - : /-/.test(spec) - ? CURRENT_BETA - : NEXT_VERSION, - } - }, -} - -const defaultNpm = { - flatOptions, - version: CURRENT_VERSION, - config: { get: k => k !== 'global' }, - command: 'view', - argv: ['npm'], -} - -const { basename } = require('path') - -let STAT_ERROR = null -let STAT_MTIME = null -let WRITE_ERROR = null -const fs = { - ...require('fs'), - stat: (path, cb) => { - if (basename(path) !== '_update-notifier-last-checked') { - process.exit(1) - } - process.nextTick(() => cb(STAT_ERROR, { mtime: new Date(STAT_MTIME) })) - }, - writeFile: (path, content, cb) => { - if (content !== '') { - process.exit(1) - } - if (basename(path) !== '_update-notifier-last-checked') { - process.exit(1) - } - process.nextTick(() => cb(WRITE_ERROR)) - }, +const runUpdateNotifier = async (t, { + STAT_ERROR, + WRITE_ERROR, + PACOTE_ERROR, + STAT_MTIME = 0, + mocks: _mocks = {}, + command = 'view', + version = CURRENT_VERSION, + argv = [], + ...config +} = {}) => { + const mockFs = { + ...require('fs/promises'), + stat: async (path) => { + if (basename(path) !== '_update-notifier-last-checked') { + t.fail('no stat allowed for non upate notifier files') + } + if (STAT_ERROR) { + throw STAT_ERROR + } + return { mtime: new Date(STAT_MTIME) } + }, + writeFile: async (path, content) => { + if (content !== '') { + t.fail('no write file content allowed') + } + if (basename(path) !== '_update-notifier-last-checked') { + t.fail('no writefile allowed for non upate notifier files') + } + if (WRITE_ERROR) { + throw WRITE_ERROR + } + }, + } + + const MANIFEST_REQUEST = [] + const mockPacote = { + manifest: async (spec) => { + if (!spec.match(/^npm@/)) { + t.fail('no pacote manifest allowed for non npm packages') + } + MANIFEST_REQUEST.push(spec) + if (PACOTE_ERROR) { + throw PACOTE_ERROR + } + const manifestV = spec === 'npm@latest' ? CURRENT_VERSION + : /-/.test(spec) ? CURRENT_BETA : NEXT_VERSION + return { version: manifestV } + }, + } + + const mocks = { + pacote: mockPacote, + 'fs/promises': mockFs, + '{ROOT}/package.json': { version }, + 'ci-info': { isCI: false, name: null }, + ..._mocks, + } + + const mock = await mockNpm(t, { + command, + mocks, + config, + argv, + }) + const updateNotifier = tmock(t, '{LIB}/utils/update-notifier.js', mocks) + + const result = await updateNotifier(mock.npm) + + return { + result, + MANIFEST_REQUEST, + } } -t.afterEach(() => { - MANIFEST_REQUEST.length = 0 - STAT_ERROR = null - PACOTE_ERROR = null - STAT_MTIME = null - WRITE_ERROR = null +t.test('does not notify by default', async t => { + const { result, MANIFEST_REQUEST } = await runUpdateNotifier(t) + t.not(result) + t.equal(MANIFEST_REQUEST.length, 1) }) -const runUpdateNotifier = async ({ color = true, ...npmOptions } = {}) => { - const _npm = { ...defaultNpm, ...npmOptions, logColor: color } - return tmock(t, '{LIB}/utils/update-notifier.js', { - 'ci-info': ciMock, - pacote, - fs, - })(_npm) -} - t.test('situations in which we do not notify', t => { t.test('nothing to do if notifier disabled', async t => { - t.equal( - await runUpdateNotifier({ - config: { get: k => k !== 'update-notifier' }, - }), - null - ) + const { result, MANIFEST_REQUEST } = await runUpdateNotifier(t, { + 'update-notifier': false, + }) + t.equal(result, null) t.strictSame(MANIFEST_REQUEST, [], 'no requests for manifests') }) t.test('do not suggest update if already updating', async t => { - t.equal( - await runUpdateNotifier({ - flatOptions: { ...flatOptions, global: true }, - command: 'install', - argv: ['npm'], - }), - null - ) + const { result, MANIFEST_REQUEST } = await runUpdateNotifier(t, { + command: 'install', + argv: ['npm'], + global: true, + }) + t.equal(result, null) t.strictSame(MANIFEST_REQUEST, [], 'no requests for manifests') }) t.test('do not suggest update if already updating with spec', async t => { - t.equal( - await runUpdateNotifier({ - flatOptions: { ...flatOptions, global: true }, - command: 'install', - argv: ['npm@latest'], - }), - null - ) + const { result, MANIFEST_REQUEST } = await runUpdateNotifier(t, { + command: 'install', + argv: ['npm@latest'], + global: true, + }) + t.equal(result, null) t.strictSame(MANIFEST_REQUEST, [], 'no requests for manifests') }) t.test('do not update if same as latest', async t => { - t.equal(await runUpdateNotifier(), null) + const { result, MANIFEST_REQUEST } = await runUpdateNotifier(t) + t.equal(result, null) t.strictSame(MANIFEST_REQUEST, ['npm@latest'], 'requested latest version') }) t.test('check if stat errors (here for coverage)', async t => { - STAT_ERROR = new Error('blorg') - t.equal(await runUpdateNotifier(), null) + const STAT_ERROR = new Error('blorg') + const { result, MANIFEST_REQUEST } = await runUpdateNotifier(t, { STAT_ERROR }) + t.equal(result, null) t.strictSame(MANIFEST_REQUEST, ['npm@latest'], 'requested latest version') }) t.test('ok if write errors (here for coverage)', async t => { - WRITE_ERROR = new Error('grolb') - t.equal(await runUpdateNotifier(), null) + const WRITE_ERROR = new Error('grolb') + const { result, MANIFEST_REQUEST } = await runUpdateNotifier(t, { WRITE_ERROR }) + t.equal(result, null) t.strictSame(MANIFEST_REQUEST, ['npm@latest'], 'requested latest version') }) t.test('ignore pacote failures (here for coverage)', async t => { - PACOTE_ERROR = new Error('pah-KO-tchay') - t.equal(await runUpdateNotifier(), null) + const PACOTE_ERROR = new Error('pah-KO-tchay') + const { result, MANIFEST_REQUEST } = await runUpdateNotifier(t, { PACOTE_ERROR }) + t.equal(result, null) t.strictSame(MANIFEST_REQUEST, ['npm@latest'], 'requested latest version') }) t.test('do not update if newer than latest, but same as next', async t => { - t.equal(await runUpdateNotifier({ version: NEXT_VERSION }), null) + const { result, MANIFEST_REQUEST } = await runUpdateNotifier(t, { version: NEXT_VERSION }) + t.equal(result, null) const reqs = ['npm@latest', `npm@^${NEXT_VERSION}`] t.strictSame(MANIFEST_REQUEST, reqs, 'requested latest and next versions') }) t.test('do not update if on the latest beta', async t => { - t.equal(await runUpdateNotifier({ version: CURRENT_BETA }), null) + const { result, MANIFEST_REQUEST } = await runUpdateNotifier(t, { version: CURRENT_BETA }) + t.equal(result, null) const reqs = [`npm@^${CURRENT_BETA}`] t.strictSame(MANIFEST_REQUEST, reqs, 'requested latest and next versions') }) t.test('do not update in CI', async t => { - t.teardown(() => { - ciMock = {} - }) - ciMock = { isCI: true, name: 'something' } - t.equal(await runUpdateNotifier(), null) + const { result, MANIFEST_REQUEST } = await runUpdateNotifier(t, { mocks: { + 'ci-info': { isCI: true, name: 'something' }, + } }) + t.equal(result, null) t.strictSame(MANIFEST_REQUEST, [], 'no requests for manifests') }) t.test('only check weekly for GA releases', async t => { // One week (plus five minutes to account for test environment fuzziness) - STAT_MTIME = Date.now() - 1000 * 60 * 60 * 24 * 7 + 1000 * 60 * 5 - t.equal(await runUpdateNotifier(), null) + const STAT_MTIME = Date.now() - 1000 * 60 * 60 * 24 * 7 + 1000 * 60 * 5 + const { result, MANIFEST_REQUEST } = await runUpdateNotifier(t, { STAT_MTIME }) + t.equal(result, null) t.strictSame(MANIFEST_REQUEST, [], 'no requests for manifests') }) t.test('only check daily for betas', async t => { // One day (plus five minutes to account for test environment fuzziness) - STAT_MTIME = Date.now() - 1000 * 60 * 60 * 24 + 1000 * 60 * 5 - t.equal(await runUpdateNotifier({ version: HAVE_BETA }), null) - t.strictSame(MANIFEST_REQUEST, [], 'no requests for manifests') + const STAT_MTIME = Date.now() - 1000 * 60 * 60 * 24 + 1000 * 60 * 5 + const res = await runUpdateNotifier(t, { STAT_MTIME, version: HAVE_BETA }) + t.equal(res.result, null) + t.strictSame(res.MANIFEST_REQUEST, [], 'no requests for manifests') }) t.end() }) -t.test('notification situations', t => { - t.test('new beta available', async t => { - const version = HAVE_BETA - t.matchSnapshot(await runUpdateNotifier({ version }), 'color') - t.matchSnapshot( - await runUpdateNotifier({ version, color: false }), - 'no color' - ) - t.strictSame(MANIFEST_REQUEST, [`npm@^${version}`, `npm@^${version}`]) - }) - - t.test('patch to next version', async t => { - const version = NEXT_PATCH - t.matchSnapshot(await runUpdateNotifier({ version }), 'color') - t.matchSnapshot( - await runUpdateNotifier({ version, color: false }), - 'no color' - ) - t.strictSame(MANIFEST_REQUEST, [ - 'npm@latest', - `npm@^${version}`, - 'npm@latest', - `npm@^${version}`, - ]) - }) - - t.test('minor to next version', async t => { - const version = NEXT_MINOR - t.matchSnapshot(await runUpdateNotifier({ version }), 'color') - t.matchSnapshot( - await runUpdateNotifier({ version, color: false }), - 'no color' - ) - t.strictSame(MANIFEST_REQUEST, [ - 'npm@latest', - `npm@^${version}`, - 'npm@latest', - `npm@^${version}`, - ]) - }) - - t.test('patch to current', async t => { - const version = CURRENT_PATCH - t.matchSnapshot(await runUpdateNotifier({ version }), 'color') - t.matchSnapshot( - await runUpdateNotifier({ version, color: false }), - 'no color' - ) - t.strictSame(MANIFEST_REQUEST, ['npm@latest', 'npm@latest']) - }) - - t.test('minor to current', async t => { - const version = CURRENT_MINOR - t.matchSnapshot(await runUpdateNotifier({ version }), 'color') - t.matchSnapshot( - await runUpdateNotifier({ version, color: false }), - 'no color' - ) - t.strictSame(MANIFEST_REQUEST, ['npm@latest', 'npm@latest']) - }) - - t.test('major to current', async t => { - const version = CURRENT_MAJOR - t.matchSnapshot(await runUpdateNotifier({ version }), 'color') - t.matchSnapshot( - await runUpdateNotifier({ version, color: false }), - 'no color' - ) - t.strictSame(MANIFEST_REQUEST, ['npm@latest', 'npm@latest']) - }) - - t.end() +t.test('notification situations', async t => { + const cases = { + [HAVE_BETA]: [`^{V}`], + [NEXT_PATCH]: [`latest`, `^{V}`], + [NEXT_MINOR]: [`latest`, `^{V}`], + [CURRENT_PATCH]: ['latest'], + [CURRENT_MINOR]: ['latest'], + [CURRENT_MAJOR]: ['latest'], + } + + for (const [version, reqs] of Object.entries(cases)) { + for (const color of [false, 'always']) { + await t.test(`${version} - color=${color}`, async t => { + const { result, MANIFEST_REQUEST } = await runUpdateNotifier(t, { version, color }) + t.matchSnapshot(result) + t.strictSame(MANIFEST_REQUEST, reqs.map(r => `npm@${r.replace('{V}', version)}`)) + }) + } + } })