From 1b9338554fc006954fae54c25c33e64e26ae997e Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 1 Feb 2022 14:44:27 -0800 Subject: [PATCH] fix(log): pass in logger to external modules Most of these module use npm-registry-fetch under the hood, which will log things like the `npm-notice` header if seen. Currently we aren't passing in a logger to them, which means that log message is never seen, among any other logged messages those modules may need to make. I added tests where I could. Some tests were in a state where the entire libnpm* module was an empty mocked function, so asserting that it got passed a `log` attribute was onerous. --- lib/commands/access.js | 6 +++- lib/commands/deprecate.js | 3 ++ lib/commands/diff.js | 2 ++ lib/commands/dist-tag.js | 5 ++- lib/commands/hook.js | 6 +++- lib/commands/logout.js | 1 + lib/commands/owner.js | 6 +++- lib/commands/ping.js | 2 +- lib/commands/profile.js | 8 ++--- lib/commands/publish.js | 2 +- lib/commands/star.js | 4 ++- lib/commands/team.js | 3 +- lib/commands/token.js | 2 +- lib/commands/unpublish.js | 2 +- lib/commands/whoami.js | 3 +- test/lib/commands/access.js | 5 +-- test/lib/commands/deprecate.js | 10 ++++++ test/lib/commands/diff.js | 3 +- test/lib/commands/dist-tag.js | 14 +++++++- test/lib/commands/hook.js | 63 ++++++++++++++++++++++------------ test/lib/commands/logout.js | 15 ++++---- test/lib/commands/owner.js | 26 ++++++++++---- test/lib/commands/ping.js | 9 +++-- test/lib/commands/publish.js | 20 +++++++---- test/lib/commands/star.js | 44 ++++++++++++++---------- 25 files changed, 183 insertions(+), 81 deletions(-) diff --git a/lib/commands/access.js b/lib/commands/access.js index f61c97d6f90de..7d3930dacbce9 100644 --- a/lib/commands/access.js +++ b/lib/commands/access.js @@ -3,6 +3,7 @@ const path = require('path') const libaccess = require('libnpmaccess') const readPackageJson = require('read-package-json-fast') +const log = require('../utils/log-shim.js') const otplease = require('../utils/otplease.js') const getIdentity = require('../utils/get-identity.js') const BaseCommand = require('../base-command.js') @@ -76,7 +77,10 @@ class Access extends BaseCommand { throw this.usageError(`${cmd} is not a recognized subcommand.`) } - return this[cmd](args, this.npm.flatOptions) + return this[cmd](args, { + ...this.npm.flatOptions, + log, + }) } public ([pkg], opts) { diff --git a/lib/commands/deprecate.js b/lib/commands/deprecate.js index 839e974caf09b..5505b9bf77cf7 100644 --- a/lib/commands/deprecate.js +++ b/lib/commands/deprecate.js @@ -1,4 +1,5 @@ const fetch = require('npm-registry-fetch') +const log = require('../utils/log-shim.js') const otplease = require('../utils/otplease.js') const npa = require('npm-package-arg') const semver = require('semver') @@ -50,6 +51,7 @@ class Deprecate extends BaseCommand { ...this.npm.flatOptions, spec: p, query: { write: true }, + log, }) Object.keys(packument.versions) @@ -64,6 +66,7 @@ class Deprecate extends BaseCommand { method: 'PUT', body: packument, ignoreBody: true, + log, })) } } diff --git a/lib/commands/diff.js b/lib/commands/diff.js index d737a58dc43d8..b3855aa08f3f1 100644 --- a/lib/commands/diff.js +++ b/lib/commands/diff.js @@ -61,6 +61,7 @@ class Diff extends BaseCommand { ...this.npm.flatOptions, diffFiles: args, where: this.top, + log, }) return this.npm.output(res) } @@ -193,6 +194,7 @@ class Diff extends BaseCommand { const packument = await pacote.packument(spec, { ...this.npm.flatOptions, preferOnline: true, + log, }) bSpec = pickManifest( packument, diff --git a/lib/commands/dist-tag.js b/lib/commands/dist-tag.js index bf2dffe912030..e2b013206d3aa 100644 --- a/lib/commands/dist-tag.js +++ b/lib/commands/dist-tag.js @@ -29,7 +29,10 @@ class DistTag extends BaseCommand { } async exec ([cmdName, pkg, tag]) { - const opts = this.npm.flatOptions + const opts = { + ...this.npm.flatOptions, + log, + } if (['add', 'a', 'set', 's'].includes(cmdName)) { return this.add(pkg, tag, opts) diff --git a/lib/commands/hook.js b/lib/commands/hook.js index 2881f044e8e28..c99a99585897a 100644 --- a/lib/commands/hook.js +++ b/lib/commands/hook.js @@ -2,6 +2,7 @@ const hookApi = require('libnpmhook') const otplease = require('../utils/otplease.js') const relativeDate = require('tiny-relative-date') const Table = require('cli-table3') +const log = require('../utils/log-shim.js') const BaseCommand = require('../base-command.js') class Hook extends BaseCommand { @@ -20,7 +21,10 @@ class Hook extends BaseCommand { ] async exec (args) { - return otplease(this.npm.flatOptions, (opts) => { + return otplease({ + ...this.npm.flatOptions, + log, + }, (opts) => { switch (args[0]) { case 'add': return this.add(args[1], args[2], args[3], opts) diff --git a/lib/commands/logout.js b/lib/commands/logout.js index aea5e93652b0e..34fbace583562 100644 --- a/lib/commands/logout.js +++ b/lib/commands/logout.js @@ -25,6 +25,7 @@ class Logout extends BaseCommand { ...this.npm.flatOptions, method: 'DELETE', ignoreBody: true, + log, }) } else if (auth.isBasicAuth) { log.verbose('logout', `clearing user credentials for ${reg}`) diff --git a/lib/commands/owner.js b/lib/commands/owner.js index c027ad6464557..7b76b7be42ec6 100644 --- a/lib/commands/owner.js +++ b/lib/commands/owner.js @@ -57,7 +57,10 @@ class Owner extends BaseCommand { } async exec ([action, ...args]) { - const opts = this.npm.flatOptions + const opts = { + ...this.npm.flatOptions, + log, + } switch (action) { case 'ls': case 'list': @@ -195,6 +198,7 @@ class Owner extends BaseCommand { method: 'PUT', body, spec, + log, }) }) diff --git a/lib/commands/ping.js b/lib/commands/ping.js index 993e029d45651..27089be4e1714 100644 --- a/lib/commands/ping.js +++ b/lib/commands/ping.js @@ -10,7 +10,7 @@ class Ping extends BaseCommand { async exec (args) { log.notice('PING', this.npm.config.get('registry')) const start = Date.now() - const details = await pingUtil(this.npm.flatOptions) + const details = await pingUtil({ ...this.npm.flatOptions, log }) const time = Date.now() - start log.notice('PONG', `${time}ms`) if (this.npm.config.get('json')) { diff --git a/lib/commands/profile.js b/lib/commands/profile.js index 1250ed7d1c756..9786e9ba4d4cf 100644 --- a/lib/commands/profile.js +++ b/lib/commands/profile.js @@ -108,7 +108,7 @@ class Profile extends BaseCommand { async get (args) { const tfa = 'two-factor auth' const info = await pulseTillDone.withPromise( - npmProfile.get(this.npm.flatOptions) + npmProfile.get({ ...this.npm.flatOptions, log }) ) if (!info.cidr_whitelist) { @@ -170,7 +170,7 @@ class Profile extends BaseCommand { } async set (args) { - const conf = this.npm.flatOptions + const conf = { ...this.npm.flatOptions, log } const prop = (args[0] || '').toLowerCase().trim() let value = args.length > 1 ? args.slice(1).join(' ') : null @@ -285,7 +285,7 @@ class Profile extends BaseCommand { if (auth.basic) { log.info('profile', 'Updating authentication to bearer token') const result = await npmProfile.createToken( - auth.basic.password, false, [], this.npm.flatOptions + auth.basic.password, false, [], { ...this.npm.flatOptions, log } ) if (!result.token) { @@ -309,7 +309,7 @@ class Profile extends BaseCommand { log.info('profile', 'Determine if tfa is pending') const userInfo = await pulseTillDone.withPromise( - npmProfile.get(this.npm.flatOptions) + npmProfile.get({ ...this.npm.flatOptions, log }) ) const conf = { ...this.npm.flatOptions } diff --git a/lib/commands/publish.js b/lib/commands/publish.js index b8209374925fe..339c80daad65e 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -61,7 +61,7 @@ class Publish extends BaseCommand { throw new Error('Tag name must not be a valid SemVer range: ' + defaultTag.trim()) } - const opts = { ...this.npm.flatOptions } + const opts = { ...this.npm.flatOptions, log } // you can publish name@version, ./foo.tgz, etc. // even though the default is the 'file:.' cwd. diff --git a/lib/commands/star.js b/lib/commands/star.js index ec11605899437..4974c39883e43 100644 --- a/lib/commands/star.js +++ b/lib/commands/star.js @@ -29,12 +29,13 @@ class Star extends BaseCommand { const pkgs = args.map(npa) for (const pkg of pkgs) { const [username, fullData] = await Promise.all([ - getIdentity(this.npm, this.npm.flatOptions), + getIdentity(this.npm, { ...this.npm.flatOptions, log }), fetch.json(pkg.escapedName, { ...this.npm.flatOptions, spec: pkg, query: { write: true }, preferOnline: true, + log, }), ]) @@ -63,6 +64,7 @@ class Star extends BaseCommand { spec: pkg, method: 'PUT', body, + log, }) this.npm.output(show + ' ' + pkg.name) diff --git a/lib/commands/team.js b/lib/commands/team.js index 3d2fff0f069d7..e3fb9b83c5a27 100644 --- a/lib/commands/team.js +++ b/lib/commands/team.js @@ -1,6 +1,7 @@ const columns = require('cli-columns') const libteam = require('libnpmteam') +const log = require('../utils/log-shim.js') const otplease = require('../utils/otplease.js') const BaseCommand = require('../base-command.js') @@ -42,7 +43,7 @@ class Team extends BaseCommand { // XXX: "description" option to libnpmteam is used as a description of the // team, but in npm's options, this is a boolean meaning "show the // description in npm search output". Hence its being set to null here. - await otplease(this.npm.flatOptions, opts => { + await otplease({ ...this.npm.flatOptions, log }, opts => { entity = entity.replace(/^@/, '') switch (cmd) { case 'create': return this.create(entity, opts) diff --git a/lib/commands/token.js b/lib/commands/token.js index df80f1afec44e..cfb5ab666565a 100644 --- a/lib/commands/token.js +++ b/lib/commands/token.js @@ -168,7 +168,7 @@ class Token extends BaseCommand { } config () { - const conf = { ...this.npm.flatOptions } + const conf = { ...this.npm.flatOptions, log } const creds = this.npm.config.getCredentialsByURI(conf.registry) if (creds.token) { conf.auth = { token: creds.token } diff --git a/lib/commands/unpublish.js b/lib/commands/unpublish.js index 85c366381cc7a..42c9c1db963e1 100644 --- a/lib/commands/unpublish.js +++ b/lib/commands/unpublish.js @@ -32,7 +32,7 @@ class Unpublish extends BaseCommand { return [] } - const opts = this.npm.flatOptions + const opts = { ...this.npm.flatOptions, log } const username = await getIdentity(this.npm, { ...opts }).catch(() => null) if (!username) { return [] diff --git a/lib/commands/whoami.js b/lib/commands/whoami.js index dbf32c0e73740..07ebe2e244753 100644 --- a/lib/commands/whoami.js +++ b/lib/commands/whoami.js @@ -1,4 +1,5 @@ const getIdentity = require('../utils/get-identity.js') +const log = require('../utils/log-shim.js') const BaseCommand = require('../base-command.js') class Whoami extends BaseCommand { @@ -7,7 +8,7 @@ class Whoami extends BaseCommand { static params = ['registry'] async exec (args) { - const username = await getIdentity(this.npm, this.npm.flatOptions) + const username = await getIdentity(this.npm, { ...this.npm.flatOptions, log }) this.npm.output( this.npm.config.get('json') ? JSON.stringify(username) : username ) diff --git a/test/lib/commands/access.js b/test/lib/commands/access.js index 298897e4f5ffc..c4e6f3167aa01 100644 --- a/test/lib/commands/access.js +++ b/test/lib/commands/access.js @@ -75,12 +75,13 @@ t.test('access public on unscoped package', async t => { }) t.test('access public on scoped package', async t => { - t.plan(2) + t.plan(3) const name = '@scoped/npm-access-public-pkg' const { npm } = await loadMockNpm(t, { mocks: { libnpmaccess: { - public: (pkg, { registry }) => { + public: (pkg, { registry, log }) => { + t.ok(log, 'should pass a logger') t.equal(pkg, name, 'should use pkg name ref') t.equal( registry, diff --git a/test/lib/commands/deprecate.js b/test/lib/commands/deprecate.js index 02256d08edb84..aa158cca3a1a1 100644 --- a/test/lib/commands/deprecate.js +++ b/test/lib/commands/deprecate.js @@ -2,12 +2,15 @@ const t = require('tap') let getIdentityImpl = () => 'someperson' let npmFetchBody = null +let npmFetchLog = null const npmFetch = async (uri, opts) => { npmFetchBody = opts.body + npmFetchLog = opts.log } npmFetch.json = async (uri, opts) => { + npmFetchLog = opts.log return { versions: { '1.0.0': {}, @@ -82,7 +85,12 @@ t.test('invalid semver range', async t => { }) t.test('undeprecate', async t => { + t.teardown(() => { + npmFetchBody = null + npmFetchLog = null + }) await deprecate.exec(['foo', '']) + t.ok(npmFetchLog, 'was passed a logger') t.match(npmFetchBody, { versions: { '1.0.0': { deprecated: '' }, @@ -95,9 +103,11 @@ t.test('undeprecate', async t => { t.test('deprecates given range', async t => { t.teardown(() => { npmFetchBody = null + npmFetchLog = null }) await deprecate.exec(['foo@1.0.0', 'this version is deprecated']) + t.ok(npmFetchLog, 'was passed a logger') t.match(npmFetchBody, { versions: { '1.0.0': { diff --git a/test/lib/commands/diff.js b/test/lib/commands/diff.js index ed0702e3784a6..f73a543cb4c51 100644 --- a/test/lib/commands/diff.js +++ b/test/lib/commands/diff.js @@ -61,9 +61,10 @@ const diff = new Diff(npm) t.test('no args', t => { t.test('in a project dir', async t => { - t.plan(3) + t.plan(4) libnpmdiff = async ([a, b], opts) => { + t.ok(opts.log, 'should be passed a logger') t.equal(a, 'foo@latest', 'should have default spec comparison') t.equal(b, `file:${fooPath}`, 'should compare to cwd') t.match(opts, npm.flatOptions, 'should forward flat options') diff --git a/test/lib/commands/dist-tag.js b/test/lib/commands/dist-tag.js index 756a09d7de002..b83c30e9c64ea 100644 --- a/test/lib/commands/dist-tag.js +++ b/test/lib/commands/dist-tag.js @@ -43,6 +43,7 @@ const routeMap = { // XXX overriding this does not appear to do anything, adding t.plan to things // that use it fails the test let npmRegistryFetchMock = (url, opts) => { + npmRegistryFetchLog = opts.log if (url === '/-/package/foo/dist-tags') { throw new Error('no package found') } @@ -50,7 +51,11 @@ let npmRegistryFetchMock = (url, opts) => { return routeMap[url] } -npmRegistryFetchMock.json = async (url, opts) => routeMap[url] +let npmRegistryFetchLog +npmRegistryFetchMock.json = async (url, opts) => { + npmRegistryFetchLog = opts.log + return routeMap[url] +} const logger = (...msgs) => { for (const msg of [...msgs]) { @@ -81,6 +86,10 @@ const npm = mockNpm({ }) const distTag = new DistTag(npm) +t.afterEach(() => { + npmRegistryFetchLog = null +}) + t.test('ls in current package', async t => { npm.prefix = t.testdir({ 'package.json': JSON.stringify({ @@ -88,6 +97,7 @@ t.test('ls in current package', async t => { }), }) await distTag.exec(['ls']) + t.ok(npmRegistryFetchLog, 'is passed a logger') t.matchSnapshot( result, 'should list available tags for current package' @@ -289,6 +299,7 @@ t.test('add new tag', async t => { }) npmRegistryFetchMock = async (url, opts) => { + t.ok(opts.log, 'is passed a logger') t.equal(opts.method, 'PUT', 'should trigger request to add new tag') t.equal(opts.body, '7.7.7', 'should point to expected version') } @@ -355,6 +366,7 @@ t.test('remove existing tag', async t => { } npm.prefix = t.testdir({}) await distTag.exec(['rm', '@scoped/another', 'c']) + t.ok(npmRegistryFetchLog, 'is passed a logger') t.matchSnapshot(log, 'should log remove info') t.matchSnapshot(result, 'should return success msg') }) diff --git a/test/lib/commands/hook.js b/test/lib/commands/hook.js index cd4b38787280f..a4eee711fe8e0 100644 --- a/test/lib/commands/hook.js +++ b/test/lib/commands/hook.js @@ -78,7 +78,8 @@ t.test('npm hook add', async t => { await hook.exec(['add', 'semver', 'https://google.com', 'some-secret']) - t.strictSame( + t.ok(hookArgs.opts.log, 'is passed a logger') + t.match( hookArgs, { pkg: 'semver', @@ -101,7 +102,8 @@ t.test('npm hook add - unicode output', async t => { await hook.exec(['add', 'semver', 'https://google.com', 'some-secret']) - t.strictSame( + t.ok(hookArgs.opts.log, 'is passed a logger') + t.match( hookArgs, { pkg: 'semver', @@ -124,7 +126,8 @@ t.test('npm hook add - json output', async t => { await hook.exec(['add', '@npmcli', 'https://google.com', 'some-secret']) - t.strictSame( + t.ok(hookArgs.opts.log, 'is passed a logger') + t.match( hookArgs, { pkg: '@npmcli', @@ -156,7 +159,8 @@ t.test('npm hook add - parseable output', async t => { await hook.exec(['add', '@npmcli', 'https://google.com', 'some-secret']) - t.strictSame( + t.ok(hookArgs.opts.log, 'is passed a logger') + t.match( hookArgs, { pkg: '@npmcli', @@ -188,7 +192,8 @@ t.test('npm hook add - silent output', async t => { await hook.exec(['add', '@npmcli', 'https://google.com', 'some-secret']) - t.strictSame( + t.ok(hookArgs.opts.log, 'is passed a logger') + t.match( hookArgs, { pkg: '@npmcli', @@ -209,7 +214,8 @@ t.test('npm hook ls', async t => { await hook.exec(['ls']) - t.strictSame( + t.ok(hookArgs.log, 'is passed a logger') + t.match( hookArgs, { ...npm.flatOptions, @@ -234,7 +240,8 @@ t.test('npm hook ls, no results', async t => { await hook.exec(['ls']) - t.strictSame( + t.ok(hookArgs.log, 'is passed a logger') + t.match( hookArgs, { ...npm.flatOptions, @@ -263,7 +270,8 @@ t.test('npm hook ls, single result', async t => { await hook.exec(['ls']) - t.strictSame( + t.ok(hookArgs.log, 'is passed a logger') + t.match( hookArgs, { ...npm.flatOptions, @@ -286,7 +294,8 @@ t.test('npm hook ls - json output', async t => { await hook.exec(['ls']) - t.strictSame( + t.ok(hookArgs.log, 'is passed a logger') + t.match( hookArgs, { ...npm.flatOptions, @@ -331,7 +340,8 @@ t.test('npm hook ls - parseable output', async t => { await hook.exec(['ls']) - t.strictSame( + t.ok(hookArgs.log, 'is passed a logger') + t.match( hookArgs, { ...npm.flatOptions, @@ -361,7 +371,8 @@ t.test('npm hook ls - silent output', async t => { await hook.exec(['ls']) - t.strictSame( + t.ok(hookArgs.log, 'is passed a logger') + t.match( hookArgs, { ...npm.flatOptions, @@ -380,7 +391,8 @@ t.test('npm hook rm', async t => { await hook.exec(['rm', '1']) - t.strictSame( + t.ok(hookArgs.opts.log, 'is passed a logger') + t.match( hookArgs, { id: '1', @@ -401,7 +413,8 @@ t.test('npm hook rm - unicode output', async t => { await hook.exec(['rm', '1']) - t.strictSame( + t.ok(hookArgs.opts.log, 'is passed a logger') + t.match( hookArgs, { id: '1', @@ -422,7 +435,8 @@ t.test('npm hook rm - silent output', async t => { await hook.exec(['rm', '1']) - t.strictSame( + t.ok(hookArgs.opts.log, 'is passed a logger') + t.match( hookArgs, { id: '1', @@ -443,7 +457,8 @@ t.test('npm hook rm - json output', async t => { await hook.exec(['rm', '1']) - t.strictSame( + t.ok(hookArgs.opts.log, 'is passed a logger') + t.match( hookArgs, { id: '1', @@ -473,7 +488,8 @@ t.test('npm hook rm - parseable output', async t => { await hook.exec(['rm', '1']) - t.strictSame( + t.ok(hookArgs.opts.log, 'is passed a logger') + t.match( hookArgs, { id: '1', @@ -499,7 +515,8 @@ t.test('npm hook update', async t => { await hook.exec(['update', '1', 'https://google.com', 'some-secret']) - t.strictSame( + t.ok(hookArgs.opts.log, 'is passed a logger') + t.match( hookArgs, { id: '1', @@ -522,7 +539,8 @@ t.test('npm hook update - unicode', async t => { await hook.exec(['update', '1', 'https://google.com', 'some-secret']) - t.strictSame( + t.ok(hookArgs.opts.log, 'is passed a logger') + t.match( hookArgs, { id: '1', @@ -545,7 +563,8 @@ t.test('npm hook update - json output', async t => { await hook.exec(['update', '1', 'https://google.com', 'some-secret']) - t.strictSame( + t.ok(hookArgs.opts.log, 'is passed a logger') + t.match( hookArgs, { id: '1', @@ -577,7 +596,8 @@ t.test('npm hook update - parseable output', async t => { await hook.exec(['update', '1', 'https://google.com', 'some-secret']) - t.strictSame( + t.ok(hookArgs.opts.log, 'is passed a logger') + t.match( hookArgs, { id: '1', @@ -607,7 +627,8 @@ t.test('npm hook update - silent output', async t => { await hook.exec(['update', '1', 'https://google.com', 'some-secret']) - t.strictSame( + t.ok(hookArgs.opts.log, 'is passed a logger') + t.match( hookArgs, { id: '1', diff --git a/test/lib/commands/logout.js b/test/lib/commands/logout.js index ee01e7500d141..1a1fbb785c808 100644 --- a/test/lib/commands/logout.js +++ b/test/lib/commands/logout.js @@ -31,7 +31,7 @@ t.afterEach(() => { }) t.test('token logout', async t => { - t.plan(5) + t.plan(6) flatOptions['//registry.npmjs.org/:_authToken'] = '@foo/' @@ -62,7 +62,8 @@ t.test('token logout', async t => { await logout.exec([]) - t.same( + t.ok(result.opts.log, 'should pass a logger') + t.match( result, { url: '/-/user/token/%40foo%2F', @@ -91,7 +92,7 @@ t.test('token scoped logout', async t => { config.save = null }) - t.plan(7) + t.plan(8) flatOptions['//diff-registry.npmjs.com/:_authToken'] = '@bar/' flatOptions['//registry.npmjs.org/:_authToken'] = '@foo/' @@ -132,7 +133,8 @@ t.test('token scoped logout', async t => { await logout.exec([]) - t.same( + t.ok(result.opts.log, 'should pass a logger') + t.match( result, { url: '/-/user/token/%40bar%2F', @@ -202,7 +204,7 @@ t.test('ignore invalid scoped registry config', async t => { config.delete = null config.save = null }) - t.plan(4) + t.plan(5) flatOptions['//registry.npmjs.org/:_authToken'] = '@foo/' config.scope = '@myscope' @@ -234,7 +236,8 @@ t.test('ignore invalid scoped registry config', async t => { await logout.exec([]) - t.same( + t.ok(result.opts.log, 'should pass a logger') + t.match( result, { url: '/-/user/token/%40foo%2F', diff --git a/test/lib/commands/owner.js b/test/lib/commands/owner.js index b5d4d1584289d..a32a3df9b7dde 100644 --- a/test/lib/commands/owner.js +++ b/test/lib/commands/owner.js @@ -51,13 +51,14 @@ t.test('owner no args', async t => { }) t.test('owner ls no args', async t => { - t.plan(4) + t.plan(5) result = '' readPackageNameResponse = '@npmcli/map-workspaces' pacote.packument = async (spec, opts) => { t.equal(spec.name, '@npmcli/map-workspaces', 'should use expect pkg name') + t.ok(opts.log, 'is passed a logger') t.match( opts, { @@ -172,10 +173,11 @@ t.test('owner ls no maintainers', async t => { }) t.test('owner add ', async t => { - t.plan(8) + t.plan(11) result = '' npmFetch.json = async (uri, opts) => { + t.ok(opts.log, 'is passed a logger') // retrieve user info from couchdb request if (uri === '/-/user/org.couchdb.user:foo') { t.ok('should request user info') @@ -216,6 +218,7 @@ t.test('owner add ', async t => { } } pacote.packument = async (spec, opts) => { + t.ok(opts.log, 'is passed a logger') t.equal(spec.name, '@npmcli/map-workspaces', 'should use expect pkg name') t.match( opts, @@ -244,6 +247,7 @@ t.test('owner add cwd package', async t => { result = '' readPackageNameResponse = '@npmcli/map-workspaces' npmFetch.json = async (uri, opts) => { + t.ok(opts.log, 'is passed a logger') // retrieve user info from couchdb request if (uri === '/-/user/org.couchdb.user:foo') { return { @@ -273,7 +277,7 @@ t.test('owner add cwd package', async t => { }) t.test('owner add already an owner', async t => { - t.plan(2) + t.plan(3) result = '' log.info = (title, msg) => { @@ -285,6 +289,7 @@ t.test('owner add already an owner', async t => { ) } npmFetch.json = async (uri, opts) => { + t.ok(opts.log, 'is passed a logger') // retrieve user info from couchdb request if (uri === '/-/user/org.couchdb.user:ruyadorno') { return { @@ -316,6 +321,7 @@ t.test('owner add fails to retrieve user', async t => { result = '' readPackageNameResponse = npmFetch.json = async (uri, opts) => { + t.ok(opts.log, 'is passed a logger') // retrieve borked user info from couchdb request if (uri === '/-/user/org.couchdb.user:foo') { return { ok: false } @@ -346,6 +352,7 @@ t.test('owner add fails to retrieve user', async t => { t.test('owner add fails to PUT updates', async t => { result = '' npmFetch.json = async (uri, opts) => { + t.ok(opts.log, 'is passed a logger') // retrieve user info from couchdb request if (uri === '/-/user/org.couchdb.user:foo') { return { @@ -382,7 +389,7 @@ t.test('owner add fails to PUT updates', async t => { }) t.test('owner add fails to retrieve user info', async t => { - t.plan(3) + t.plan(4) result = '' log.error = (title, msg) => { @@ -390,6 +397,7 @@ t.test('owner add fails to retrieve user info', async t => { t.equal(msg, 'Error getting user data for foo') } npmFetch.json = async (uri, opts) => { + t.ok(opts.log, 'is passed a logger') // retrieve user info from couchdb request if (uri === '/-/user/org.couchdb.user:foo') { throw Object.assign( @@ -421,6 +429,7 @@ t.test('owner add fails to retrieve user info', async t => { t.test('owner add no previous maintainers property from server', async t => { result = '' npmFetch.json = async (uri, opts) => { + t.ok(opts.log, 'is passed a logger') // retrieve user info from couchdb request if (uri === '/-/user/org.couchdb.user:foo') { return { @@ -487,10 +496,11 @@ t.test('owner add no cwd package', async t => { }) t.test('owner rm ', async t => { - t.plan(8) + t.plan(11) result = '' npmFetch.json = async (uri, opts) => { + t.ok(opts.log, 'is passed a logger') // retrieve user info from couchdb request if (uri === '/-/user/org.couchdb.user:ruyadorno') { t.ok('should request user info') @@ -524,6 +534,7 @@ t.test('owner rm ', async t => { } } pacote.packument = async (spec, opts) => { + t.ok(opts.log, 'is passed a logger') t.equal(spec.name, '@npmcli/map-workspaces', 'should use expect pkg name') t.match( opts, @@ -549,7 +560,7 @@ t.test('owner rm ', async t => { }) t.test('owner rm not a current owner', async t => { - t.plan(2) + t.plan(3) result = '' log.info = (title, msg) => { @@ -557,6 +568,7 @@ t.test('owner rm not a current owner', async t => { t.equal(msg, 'Not a package owner: foo', 'should log.info not a package owner msg') } npmFetch.json = async (uri, opts) => { + t.ok(opts.log, 'is passed a logger') // retrieve user info from couchdb request if (uri === '/-/user/org.couchdb.user:foo') { return { @@ -590,6 +602,7 @@ t.test('owner rm cwd package', async t => { result = '' readPackageNameResponse = '@npmcli/map-workspaces' npmFetch.json = async (uri, opts) => { + t.ok(opts.log, 'is passed a logger') // retrieve user info from couchdb request if (uri === '/-/user/org.couchdb.user:ruyadorno') { return { @@ -622,6 +635,7 @@ t.test('owner rm only user', async t => { result = '' readPackageNameResponse = 'ipt' npmFetch.json = async (uri, opts) => { + t.ok(opts.log, 'is passed a logger') // retrieve user info from couchdb request if (uri === '/-/user/org.couchdb.user:ruyadorno') { return { diff --git a/test/lib/commands/ping.js b/test/lib/commands/ping.js index f808e0ac3ba2a..19ba9d586b763 100644 --- a/test/lib/commands/ping.js +++ b/test/lib/commands/ping.js @@ -2,12 +2,13 @@ const t = require('tap') const { fake: mockNpm } = require('../../fixtures/mock-npm') t.test('pings', async t => { - t.plan(6) + t.plan(7) const registry = 'https://registry.npmjs.org' let noticeCalls = 0 const Ping = t.mock('../../../lib/commands/ping.js', { '../../../lib/utils/ping.js': function (spec) { + t.ok(spec.log, 'is passed a logger') t.equal(spec.registry, registry, 'passes flatOptions') return {} }, @@ -35,13 +36,14 @@ t.test('pings', async t => { }) t.test('pings and logs details', async t => { - t.plan(8) + t.plan(9) const registry = 'https://registry.npmjs.org' const details = { extra: 'data' } let noticeCalls = 0 const Ping = t.mock('../../../lib/commands/ping.js', { '../../../lib/utils/ping.js': function (spec) { + t.ok(spec.log, 'is passed a logger') t.equal(spec.registry, registry, 'passes flatOptions') return details }, @@ -73,13 +75,14 @@ t.test('pings and logs details', async t => { }) t.test('pings and returns json', async t => { - t.plan(9) + t.plan(10) const registry = 'https://registry.npmjs.org' const details = { extra: 'data' } let noticeCalls = 0 const Ping = t.mock('../../../lib/commands/ping.js', { '../../../lib/utils/ping.js': function (spec) { + t.ok(spec.log, 'is passed a logger') t.equal(spec.registry, registry, 'passes flatOptions') return details }, diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index 2a591fd4c7534..52d4c1b342908 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -25,7 +25,7 @@ t.test( /* eslint-disable-next-line max-len */ 'should publish with libnpmpublish, passing through flatOptions and respecting publishConfig.registry', async t => { - t.plan(6) + t.plan(7) const registry = 'https://some.registry' const publishConfig = { registry } @@ -59,6 +59,7 @@ t.test( t.match(manifest, { name: 'my-cool-pkg', version: '1.0.0' }, 'gets manifest') t.type(tarData, Buffer, 'tarData is a buffer') t.ok(opts, 'gets opts object') + t.ok(opts.log, 'gets passed a logger') t.same(opts.customValue, true, 'flatOptions values are passed through') t.same(opts.registry, registry, 'publishConfig.registry is passed through') }, @@ -81,7 +82,7 @@ t.test( ) t.test('re-loads publishConfig.registry if added during script process', async t => { - t.plan(5) + t.plan(6) const registry = 'https://some.registry' const publishConfig = { registry } const testDir = t.testdir({ @@ -112,6 +113,7 @@ t.test('re-loads publishConfig.registry if added during script process', async t t.match(manifest, { name: 'my-cool-pkg', version: '1.0.0' }, 'gets manifest') t.type(tarData, Buffer, 'tarData is a buffer') t.ok(opts, 'gets opts object') + t.ok(opts.log, 'gets passed a logger') t.same(opts.registry, registry, 'publishConfig.registry is passed through') }, }, @@ -292,7 +294,7 @@ t.test('throws when invalid tag', async t => { }) t.test('can publish a tarball', async t => { - t.plan(3) + t.plan(4) const testDir = t.testdir({ tarball: {}, @@ -317,6 +319,7 @@ t.test('can publish a tarball', async t => { const Publish = t.mock('../../../lib/commands/publish.js', { libnpmpublish: { publish: (manifest, tarData, opts) => { + t.ok(opts.log, 'gets passed a logger') t.match( manifest, { @@ -412,7 +415,7 @@ t.test('should check auth for scope specific registry', async t => { }) t.test('should use auth for scope specific registry', async t => { - t.plan(3) + t.plan(4) const registry = 'https://some.registry' const testDir = t.testdir({ 'package.json': JSON.stringify( @@ -429,6 +432,7 @@ t.test('should use auth for scope specific registry', async t => { libnpmpublish: { publish: (manifest, tarData, opts) => { t.ok(opts, 'gets opts object') + t.ok(opts.log, 'gets passed a logger') t.same(opts['@npm:registry'], registry, 'scope specific registry is passed through') }, }, @@ -446,7 +450,7 @@ t.test('should use auth for scope specific registry', async t => { }) t.test('read registry only from publishConfig', async t => { - t.plan(3) + t.plan(4) const registry = 'https://some.registry' const publishConfig = { registry } @@ -465,6 +469,7 @@ t.test('read registry only from publishConfig', async t => { const Publish = t.mock('../../../lib/commands/publish.js', { libnpmpublish: { publish: (manifest, tarData, opts) => { + t.ok(opts.log, 'gets passed a logger') t.match(manifest, { name: 'my-cool-pkg', version: '1.0.0' }, 'gets manifest') t.same(opts.registry, registry, 'publishConfig is passed through') }, @@ -481,7 +486,7 @@ t.test('read registry only from publishConfig', async t => { }) t.test('able to publish after if encountered multiple configs', async t => { - t.plan(2) + t.plan(3) const registry = 'https://some.registry' const tag = 'better-tag' @@ -510,6 +515,7 @@ t.test('able to publish after if encountered multiple configs', async t => { const Publish = t.mock('../../../lib/commands/publish.js', { libnpmpublish: { publish: (manifest, tarData, opts) => { + t.ok(opts.log, 'gets passed a logger') t.same(opts.defaultTag, tag, 'gets option for expected tag') }, }, @@ -748,7 +754,7 @@ t.test('private workspaces', async t => { if (manifest.private) { throw new Error('ERR') } - + t.ok(opts.log, 'gets passed a logger') publishes.push(manifest) }, }, diff --git a/test/lib/commands/star.js b/test/lib/commands/star.js index 9a49036422d5e..2f4ddc9dc6439 100644 --- a/test/lib/commands/star.js +++ b/test/lib/commands/star.js @@ -42,17 +42,20 @@ t.test('no args', async t => { }) t.test('star a package', async t => { - t.plan(4) + t.plan(6) const pkgName = '@npmcli/arborist' - npmFetch.json = async (uri, opts) => ({ - _id: pkgName, - _rev: 'hash', - users: ( - opts.method === 'PUT' - ? { foo: true } - : {} - ), - }) + npmFetch.json = async (uri, opts) => { + t.ok(opts.log, 'is passed a logger') + return { + _id: pkgName, + _rev: 'hash', + users: ( + opts.method === 'PUT' + ? { foo: true } + : {} + ), + } + } log.info = (title, msg, id) => { t.equal(title, 'star', 'should use expected title') t.equal(msg, 'starring', 'should use expected msg') @@ -67,17 +70,20 @@ t.test('star a package', async t => { }) t.test('unstar a package', async t => { - t.plan(4) + t.plan(6) const pkgName = '@npmcli/arborist' config['star.unstar'] = true - npmFetch.json = async (uri, opts) => ({ - _id: pkgName, - _rev: 'hash', - ...(opts.method === 'PUT' - ? {} - : { foo: true } - ), - }) + npmFetch.json = async (uri, opts) => { + t.ok(opts.log, 'is passed a logger') + return { + _id: pkgName, + _rev: 'hash', + ...(opts.method === 'PUT' + ? {} + : { foo: true } + ), + } + } log.info = (title, msg, id) => { t.equal(title, 'unstar', 'should use expected title') t.equal(msg, 'unstarring', 'should use expected msg')