From f66808b7463b0974a98d576f1f22b84014f4229b Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Tue, 30 Apr 2024 15:28:25 -0700 Subject: [PATCH] fix: run input.start around help and openining urls This also refactors open url and open url prompt to the same file. --- lib/commands/fund.js | 3 +- lib/commands/help.js | 10 +- lib/package-url-cmd.js | 2 +- lib/utils/auth.js | 2 +- lib/utils/open-url-prompt.js | 66 ------ lib/utils/open-url.js | 109 +++++++--- lib/utils/otplease.js | 2 +- test/lib/commands/bugs.js | 2 +- test/lib/commands/docs.js | 2 +- test/lib/commands/repo.js | 4 +- test/lib/utils/open-url-prompt.js | 156 -------------- test/lib/utils/open-url.js | 343 ++++++++++++++++++++++-------- test/lib/utils/otplease.js | 2 +- 13 files changed, 344 insertions(+), 359 deletions(-) delete mode 100644 lib/utils/open-url-prompt.js delete mode 100644 test/lib/utils/open-url-prompt.js diff --git a/lib/commands/fund.js b/lib/commands/fund.js index 8bcd184e70968..8bb4c304b379b 100644 --- a/lib/commands/fund.js +++ b/lib/commands/fund.js @@ -5,8 +5,7 @@ const { output } = require('proc-log') const npa = require('npm-package-arg') const { depth } = require('treeverse') const { readTree: getFundingInfo, normalizeFunding, isValidFunding } = require('libnpmfund') - -const openUrl = require('../utils/open-url.js') +const { openUrl } = require('../utils/open-url.js') const ArboristWorkspaceCmd = require('../arborist-cmd.js') const getPrintableName = ({ name, version }) => { diff --git a/lib/commands/help.js b/lib/commands/help.js index fb3fe664e017d..a2307bf1d8a2c 100644 --- a/lib/commands/help.js +++ b/lib/commands/help.js @@ -1,8 +1,8 @@ const spawn = require('@npmcli/promise-spawn') const path = require('path') -const openUrl = require('../utils/open-url.js') +const { openUrl } = require('../utils/open-url.js') const { glob } = require('glob') -const { output } = require('proc-log') +const { output, input } = require('proc-log') const localeCompare = require('@isaacs/string-locale-compare')('en') const { deref } = require('../utils/cmd-list.js') const BaseCommand = require('../base-cmd.js') @@ -95,13 +95,15 @@ class Help extends BaseCommand { args = ['emacsclient', ['-e', `(woman-find-file '${man}')`]] } - return spawn(...args, { stdio: 'inherit' }).catch(err => { + try { + await input.start(() => spawn(...args, { stdio: 'inherit' })) + } catch (err) { if (err.code) { throw new Error(`help process exited with code: ${err.code}`) } else { throw err } - }) + } } // Returns the path to the html version of the man page diff --git a/lib/package-url-cmd.js b/lib/package-url-cmd.js index bcefd17af4492..c7ae32174fcb6 100644 --- a/lib/package-url-cmd.js +++ b/lib/package-url-cmd.js @@ -1,5 +1,5 @@ const pacote = require('pacote') -const openUrl = require('./utils/open-url.js') +const { openUrl } = require('./utils/open-url.js') const { log } = require('proc-log') const BaseCommand = require('./base-cmd.js') diff --git a/lib/utils/auth.js b/lib/utils/auth.js index 04ca455ceb526..8da6f0a7ca8f1 100644 --- a/lib/utils/auth.js +++ b/lib/utils/auth.js @@ -1,6 +1,6 @@ const profile = require('npm-profile') const { log } = require('proc-log') -const openUrlPrompt = require('../utils/open-url-prompt.js') +const { openUrlPrompt } = require('../utils/open-url.js') const read = require('../utils/read-user-info.js') const otplease = require('../utils/otplease.js') diff --git a/lib/utils/open-url-prompt.js b/lib/utils/open-url-prompt.js deleted file mode 100644 index 6f4d453a959d5..0000000000000 --- a/lib/utils/open-url-prompt.js +++ /dev/null @@ -1,66 +0,0 @@ -const readline = require('readline') -const { input, output } = require('proc-log') -const open = require('./open-url.js') - -function print (npm, title, url) { - const json = npm.config.get('json') - - const message = json ? JSON.stringify({ title, url }) : `${title}:\n${url}` - - output.standard(message) -} - -// Prompt to open URL in browser if possible -const promptOpen = async (npm, url, title, prompt, emitter) => { - const browser = npm.config.get('browser') - const isInteractive = process.stdin.isTTY === true && process.stdout.isTTY === true - - try { - if (!/^https?:$/.test(new URL(url).protocol)) { - throw new Error() - } - } catch (_) { - throw new Error('Invalid URL: ' + url) - } - - print(npm, title, url) - - if (browser === false || !isInteractive) { - return - } - - const rl = readline.createInterface({ - input: process.stdin, - output: process.stdout, - }) - - const tryOpen = await input.read(() => new Promise(resolve => { - rl.on('SIGINT', () => { - rl.close() - resolve('SIGINT') - }) - - rl.question(prompt, () => { - resolve(true) - }) - - if (emitter && emitter.addListener) { - emitter.addListener('abort', () => { - rl.close() - resolve(false) - }) - } - })) - - if (tryOpen === 'SIGINT') { - throw new Error('canceled') - } - - if (!tryOpen) { - return - } - - await open(npm, url, 'Browser unavailable. Please open the URL manually') -} - -module.exports = promptOpen diff --git a/lib/utils/open-url.js b/lib/utils/open-url.js index 46b7abc731fa1..2201dbfbd04c1 100644 --- a/lib/utils/open-url.js +++ b/lib/utils/open-url.js @@ -1,51 +1,100 @@ const promiseSpawn = require('@npmcli/promise-spawn') -const { output } = require('proc-log') - +const { output, input } = require('proc-log') const { URL } = require('url') +const readline = require('readline') + +const assertValidUrl = (url) => { + try { + if (!/^https?:$/.test(new URL(url).protocol)) { + throw new Error() + } + } catch { + throw new Error('Invalid URL: ' + url) + } +} + +const outputMsg = (json, title, url) => { + const msg = json ? JSON.stringify({ title, url }) : `${title}:\n${url}\n` + output.standard(msg) +} // attempt to open URL in web-browser, print address otherwise: -const open = async (npm, url, errMsg, isFile) => { +const openUrl = async (npm, url, title, isFile) => { url = encodeURI(url) const browser = npm.config.get('browser') - - function printAlternateMsg () { - const json = npm.config.get('json') - const alternateMsg = json - ? JSON.stringify({ - title: errMsg, - url, - }, null, 2) - : `${errMsg}:\n ${url}\n` - - output.standard(alternateMsg) - } + const json = npm.config.get('json') if (browser === false) { - printAlternateMsg() + outputMsg(json, title, url) return } // We pass this in as true from the help command so we know we don't have to // check the protocol if (!isFile) { - try { - if (!/^https?:$/.test(new URL(url).protocol)) { - throw new Error() - } - } catch { - throw new Error('Invalid URL: ' + url) + assertValidUrl(url) + } + + try { + await input.start(() => promiseSpawn.open(url, { + command: browser === true ? null : browser, + })) + } catch (err) { + if (err.code !== 127) { + throw err } + outputMsg(json, title, url) + } +} + +// Prompt to open URL in browser if possible +const openUrlPrompt = async (npm, url, title, prompt, emitter) => { + const browser = npm.config.get('browser') + const json = npm.config.get('json') + const isInteractive = process.stdin.isTTY === true && process.stdout.isTTY === true + + assertValidUrl(url) + outputMsg(json, title, url) + + if (browser === false || !isInteractive) { + return } - const command = browser === true ? null : browser - await promiseSpawn.open(url, { command }) - .catch((err) => { - if (err.code !== 127) { - throw err - } + const rl = readline.createInterface({ + input: process.stdin, + output: process.stdout, + }) - printAlternateMsg() + const tryOpen = await input.read(() => new Promise(resolve => { + rl.on('SIGINT', () => { + rl.close() + resolve('SIGINT') }) + + rl.question(prompt, () => { + resolve(true) + }) + + if (emitter && emitter.addListener) { + emitter.addListener('abort', () => { + rl.close() + resolve(false) + }) + } + })) + + if (tryOpen === 'SIGINT') { + throw new Error('canceled') + } + + if (!tryOpen) { + return + } + + await openUrl(npm, url, 'Browser unavailable. Please open the URL manually') } -module.exports = open +module.exports = { + openUrl, + openUrlPrompt, +} diff --git a/lib/utils/otplease.js b/lib/utils/otplease.js index b8dd0b66ed766..68bb881c7ec2a 100644 --- a/lib/utils/otplease.js +++ b/lib/utils/otplease.js @@ -8,7 +8,7 @@ async function otplease (npm, opts, fn) { if (isWebOTP(err)) { const webAuth = require('./web-auth') - const openUrlPrompt = require('./open-url-prompt') + const { openUrlPrompt } = require('./open-url') const openerPromise = (url, emitter) => openUrlPrompt( diff --git a/test/lib/commands/bugs.js b/test/lib/commands/bugs.js index e2ebfb5306574..c624ba3f2d91d 100644 --- a/test/lib/commands/bugs.js +++ b/test/lib/commands/bugs.js @@ -63,7 +63,7 @@ t.test('open bugs urls & emails', async t => { const { npm } = await loadMockNpm(t, { mocks: { pacote, - '{LIB}/utils/open-url.js': openUrl, + '{LIB}/utils/open-url.js': { openUrl }, }, }) diff --git a/test/lib/commands/docs.js b/test/lib/commands/docs.js index e11df6b07bc5e..2a7951edeaf76 100644 --- a/test/lib/commands/docs.js +++ b/test/lib/commands/docs.js @@ -84,7 +84,7 @@ const setup = async (t, { prefixDir = fixtures.pkg, config } = {}) => { const res = await mockNpm(t, { prefixDir, mocks: { - '{LIB}/utils/open-url.js': openUrl, + '{LIB}/utils/open-url.js': { openUrl }, }, config, }) diff --git a/test/lib/commands/repo.js b/test/lib/commands/repo.js index 114cdf919510a..ac27d9c0a719c 100644 --- a/test/lib/commands/repo.js +++ b/test/lib/commands/repo.js @@ -187,10 +187,10 @@ const loadMockNpm = async (t, prefixDir, config = {}) => { const mock = await mockNpm(t, { command: 'repo', mocks: { - '{LIB}/utils/open-url.js': async (_, url) => { + '{LIB}/utils/open-url.js': { openUrl: async (_, url) => { opened[url] = opened[url] || 0 opened[url]++ - }, + } }, }, config, prefixDir, diff --git a/test/lib/utils/open-url-prompt.js b/test/lib/utils/open-url-prompt.js deleted file mode 100644 index 91058ec9e29a4..0000000000000 --- a/test/lib/utils/open-url-prompt.js +++ /dev/null @@ -1,156 +0,0 @@ -const t = require('tap') -const EventEmitter = require('events') -const tmock = require('../../fixtures/tmock') -const mockNpm = require('../../fixtures/mock-npm') - -const mockOpenUrlPrompt = async (t, { - questionShouldResolve = true, - openUrlPromptInterrupted = false, - openerResult = null, - isTTY = true, - emitter = null, - url: openUrl = 'https://www.npmjs.com', - ...config -}) => { - const mock = await mockNpm(t, { - globals: { - 'process.stdin.isTTY': isTTY, - 'process.stdout.isTTY': isTTY, - }, - config, - }) - - let openerUrl = null - let openerOpts = null - - const openUrlPrompt = tmock(t, '{LIB}/utils/open-url-prompt.js', { - '@npmcli/promise-spawn': { - open: async (url, options) => { - openerUrl = url - openerOpts = options - if (openerResult) { - throw openerResult - } - }, - }, - readline: { - createInterface: () => ({ - question: (_q, cb) => { - if (questionShouldResolve === true) { - cb() - } - }, - close: () => {}, - on: (_signal, cb) => { - if (openUrlPromptInterrupted && _signal === 'SIGINT') { - cb() - } - }, - }), - }, - }) - - let error - const args = [mock.npm, openUrl, 'npm home', 'prompt'] - if (emitter) { - mock.open = openUrlPrompt(...args, emitter) - } else { - await openUrlPrompt(...args).catch((er) => error = er) - } - - return { - ...mock, - openerUrl, - openerOpts, - OUTPUT: mock.joinedOutput(), - emitter, - error, - } -} - -t.test('does not open a url in non-interactive environments', async t => { - const { openerUrl, openerOpts } = await mockOpenUrlPrompt(t, { isTTY: false }) - - t.equal(openerUrl, null, 'did not open') - t.same(openerOpts, null, 'did not open') -}) - -t.test('opens a url', async t => { - const { OUTPUT, openerUrl, openerOpts } = await mockOpenUrlPrompt(t, { browser: true }) - - t.equal(openerUrl, 'https://www.npmjs.com', 'opened the given url') - t.same(openerOpts, { command: null }, 'passed command as null (the default)') - t.matchSnapshot(OUTPUT) -}) - -t.test('opens a url with browser string', async t => { - const { openerUrl, openerOpts } = await mockOpenUrlPrompt(t, { browser: 'firefox' }) - - t.equal(openerUrl, 'https://www.npmjs.com', 'opened the given url') - // FIXME: browser string is parsed as a boolean in config layer - // this is a bug that should be fixed or the config should not allow it - t.same(openerOpts, { command: null }, 'passed command as null (the default)') -}) - -t.test('prints json output', async t => { - const { OUTPUT } = await mockOpenUrlPrompt(t, { json: true }) - - t.matchSnapshot(OUTPUT) -}) - -t.test('returns error for non-https url', async t => { - const { error, OUTPUT, openerUrl, openerOpts } = await mockOpenUrlPrompt(t, { - url: 'ftp://www.npmjs.com', - }) - - t.match(error, /Invalid URL/, 'got the correct error') - t.equal(openerUrl, null, 'did not open') - t.same(openerOpts, null, 'did not open') - t.same(OUTPUT, '', 'printed no output') -}) - -t.test('does not open url if canceled', async t => { - const emitter = new EventEmitter() - const { openerUrl, openerOpts, open } = await mockOpenUrlPrompt(t, { - questionShouldResolve: false, - emitter, - }) - - emitter.emit('abort') - - await open - - t.equal(openerUrl, null, 'did not open') - t.same(openerOpts, null, 'did not open') -}) - -t.test('returns error when opener errors', async t => { - const { error, openerUrl } = await mockOpenUrlPrompt(t, { - openerResult: Object.assign(new Error('Opener failed'), { code: 1 }), - }) - - t.match(error, /Opener failed/, 'got the correct error') - t.equal(openerUrl, 'https://www.npmjs.com', 'did not open') -}) - -t.test('does not error when opener can not find command', async t => { - const { OUTPUT, error, openerUrl } = await mockOpenUrlPrompt(t, { - // openerResult: new Error('Opener failed'), - openerResult: Object.assign(new Error('Opener failed'), { code: 127 }), - }) - - t.notOk(error, 'Did not error') - t.equal(openerUrl, 'https://www.npmjs.com', 'did not open') - t.matchSnapshot(OUTPUT, 'Outputs extra Browser unavailable message and url') -}) - -t.test('throws "canceled" error on SIGINT', async t => { - const emitter = new EventEmitter() - const { open } = await mockOpenUrlPrompt(t, { - questionShouldResolve: false, - openUrlPromptInterrupted: true, - emitter, - }) - - await t.rejects(open, /canceled/, 'message is canceled') -}) diff --git a/test/lib/utils/open-url.js b/test/lib/utils/open-url.js index dab7b41b92f1f..23d8f8b3edc59 100644 --- a/test/lib/utils/open-url.js +++ b/test/lib/utils/open-url.js @@ -1,6 +1,7 @@ const t = require('tap') const tmock = require('../../fixtures/tmock') const mockNpm = require('../../fixtures/mock-npm') +const EventEmitter = require('events') const mockOpenUrl = async (t, args, { openerResult, ...config } = {}) => { let openerUrl = null @@ -16,7 +17,7 @@ const mockOpenUrl = async (t, args, { openerResult, ...config } = {}) => { const mock = await mockNpm(t, { config }) - const openUrl = tmock(t, '{LIB}/utils/open-url.js', { + const { openUrl } = tmock(t, '{LIB}/utils/open-url.js', { '@npmcli/promise-spawn': { open }, }) @@ -34,110 +35,266 @@ const mockOpenUrl = async (t, args, { openerResult, ...config } = {}) => { } } -t.test('opens a url', async t => { - const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, - ['https://www.npmjs.com', 'npm home']) - t.equal(openerUrl(), 'https://www.npmjs.com', 'opened the given url') - t.same(openerOpts(), { command: null }, 'passed command as null (the default)') - t.same(joinedOutput(), '', 'printed no output') -}) +const mockOpenUrlPrompt = async (t, { + questionShouldResolve = true, + openUrlPromptInterrupted = false, + openerResult = null, + isTTY = true, + emitter = null, + url: openUrl = 'https://www.npmjs.com', + ...config +}) => { + const mock = await mockNpm(t, { + globals: { + 'process.stdin.isTTY': isTTY, + 'process.stdout.isTTY': isTTY, + }, + config, + }) -t.test('returns error for non-https url', async t => { - const { openUrl, openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t) - await t.rejects( - openUrl('ftp://www.npmjs.com', 'npm home'), - /Invalid URL/, - 'got the correct error' - ) - t.equal(openerUrl(), null, 'did not open') - t.same(openerOpts(), null, 'did not open') - t.same(joinedOutput(), '', 'printed no output') -}) + let openerUrl = null + let openerOpts = null -t.test('returns error for file url', async t => { - const { openUrl, openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t) - await t.rejects( - openUrl('file:///usr/local/bin/ls', 'npm home'), - /Invalid URL/, - 'got the correct error' - ) - t.equal(openerUrl(), null, 'did not open') - t.same(openerOpts(), null, 'did not open') - t.same(joinedOutput(), '', 'printed no output') -}) + const { openUrlPrompt } = tmock(t, '{LIB}/utils/open-url.js', { + '@npmcli/promise-spawn': { + open: async (url, options) => { + openerUrl = url + openerOpts = options + if (openerResult) { + throw openerResult + } + }, + }, + readline: { + createInterface: () => ({ + question: (_q, cb) => { + if (questionShouldResolve === true) { + cb() + } + }, + close: () => {}, + on: (_signal, cb) => { + if (openUrlPromptInterrupted && _signal === 'SIGINT') { + cb() + } + }, + }), + }, + }) -t.test('file url allowed if explicitly asked for', async t => { - const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, - ['file:///man/page/npm-install', 'npm home', true]) - t.equal(openerUrl(), 'file:///man/page/npm-install', 'opened the given url') - t.same(openerOpts(), { command: null }, 'passed command as null (the default)') - t.same(joinedOutput(), '', 'printed no output') -}) + let error + const args = [mock.npm, openUrl, 'npm home', 'prompt'] + if (emitter) { + mock.open = openUrlPrompt(...args, emitter) + } else { + await openUrlPrompt(...args).catch((er) => error = er) + } -t.test('returns error for non-parseable url', async t => { - const { openUrl, openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t) - await t.rejects( - openUrl('git+ssh://user@host:repo.git', 'npm home'), - /Invalid URL/, - 'got the correct error' - ) - t.equal(openerUrl(), null, 'did not open') - t.same(openerOpts(), null, 'did not open') - t.same(joinedOutput(), '', 'printed no output') -}) + return { + ...mock, + openerUrl, + openerOpts, + OUTPUT: mock.joinedOutput(), + emitter, + error, + } +} -t.test('encodes non-URL-safe characters in url provided', async t => { - const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, - ['https://www.npmjs.com/|cat', 'npm home']) - t.equal(openerUrl(), 'https://www.npmjs.com/%7Ccat', 'opened the encoded url') - t.same(openerOpts(), { command: null }, 'passed command as null (the default)') - t.same(joinedOutput(), '', 'printed no output') -}) +t.test('open url prompt', async t => { + t.test('does not open a url in non-interactive environments', async t => { + const { openerUrl, openerOpts } = await mockOpenUrlPrompt(t, { isTTY: false }) -t.test('opens a url with the given browser', async t => { - const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, - ['https://www.npmjs.com', 'npm home'], { browser: 'chrome' }) - t.equal(openerUrl(), 'https://www.npmjs.com', 'opened the given url') - // FIXME: browser string is parsed as a boolean in config layer - // this is a bug that should be fixed or the config should not allow it - t.same(openerOpts(), { command: null }, 'passed the given browser as command') - t.same(joinedOutput(), '', 'printed no output') -}) + t.equal(openerUrl, null, 'did not open') + t.same(openerOpts, null, 'did not open') + }) -t.test('prints where to go when browser is disabled', async t => { - const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, - ['https://www.npmjs.com', 'npm home'], { browser: false }) - t.equal(openerUrl(), null, 'did not open') - t.same(openerOpts(), null, 'did not open') - t.matchSnapshot(joinedOutput(), 'printed expected message') -}) + t.test('opens a url', async t => { + const { OUTPUT, openerUrl, openerOpts } = await mockOpenUrlPrompt(t, { browser: true }) -t.test('prints where to go when browser is disabled and json is enabled', async t => { - const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, - ['https://www.npmjs.com', 'npm home'], { browser: false, json: true }) - t.equal(openerUrl(), null, 'did not open') - t.same(openerOpts(), null, 'did not open') - t.matchSnapshot(joinedOutput(), 'printed expected message') -}) + t.equal(openerUrl, 'https://www.npmjs.com', 'opened the given url') + t.same(openerOpts, { command: null }, 'passed command as null (the default)') + t.matchSnapshot(OUTPUT) + }) -t.test('prints where to go when given browser does not exist', async t => { - const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, - ['https://www.npmjs.com', 'npm home'], - { - openerResult: Object.assign(new Error('failed'), { code: 127 }), - } - ) + t.test('opens a url with browser string', async t => { + const { openerUrl, openerOpts } = await mockOpenUrlPrompt(t, { browser: 'firefox' }) + + t.equal(openerUrl, 'https://www.npmjs.com', 'opened the given url') + // FIXME: browser string is parsed as a boolean in config layer + // this is a bug that should be fixed or the config should not allow it + t.same(openerOpts, { command: null }, 'passed command as null (the default)') + }) + + t.test('prints json output', async t => { + const { OUTPUT } = await mockOpenUrlPrompt(t, { json: true }) + + t.matchSnapshot(OUTPUT) + }) + + t.test('returns error for non-https url', async t => { + const { error, OUTPUT, openerUrl, openerOpts } = await mockOpenUrlPrompt(t, { + url: 'ftp://www.npmjs.com', + }) + + t.match(error, /Invalid URL/, 'got the correct error') + t.equal(openerUrl, null, 'did not open') + t.same(openerOpts, null, 'did not open') + t.same(OUTPUT, '', 'printed no output') + }) + + t.test('does not open url if canceled', async t => { + const emitter = new EventEmitter() + const { openerUrl, openerOpts, open } = await mockOpenUrlPrompt(t, { + questionShouldResolve: false, + emitter, + }) + + emitter.emit('abort') + + await open - t.equal(openerUrl(), 'https://www.npmjs.com', 'tried to open the correct url') - t.same(openerOpts(), { command: null }, 'tried to use the correct browser') - t.matchSnapshot(joinedOutput(), 'printed expected message') + t.equal(openerUrl, null, 'did not open') + t.same(openerOpts, null, 'did not open') + }) + + t.test('returns error when opener errors', async t => { + const { error, openerUrl } = await mockOpenUrlPrompt(t, { + openerResult: Object.assign(new Error('Opener failed'), { code: 1 }), + }) + + t.match(error, /Opener failed/, 'got the correct error') + t.equal(openerUrl, 'https://www.npmjs.com', 'did not open') + }) + + t.test('does not error when opener can not find command', async t => { + const { OUTPUT, error, openerUrl } = await mockOpenUrlPrompt(t, { + // openerResult: new Error('Opener failed'), + openerResult: Object.assign(new Error('Opener failed'), { code: 127 }), + }) + + t.notOk(error, 'Did not error') + t.equal(openerUrl, 'https://www.npmjs.com', 'did not open') + t.matchSnapshot(OUTPUT, 'Outputs extra Browser unavailable message and url') + }) + + t.test('throws "canceled" error on SIGINT', async t => { + const emitter = new EventEmitter() + const { open } = await mockOpenUrlPrompt(t, { + questionShouldResolve: false, + openUrlPromptInterrupted: true, + emitter, + }) + + await t.rejects(open, /canceled/, 'message is canceled') + }) }) -t.test('handles unknown opener error', async t => { - const { openUrl } = await mockOpenUrl(t, null, { - browser: 'firefox', - openerResult: Object.assign(new Error('failed'), { code: 'ENOBRIAN' }), +t.test('open url', async t => { + t.test('opens a url', async t => { + const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, + ['https://www.npmjs.com', 'npm home']) + t.equal(openerUrl(), 'https://www.npmjs.com', 'opened the given url') + t.same(openerOpts(), { command: null }, 'passed command as null (the default)') + t.same(joinedOutput(), '', 'printed no output') + }) + + t.test('returns error for non-https url', async t => { + const { openUrl, openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t) + await t.rejects( + openUrl('ftp://www.npmjs.com', 'npm home'), + /Invalid URL/, + 'got the correct error' + ) + t.equal(openerUrl(), null, 'did not open') + t.same(openerOpts(), null, 'did not open') + t.same(joinedOutput(), '', 'printed no output') + }) + + t.test('returns error for file url', async t => { + const { openUrl, openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t) + await t.rejects( + openUrl('file:///usr/local/bin/ls', 'npm home'), + /Invalid URL/, + 'got the correct error' + ) + t.equal(openerUrl(), null, 'did not open') + t.same(openerOpts(), null, 'did not open') + t.same(joinedOutput(), '', 'printed no output') + }) + + t.test('file url allowed if explicitly asked for', async t => { + const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, + ['file:///man/page/npm-install', 'npm home', true]) + t.equal(openerUrl(), 'file:///man/page/npm-install', 'opened the given url') + t.same(openerOpts(), { command: null }, 'passed command as null (the default)') + t.same(joinedOutput(), '', 'printed no output') }) - await t.rejects(openUrl('https://www.npmjs.com', 'npm home'), 'failed', 'got the correct error') + t.test('returns error for non-parseable url', async t => { + const { openUrl, openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t) + await t.rejects( + openUrl('git+ssh://user@host:repo.git', 'npm home'), + /Invalid URL/, + 'got the correct error' + ) + t.equal(openerUrl(), null, 'did not open') + t.same(openerOpts(), null, 'did not open') + t.same(joinedOutput(), '', 'printed no output') + }) + + t.test('encodes non-URL-safe characters in url provided', async t => { + const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, + ['https://www.npmjs.com/|cat', 'npm home']) + t.equal(openerUrl(), 'https://www.npmjs.com/%7Ccat', 'opened the encoded url') + t.same(openerOpts(), { command: null }, 'passed command as null (the default)') + t.same(joinedOutput(), '', 'printed no output') + }) + + t.test('opens a url with the given browser', async t => { + const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, + ['https://www.npmjs.com', 'npm home'], { browser: 'chrome' }) + t.equal(openerUrl(), 'https://www.npmjs.com', 'opened the given url') + // FIXME: browser string is parsed as a boolean in config layer + // this is a bug that should be fixed or the config should not allow it + t.same(openerOpts(), { command: null }, 'passed the given browser as command') + t.same(joinedOutput(), '', 'printed no output') + }) + + t.test('prints where to go when browser is disabled', async t => { + const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, + ['https://www.npmjs.com', 'npm home'], { browser: false }) + t.equal(openerUrl(), null, 'did not open') + t.same(openerOpts(), null, 'did not open') + t.matchSnapshot(joinedOutput(), 'printed expected message') + }) + + t.test('prints where to go when browser is disabled and json is enabled', async t => { + const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, + ['https://www.npmjs.com', 'npm home'], { browser: false, json: true }) + t.equal(openerUrl(), null, 'did not open') + t.same(openerOpts(), null, 'did not open') + t.matchSnapshot(joinedOutput(), 'printed expected message') + }) + + t.test('prints where to go when given browser does not exist', async t => { + const { openerUrl, openerOpts, joinedOutput } = await mockOpenUrl(t, + ['https://www.npmjs.com', 'npm home'], + { + openerResult: Object.assign(new Error('failed'), { code: 127 }), + } + ) + + t.equal(openerUrl(), 'https://www.npmjs.com', 'tried to open the correct url') + t.same(openerOpts(), { command: null }, 'tried to use the correct browser') + t.matchSnapshot(joinedOutput(), 'printed expected message') + }) + + t.test('handles unknown opener error', async t => { + const { openUrl } = await mockOpenUrl(t, null, { + browser: 'firefox', + openerResult: Object.assign(new Error('failed'), { code: 'ENOBRIAN' }), + }) + + await t.rejects(openUrl('https://www.npmjs.com', 'npm home'), 'failed', 'got the correct error') + }) }) diff --git a/test/lib/utils/otplease.js b/test/lib/utils/otplease.js index 6dc3ee0f0b069..502364e282ea8 100644 --- a/test/lib/utils/otplease.js +++ b/test/lib/utils/otplease.js @@ -14,7 +14,7 @@ const setupOtplease = async (t, { otp = {}, ...rest }, fn) => { const otplease = tmock(t, '{LIB}/utils/otplease.js', { '{LIB}/utils/read-user-info.js': readUserInfo, - '{LIB}/utils/open-url-prompt.js': () => {}, + '{LIB}/utils/open-url.js': { openUrlPrompt: () => {} }, '{LIB}/utils/web-auth': webAuth, })