Skip to content

Commit

Permalink
fix: add first param titles to logs where missing (#7533)
Browse files Browse the repository at this point in the history
The first argument to all `log.method()` calls gets formatted
differently with a color. So calls to these should always be a short
descriptive title or an empty string.
  • Loading branch information
lukekarrys authored May 15, 2024
1 parent badeac2 commit 12f103c
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 30 deletions.
2 changes: 1 addition & 1 deletion lib/commands/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class Audit extends ArboristWorkspaceCmd {
)
}

log.verbose('loading installed dependencies')
log.verbose('audit', 'loading installed dependencies')
const Arborist = require('@npmcli/arborist')
const opts = {
...this.npm.flatOptions,
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class Cache extends BaseCommand {
try {
entry = await cacache.get(cachePath, key)
} catch (err) {
log.warn(`Not Found: ${key}`)
log.warn('cache', `Not Found: ${key}`)
break
}
output.standard(`Deleted: ${key}`)
Expand Down
16 changes: 8 additions & 8 deletions tap-snapshots/test/lib/cli/exit-handler.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
'use strict'
exports[`test/lib/cli/exit-handler.js TAP handles unknown error with logs and debug file > debug file contents 1`] = `
XX timing npm:load:whichnode Completed in {TIME}ms
XX silly config:load:file:{CWD}/npmrc
XX silly config:load:file:{CWD}/prefix/.npmrc
XX silly config:load:file:{CWD}/home/.npmrc
XX silly config:load:file:{CWD}/global/etc/npmrc
XX silly config load:file:{CWD}/npmrc
XX silly config load:file:{CWD}/prefix/.npmrc
XX silly config load:file:{CWD}/home/.npmrc
XX silly config load:file:{CWD}/global/etc/npmrc
XX timing npm:load:configload Completed in {TIME}ms
XX timing npm:load:mkdirpcache Completed in {TIME}ms
XX timing npm:load:mkdirplogs Completed in {TIME}ms
Expand All @@ -37,10 +37,10 @@ XX error A complete log of this run can be found in: {CWD}/cache/_logs/{DATE}-de

exports[`test/lib/cli/exit-handler.js TAP handles unknown error with logs and debug file > logs 1`] = `
timing npm:load:whichnode Completed in {TIME}ms
silly config:load:file:{CWD}/npmrc
silly config:load:file:{CWD}/prefix/.npmrc
silly config:load:file:{CWD}/home/.npmrc
silly config:load:file:{CWD}/global/etc/npmrc
silly config load:file:{CWD}/npmrc
silly config load:file:{CWD}/prefix/.npmrc
silly config load:file:{CWD}/home/.npmrc
silly config load:file:{CWD}/global/etc/npmrc
timing npm:load:configload Completed in {TIME}ms
timing npm:load:mkdirpcache Completed in {TIME}ms
timing npm:load:mkdirplogs Completed in {TIME}ms
Expand Down
22 changes: 11 additions & 11 deletions tap-snapshots/test/lib/commands/shrinkwrap.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with hidden lockfile ancient upgrad
"created a lockfile as npm-shrinkwrap.json with version 3"
],
"warn": [
"Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
"shrinkwrap Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
]
}
`
Expand Down Expand Up @@ -98,7 +98,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with hidden lockfile existing downg
"created a lockfile as npm-shrinkwrap.json with version 1"
],
"warn": [
"Converting lock file (npm-shrinkwrap.json) from v2 -> v1"
"shrinkwrap Converting lock file (npm-shrinkwrap.json) from v2 -> v1"
]
}
`
Expand All @@ -125,7 +125,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with hidden lockfile existing upgra
"created a lockfile as npm-shrinkwrap.json with version 3"
],
"warn": [
"Converting lock file (npm-shrinkwrap.json) from v2 -> v3"
"shrinkwrap Converting lock file (npm-shrinkwrap.json) from v2 -> v3"
]
}
`
Expand Down Expand Up @@ -188,7 +188,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with npm-shrinkwrap.json ancient >
"npm-shrinkwrap.json updated to version 3"
],
"warn": [
"Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
"shrinkwrap Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
]
}
`
Expand Down Expand Up @@ -217,7 +217,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with npm-shrinkwrap.json ancient up
"npm-shrinkwrap.json updated to version 3"
],
"warn": [
"Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
"shrinkwrap Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
]
}
`
Expand Down Expand Up @@ -266,7 +266,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with npm-shrinkwrap.json existing d
"npm-shrinkwrap.json updated to version 1"
],
"warn": [
"Converting lock file (npm-shrinkwrap.json) from v2 -> v1"
"shrinkwrap Converting lock file (npm-shrinkwrap.json) from v2 -> v1"
]
}
`
Expand Down Expand Up @@ -295,7 +295,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with npm-shrinkwrap.json existing u
"npm-shrinkwrap.json updated to version 3"
],
"warn": [
"Converting lock file (npm-shrinkwrap.json) from v2 -> v3"
"shrinkwrap Converting lock file (npm-shrinkwrap.json) from v2 -> v3"
]
}
`
Expand All @@ -322,7 +322,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with package-lock.json ancient > mu
"package-lock.json has been renamed to npm-shrinkwrap.json and updated to version 3"
],
"warn": [
"Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
"shrinkwrap Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
]
}
`
Expand Down Expand Up @@ -351,7 +351,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with package-lock.json ancient upgr
"package-lock.json has been renamed to npm-shrinkwrap.json and updated to version 3"
],
"warn": [
"Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
"shrinkwrap Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
]
}
`
Expand Down Expand Up @@ -400,7 +400,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with package-lock.json existing dow
"package-lock.json has been renamed to npm-shrinkwrap.json and updated to version 1"
],
"warn": [
"Converting lock file (npm-shrinkwrap.json) from v2 -> v1"
"shrinkwrap Converting lock file (npm-shrinkwrap.json) from v2 -> v1"
]
}
`
Expand Down Expand Up @@ -429,7 +429,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with package-lock.json existing upg
"package-lock.json has been renamed to npm-shrinkwrap.json and updated to version 3"
],
"warn": [
"Converting lock file (npm-shrinkwrap.json) from v2 -> v3"
"shrinkwrap Converting lock file (npm-shrinkwrap.json) from v2 -> v3"
]
}
`
3 changes: 2 additions & 1 deletion workspaces/arborist/lib/shrinkwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -1153,7 +1153,8 @@ class Shrinkwrap {
&& this.originalLockfileVersion !== this.lockfileVersion
) {
log.warn(
`Converting lock file (${relative(process.cwd(), this.filename)}) from v${this.originalLockfileVersion} -> v${this.lockfileVersion}`
'shrinkwrap',
`Converting lock file (${relative(process.cwd(), this.filename)}) from v${this.originalLockfileVersion} -> v${this.lockfileVersion}`
)
}

Expand Down
6 changes: 3 additions & 3 deletions workspaces/config/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,7 @@ class Config {

async #loadFile (file, type) {
// only catch the error from readFile, not from the loadObject call
log.silly(`config:load:file:${file}`)
log.silly('config', `load:file:${file}`)
await readFile(file, 'utf8').then(
data => {
const parsedConfig = ini.parse(data)
Expand Down Expand Up @@ -684,15 +684,15 @@ class Config {
if (w === this.localPrefix) {
// see if there's a .npmrc file in the workspace, if so log a warning
if (await fileExists(this.localPrefix, '.npmrc')) {
log.warn(`ignoring workspace config at ${this.localPrefix}/.npmrc`)
log.warn('config', `ignoring workspace config at ${this.localPrefix}/.npmrc`)
}

// set the workspace in the default layer, which allows it to be overridden easily
const { data } = this.data.get('default')
data.workspace = [this.localPrefix]
this.localPrefix = p
this.localPackage = hasPackageJson
log.info(`found workspace root at ${this.localPrefix}`)
log.info('config', `found workspace root at ${this.localPrefix}`)
// we found a root, so we return now
return
}
Expand Down
15 changes: 10 additions & 5 deletions workspaces/config/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1185,7 +1185,8 @@ t.test('workspaces', async (t) => {
t.same(config.get('workspace'), [join(path, 'workspaces', 'one')], 'set the workspace')
const info = logs.filter(l => l[0] === 'info')
t.equal(info.length, 1, 'got one log message')
t.match(info[0], ['info', /^found workspace root at/], 'logged info about workspace root')
t.match(info[0],
['info', 'config', /^found workspace root at/], 'logged info about workspace root')
})

t.test('finds other workspace parent', async (t) => {
Expand All @@ -1207,7 +1208,8 @@ t.test('workspaces', async (t) => {
t.same(config.get('workspace'), ['../two'], 'kept the specified workspace')
const info = logs.filter(l => l[0] === 'info')
t.equal(info.length, 1, 'got one log message')
t.match(info[0], ['info', /^found workspace root at/], 'logged info about workspace root')
t.match(info[0],
['info', 'config', /^found workspace root at/], 'logged info about workspace root')
})

t.test('warns when workspace has .npmrc', async (t) => {
Expand All @@ -1229,8 +1231,10 @@ t.test('workspaces', async (t) => {
t.same(config.get('workspace'), [join(path, 'workspaces', 'three')], 'kept the workspace')
const filtered = logs.filter(l => l[0] === 'info' || l[0] === 'warn')
t.equal(filtered.length, 2, 'got two log messages')
t.match(filtered[0], ['warn', /^ignoring workspace config/], 'warned about ignored config')
t.match(filtered[1], ['info', /^found workspace root at/], 'logged info about workspace root')
t.match(filtered[0],
['warn', 'config', /^ignoring workspace config/], 'warned about ignored config')
t.match(filtered[1],
['info', 'config', /^found workspace root at/], 'logged info about workspace root')
})

t.test('prefix skips auto detect', async (t) => {
Expand Down Expand Up @@ -1364,7 +1368,8 @@ t.test('workspaces', async (t) => {
t.same(config.get('workspace'), [join(path, 'workspaces', 'one')], 'set the workspace')
const filtered = logs.filter(l => l[0] !== 'silly')
t.equal(filtered.length, 1, 'got one log message')
t.match(filtered[0], ['info', /^found workspace root at/], 'logged info about workspace root')
t.match(filtered[0],
['info', 'config', /^found workspace root at/], 'logged info about workspace root')
})
})

Expand Down

0 comments on commit 12f103c

Please sign in to comment.