From 57d65ee889d1087fcb2d919da6384a873137d1b3 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sun, 10 Apr 2022 22:21:17 -0700 Subject: [PATCH 1/3] fix: show more information during publish dry-run Fixes #4317 --- lib/commands/publish.js | 25 +++++++++++++------------ test/lib/commands/publish.js | 17 ++++++++++------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/lib/commands/publish.js b/lib/commands/publish.js index 1f26370e89a56..560dae1b6f0e4 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -102,19 +102,20 @@ class Publish extends BaseCommand { logTar(pkgContents, { unicode }) } + const resolved = npa.resolve(manifest.name, manifest.version) + const registry = npmFetch.pickRegistry(resolved, opts) + const creds = this.npm.config.getCredentialsByURI(registry) + const outputRegistry = replaceInfo(registry) + if (!creds.token && !creds.username) { + throw Object.assign( + new Error(`This command requires you to be logged in to ${outputRegistry}`), { + code: 'ENEEDAUTH', + } + ) + } + log.notice('', `Publishing to ${outputRegistry}${dryRun ? ' (dry-run)' : ''}`) + if (!dryRun) { - const resolved = npa.resolve(manifest.name, manifest.version) - const registry = npmFetch.pickRegistry(resolved, opts) - const creds = this.npm.config.getCredentialsByURI(registry) - const outputRegistry = replaceInfo(registry) - if (!creds.token && !creds.username) { - throw Object.assign( - new Error(`This command requires you to be logged in to ${outputRegistry}`), { - code: 'ENEEDAUTH', - } - ) - } - log.notice('', `Publishing to ${outputRegistry}`) await otplease(opts, opts => libpub(manifest, tarballData, opts)) } diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index 0acce8b001921..e266ea524ccc0 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -163,9 +163,9 @@ t.test('if loglevel=info and json, should not output package contents', async t t.test( /* eslint-disable-next-line max-len */ - 'if loglevel=silent and dry-run, should not output package contents or publish or validate credentials, should log tarball contents', + 'if loglevel=silent and dry-run, should not output package contents or publish, should log tarball contents', async t => { - t.plan(1) + t.plan(2) const testDir = t.testdir({ 'package.json': JSON.stringify( @@ -199,8 +199,9 @@ t.test( throw new Error('should not output in dry run mode') }, }, t) - npm.config.getCredentialsByURI = () => { - throw new Error('should not call getCredentialsByURI in dry run') + npm.config.getCredentialsByURI = uri => { + t.same(uri, npm.config.get('registry'), 'gets credentials for expected registry') + return { token: 'some.registry.token' } } const publish = new Publish(npm) @@ -213,7 +214,7 @@ t.test( /* eslint-disable-next-line max-len */ 'if loglevel=info and dry-run, should not publish, should log package contents and log tarball contents', async t => { - t.plan(2) + t.plan(3) const testDir = t.testdir({ 'package.json': JSON.stringify( @@ -247,9 +248,11 @@ t.test( t.pass('output fn is called') }, }, t) - npm.config.getCredentialsByURI = () => { - throw new Error('should not call getCredentialsByURI in dry run') + npm.config.getCredentialsByURI = uri => { + t.same(uri, npm.config.get('registry'), 'gets credentials for expected registry') + return { token: 'some.registry.token' } } + const publish = new Publish(npm) await publish.exec([testDir]) From bcb5b2876a4ccc6157ed963fdbc708b7460d71f2 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Mon, 11 Apr 2022 11:58:18 -0700 Subject: [PATCH 2/3] fixup! fix: show more information during publish dry-run --- lib/commands/publish.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/commands/publish.js b/lib/commands/publish.js index 560dae1b6f0e4..51861c5aa3554 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -105,14 +105,18 @@ class Publish extends BaseCommand { const resolved = npa.resolve(manifest.name, manifest.version) const registry = npmFetch.pickRegistry(resolved, opts) const creds = this.npm.config.getCredentialsByURI(registry) + const noCreds = !creds.token && !creds.username const outputRegistry = replaceInfo(registry) - if (!creds.token && !creds.username) { - throw Object.assign( - new Error(`This command requires you to be logged in to ${outputRegistry}`), { - code: 'ENEEDAUTH', - } - ) + + if (noCreds) { + const msg = `This command requires you to be logged in to ${outputRegistry}` + if (dryRun) { + log.warn('', `${msg} (dry-run)`) + } else { + throw Object.assign(new Error(msg), { code: 'ENEEDAUTH' }) + } } + log.notice('', `Publishing to ${outputRegistry}${dryRun ? ' (dry-run)' : ''}`) if (!dryRun) { From 068d8b265ac927c7a13096138e08ccf5052ae28b Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Mon, 11 Apr 2022 12:25:06 -0700 Subject: [PATCH 3/3] fixup! fix: show more information during publish dry-run --- test/lib/commands/publish.js | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index e266ea524ccc0..64eb7c60cb062 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -227,6 +227,18 @@ t.test( ), }) + const npm = mockNpm({ + config: { 'dry-run': true, loglevel: 'info' }, + output: () => { + t.pass('output fn is called') + }, + }, t) + const registry = npm.config.get('registry') + npm.config.getCredentialsByURI = uri => { + t.same(uri, registry, 'gets credentials for expected registry') + return { /* no token will call log.warn */ } + } + const Publish = t.mock('../../../lib/commands/publish.js', { '../../../lib/utils/tar.js': { getContents: () => ({ @@ -235,6 +247,12 @@ t.test( logTar: () => { t.pass('logTar is called') }, + 'proc-log': { + warn (_, msg) { + t.match(msg, + `This command requires you to be logged in to ${registry} (dry-run)`) + }, + }, }, libnpmpublish: { publish: () => { @@ -242,16 +260,6 @@ t.test( }, }, }) - const npm = mockNpm({ - config: { 'dry-run': true, loglevel: 'info' }, - output: () => { - t.pass('output fn is called') - }, - }, t) - npm.config.getCredentialsByURI = uri => { - t.same(uri, npm.config.get('registry'), 'gets credentials for expected registry') - return { token: 'some.registry.token' } - } const publish = new Publish(npm)